Skip to content

Commit

Permalink
fix: correct handling of absolute and relative paths
Browse files Browse the repository at this point in the history
This fixes a few issues with both relative and absolute paths:

- Previously, treefmt refused to format any files passed as absolute
  paths, regardless of if they were in tree or not: #442
- Previously, treefmt would treat relative paths as relative to the
  project root, which is not ideal if you're in a subdirectory and you
  just want to format the file you're right next to: #443
  - There was even a test asserting this behavior. I changed it to
    reflect the updated behavior.
- treefmt's handling of paths outside of the project root was a bit
  inconsistent: #444
  • Loading branch information
jfly committed Oct 12, 2024
1 parent 659aa0f commit 0ee561e
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 21 deletions.
29 changes: 22 additions & 7 deletions cmd/format/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"path/filepath"
"runtime"
"runtime/pprof"
"strings"
"syscall"
"time"

Expand Down Expand Up @@ -173,11 +174,27 @@ func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string)
return fmt.Errorf("exactly one path should be specified when using the --stdin flag")
}
} else {
// checks all paths are contained within the tree root
for _, path := range paths {
rootPath := filepath.Join(cfg.TreeRoot, path)
if _, err = os.Stat(rootPath); err != nil {
return fmt.Errorf("path %s not found within the tree root %s", path, cfg.TreeRoot)
// checks all paths are contained within the tree root and exist
// also "normalize" paths so they're relative to cfg.TreeRoot
for i, path := range paths {
absolutePath, err := filepath.Abs(path)
if err != nil {
return fmt.Errorf("error computing absolute path of %s: %w", path, err)
}

relativePath, err := filepath.Rel(cfg.TreeRoot, absolutePath)
if err != nil {
return fmt.Errorf("error computing relative path from %s to %s: %s", cfg.TreeRoot, absolutePath, err)
}

if strings.HasPrefix(relativePath, "..") {
return fmt.Errorf("path %s not inside the tree root %s", path, cfg.TreeRoot)
}

paths[i] = relativePath

if _, err = os.Stat(absolutePath); err != nil {
return fmt.Errorf("path %s not found", path)
}
}
}
Expand All @@ -188,8 +205,6 @@ func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string)
return fmt.Errorf("failed to create walker: %w", err)
}

//

files := make([]*walk.File, BatchSize)
for {
ctx, cancel := context.WithTimeout(ctx, 1*time.Second)
Expand Down
51 changes: 40 additions & 11 deletions cmd/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ func TestGitWorktree(t *testing.T) {

// try with a bad path
_, _, err = treefmt(t, "-C", tempDir, "-c", "haskell", "foo")
as.ErrorContains(err, "path foo not found within the tree root")
as.ErrorContains(err, "path foo not found")

// try with a path not in the git index, e.g. it is skipped
_, err = os.Create(filepath.Join(tempDir, "foo.txt"))
Expand All @@ -647,11 +647,20 @@ func TestPathsArg(t *testing.T) {
as.NoError(os.Chdir(cwd))
})

tempDir := test.TempExamples(t)
configPath := filepath.Join(tempDir, "/treefmt.toml")
// create a project root under a temp dir, in order verify behavior with
// files inside of temp dir, but outside of the project root
tempDir := t.TempDir()
treeRoot := filepath.Join(tempDir, "tree-root")
test.TempExamplesInDir(t, treeRoot)

// change working directory to temp root
as.NoError(os.Chdir(tempDir))
configPath := filepath.Join(treeRoot, "/treefmt.toml")

// create a file outside of treeRoot
externalFile, err := os.Create(filepath.Join(tempDir, "outside_tree.go"))
as.NoError(err)

// change working directory to project root
as.NoError(os.Chdir(treeRoot))

// basic config
cfg := &config.Config{
Expand All @@ -675,14 +684,29 @@ func TestPathsArg(t *testing.T) {
as.NoError(err)
assertStats(t, as, statz, 2, 2, 2, 0)

// specify an absolute path
absoluteInternalPath, err := filepath.Abs("elm/elm.json")
as.NoError(err)
_, statz, err = treefmt(t, "-c", absoluteInternalPath)
as.NoError(err)
assertStats(t, as, statz, 1, 1, 1, 0)

// specify a bad path
_, _, err = treefmt(t, "-c", "elm/elm.json", "haskell/Nested/Bar.hs")
as.ErrorContains(err, "path haskell/Nested/Bar.hs not found within the tree root")
as.Errorf(err, "path haskell/Nested/Bar.hs not found")

// specify a path outside the tree root
externalPath := filepath.Join(cwd, "go.mod")
_, _, err = treefmt(t, "-c", externalPath)
as.ErrorContains(err, fmt.Sprintf("path %s not found within the tree root", externalPath))
// specify an absolute path outside the tree root
absoluteExternalPath, err := filepath.Abs(externalFile.Name())
as.NoError(err)
as.FileExists(absoluteExternalPath, "exernal file must exist")
_, _, err = treefmt(t, "-c", absoluteExternalPath)
as.Errorf(err, "path %s not found within the tree root", absoluteExternalPath)

// specify a relative path outside the tree root
relativeExternalPath := "../outside_tree.go"
as.FileExists(relativeExternalPath, "exernal file must exist")
_, _, err = treefmt(t, "-c", relativeExternalPath)
as.Errorf(err, "path %s not found within the tree root", relativeExternalPath)
}

func TestStdin(t *testing.T) {
Expand Down Expand Up @@ -852,7 +876,12 @@ func TestRunInSubdir(t *testing.T) {
assertStats(t, as, statz, 32, 32, 32, 0)

// specify some explicit paths, relative to the tree root
_, statz, err = treefmt(t, "-c", "elm/elm.json", "haskell/Nested/Foo.hs")
// this should not work, as we're in a subdirectory
_, _, err = treefmt(t, "-c", "elm/elm.json", "haskell/Nested/Foo.hs")
as.ErrorContains(err, "path elm/elm.json not found")

// specify some explicit paths, relative to the current directory
_, statz, err = treefmt(t, "-c", "elm.json", "../haskell/Nested/Foo.hs")
as.NoError(err)
assertStats(t, as, statz, 2, 2, 2, 0)
}
Expand Down
9 changes: 6 additions & 3 deletions test/temp.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,16 @@ func WriteConfig(t *testing.T, path string, cfg *config.Config) {

func TempExamples(t *testing.T) string {
tempDir := t.TempDir()
require.NoError(t, cp.Copy("../test/examples", tempDir), "failed to copy test data to temp dir")
TempExamplesInDir(t, tempDir)
return tempDir
}

func TempExamplesInDir(t *testing.T, dir string) {
require.NoError(t, cp.Copy("../test/examples", dir), "failed to copy test data to dir")

// we have second precision mod time tracking, so we wait a second before returning, so we don't trigger false
//positives for things like fail on change
time.Sleep(time.Second)

return tempDir
}

func TempFile(t *testing.T, dir string, pattern string, contents *string) *os.File {
Expand Down

0 comments on commit 0ee561e

Please sign in to comment.