From c32140fa94cfc51a2152855825f57e27ae3ba133 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 4 Nov 2020 18:20:17 -0500 Subject: [PATCH] all: update to use filepath.WalkDir instead of filepath.Walk Now that filepath.WalkDir is available, it is more efficient and should be used in place of filepath.Walk. Update the tree to reflect best practices. As usual, the code compiled with Go 1.4 during bootstrap is excluded. (In this CL, that's only cmd/dist.) For #42027. Change-Id: Ib0f7b1e43e50b789052f9835a63ced701d8c411c Reviewed-on: https://go-review.googlesource.com/c/go/+/267719 Trust: Russ Cox Run-TryBot: Russ Cox TryBot-Result: Go Bot Reviewed-by: Rob Pike --- src/cmd/compile/fmt_test.go | 3 ++- src/cmd/dist/test.go | 1 + src/cmd/fix/main.go | 6 +++--- src/cmd/go/go_test.go | 9 +++------ src/cmd/go/internal/modfetch/fetch.go | 9 +++++---- src/cmd/go/internal/version/version.go | 12 ++++++++++-- src/cmd/go/testdata/addmod.go | 4 ++-- src/cmd/go/testdata/savedir.go | 4 ++-- src/cmd/gofmt/gofmt.go | 6 +++--- src/cmd/gofmt/long_test.go | 17 +++++++++++++---- src/cmd/internal/moddeps/moddeps_test.go | 2 +- src/compress/gzip/issue14937_test.go | 2 +- src/go/build/deps_test.go | 6 +++--- src/go/doc/headscan.go | 4 ++-- src/index/suffixarray/suffixarray_test.go | 2 +- test/run.go | 17 ++++++++++------- test/winbatch.go | 6 +++--- 17 files changed, 65 insertions(+), 45 deletions(-) diff --git a/src/cmd/compile/fmt_test.go b/src/cmd/compile/fmt_test.go index e372259c788e18..6625ccf5e24a80 100644 --- a/src/cmd/compile/fmt_test.go +++ b/src/cmd/compile/fmt_test.go @@ -52,6 +52,7 @@ import ( "go/types" "internal/testenv" "io" + "io/fs" "io/ioutil" "log" "os" @@ -89,7 +90,7 @@ func TestFormats(t *testing.T) { testenv.MustHaveGoBuild(t) // more restrictive than necessary, but that's ok // process all directories - filepath.Walk(".", func(path string, info os.FileInfo, err error) error { + filepath.WalkDir(".", func(path string, info fs.DirEntry, err error) error { if info.IsDir() { if info.Name() == "testdata" { return filepath.SkipDir diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go index 2a17ab9caebb0b..955ce2a063c833 100644 --- a/src/cmd/dist/test.go +++ b/src/cmd/dist/test.go @@ -1509,6 +1509,7 @@ func (t *tester) makeGOROOTUnwritable() (undo func()) { } gocacheSubdir, _ := filepath.Rel(dir, gocache) + // Note: Can't use WalkDir here, because this has to compile with Go 1.4. filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { if suffix := strings.TrimPrefix(path, dir+string(filepath.Separator)); suffix != "" { if suffix == gocacheSubdir { diff --git a/src/cmd/fix/main.go b/src/cmd/fix/main.go index 1cea9a876af190..1cedf992cf3458 100644 --- a/src/cmd/fix/main.go +++ b/src/cmd/fix/main.go @@ -234,10 +234,10 @@ func report(err error) { } func walkDir(path string) { - filepath.Walk(path, visitFile) + filepath.WalkDir(path, visitFile) } -func visitFile(path string, f fs.FileInfo, err error) error { +func visitFile(path string, f fs.DirEntry, err error) error { if err == nil && isGoFile(f) { err = processFile(path, false) } @@ -247,7 +247,7 @@ func visitFile(path string, f fs.FileInfo, err error) error { return nil } -func isGoFile(f fs.FileInfo) bool { +func isGoFile(f fs.DirEntry) bool { // ignore non-Go files name := f.Name() return !f.IsDir() && !strings.HasPrefix(name, ".") && strings.HasSuffix(name, ".go") diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index a02231fa98324d..a730c87f977814 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -774,7 +774,7 @@ func (tg *testgoData) cleanup() { func removeAll(dir string) error { // module cache has 0444 directories; // make them writable in order to remove content. - filepath.Walk(dir, func(path string, info fs.FileInfo, err error) error { + filepath.WalkDir(dir, func(path string, info fs.DirEntry, err error) error { // chmod not only directories, but also things that we couldn't even stat // due to permission errors: they may also be unreadable directories. if err != nil || info.IsDir() { @@ -820,8 +820,8 @@ func TestNewReleaseRebuildsStalePackagesInGOPATH(t *testing.T) { } { srcdir := filepath.Join(testGOROOT, copydir) tg.tempDir(filepath.Join("goroot", copydir)) - err := filepath.Walk(srcdir, - func(path string, info fs.FileInfo, err error) error { + err := filepath.WalkDir(srcdir, + func(path string, info fs.DirEntry, err error) error { if err != nil { return err } @@ -838,9 +838,6 @@ func TestNewReleaseRebuildsStalePackagesInGOPATH(t *testing.T) { return err } tg.tempFile(dest, string(data)) - if err := os.Chmod(tg.path(dest), info.Mode()|0200); err != nil { - return err - } return nil }) if err != nil { diff --git a/src/cmd/go/internal/modfetch/fetch.go b/src/cmd/go/internal/modfetch/fetch.go index a3e2cd1f9d66cc..2ee78de5b24847 100644 --- a/src/cmd/go/internal/modfetch/fetch.go +++ b/src/cmd/go/internal/modfetch/fetch.go @@ -318,9 +318,10 @@ func makeDirsReadOnly(dir string) { mode fs.FileMode } var dirs []pathMode // in lexical order - filepath.Walk(dir, func(path string, info fs.FileInfo, err error) error { - if err == nil && info.Mode()&0222 != 0 { - if info.IsDir() { + filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { + if err == nil && d.IsDir() { + info, err := d.Info() + if err == nil && info.Mode()&0222 != 0 { dirs = append(dirs, pathMode{path, info.Mode()}) } } @@ -337,7 +338,7 @@ func makeDirsReadOnly(dir string) { // any permission changes needed to do so. func RemoveAll(dir string) error { // Module cache has 0555 directories; make them writable in order to remove content. - filepath.Walk(dir, func(path string, info fs.FileInfo, err error) error { + filepath.WalkDir(dir, func(path string, info fs.DirEntry, err error) error { if err != nil { return nil // ignore errors walking in file system } diff --git a/src/cmd/go/internal/version/version.go b/src/cmd/go/internal/version/version.go index 44ac24c62d418a..58cbd32e78d115 100644 --- a/src/cmd/go/internal/version/version.go +++ b/src/cmd/go/internal/version/version.go @@ -88,8 +88,15 @@ func runVersion(ctx context.Context, cmd *base.Command, args []string) { // scanDir scans a directory for executables to run scanFile on. func scanDir(dir string) { - filepath.Walk(dir, func(path string, info fs.FileInfo, err error) error { - if info.Mode().IsRegular() || info.Mode()&fs.ModeSymlink != 0 { + filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { + if d.Type().IsRegular() || d.Type()&fs.ModeSymlink != 0 { + info, err := d.Info() + if err != nil { + if *versionV { + fmt.Fprintf(os.Stderr, "%s: %v\n", path, err) + } + return nil + } scanFile(path, info, *versionV) } return nil @@ -120,6 +127,7 @@ func scanFile(file string, info fs.FileInfo, mustPrint bool) { } info = i } + if !isExe(file, info) { if mustPrint { fmt.Fprintf(os.Stderr, "%s: not executable file\n", file) diff --git a/src/cmd/go/testdata/addmod.go b/src/cmd/go/testdata/addmod.go index d1b6467c5d1cd9..71ac47fdc1250f 100644 --- a/src/cmd/go/testdata/addmod.go +++ b/src/cmd/go/testdata/addmod.go @@ -122,8 +122,8 @@ func main() { {Name: ".info", Data: info}, } dir = filepath.Clean(dir) - err = filepath.Walk(dir, func(path string, info fs.FileInfo, err error) error { - if !info.Mode().IsRegular() { + err = filepath.WalkDir(dir, func(path string, info fs.DirEntry, err error) error { + if !info.Type().IsRegular() { return nil } name := info.Name() diff --git a/src/cmd/go/testdata/savedir.go b/src/cmd/go/testdata/savedir.go index 04902df61ef225..75895ee279f96d 100644 --- a/src/cmd/go/testdata/savedir.go +++ b/src/cmd/go/testdata/savedir.go @@ -49,7 +49,7 @@ func main() { a := new(txtar.Archive) dir = filepath.Clean(dir) - filepath.Walk(dir, func(path string, info fs.FileInfo, err error) error { + filepath.WalkDir(dir, func(path string, info fs.DirEntry, err error) error { if path == dir { return nil } @@ -60,7 +60,7 @@ func main() { } return nil } - if !info.Mode().IsRegular() { + if !info.Type().IsRegular() { return nil } data, err := ioutil.ReadFile(path) diff --git a/src/cmd/gofmt/gofmt.go b/src/cmd/gofmt/gofmt.go index dba2411eed356a..719c681a3e37ec 100644 --- a/src/cmd/gofmt/gofmt.go +++ b/src/cmd/gofmt/gofmt.go @@ -74,7 +74,7 @@ func initParserMode() { } } -func isGoFile(f fs.FileInfo) bool { +func isGoFile(f fs.DirEntry) bool { // ignore non-Go files name := f.Name() return !f.IsDir() && !strings.HasPrefix(name, ".") && strings.HasSuffix(name, ".go") @@ -164,7 +164,7 @@ func processFile(filename string, in io.Reader, out io.Writer, stdin bool) error return err } -func visitFile(path string, f fs.FileInfo, err error) error { +func visitFile(path string, f fs.DirEntry, err error) error { if err == nil && isGoFile(f) { err = processFile(path, nil, os.Stdout, false) } @@ -177,7 +177,7 @@ func visitFile(path string, f fs.FileInfo, err error) error { } func walkDir(path string) { - filepath.Walk(path, visitFile) + filepath.WalkDir(path, visitFile) } func main() { diff --git a/src/cmd/gofmt/long_test.go b/src/cmd/gofmt/long_test.go index 28306ce83e71df..4a821705f16d06 100644 --- a/src/cmd/gofmt/long_test.go +++ b/src/cmd/gofmt/long_test.go @@ -108,12 +108,12 @@ func testFiles(t *testing.T, filenames <-chan string, done chan<- int) { func genFilenames(t *testing.T, filenames chan<- string) { defer close(filenames) - handleFile := func(filename string, fi fs.FileInfo, err error) error { + handleFile := func(filename string, d fs.DirEntry, err error) error { if err != nil { t.Error(err) return nil } - if isGoFile(fi) { + if isGoFile(d) { filenames <- filename nfiles++ } @@ -124,13 +124,13 @@ func genFilenames(t *testing.T, filenames chan<- string) { if *files != "" { for _, filename := range strings.Split(*files, ",") { fi, err := os.Stat(filename) - handleFile(filename, fi, err) + handleFile(filename, &statDirEntry{fi}, err) } return // ignore files under -root } // otherwise, test all Go files under *root - filepath.Walk(*root, handleFile) + filepath.WalkDir(*root, handleFile) } func TestAll(t *testing.T) { @@ -164,3 +164,12 @@ func TestAll(t *testing.T) { fmt.Printf("processed %d files\n", nfiles) } } + +type statDirEntry struct { + info fs.FileInfo +} + +func (d *statDirEntry) Name() string { return d.info.Name() } +func (d *statDirEntry) IsDir() bool { return d.info.IsDir() } +func (d *statDirEntry) Type() fs.FileMode { return d.info.Mode().Type() } +func (d *statDirEntry) Info() (fs.FileInfo, error) { return d.info, nil } diff --git a/src/cmd/internal/moddeps/moddeps_test.go b/src/cmd/internal/moddeps/moddeps_test.go index 7362e7868b67da..9ea21873c5ed90 100644 --- a/src/cmd/internal/moddeps/moddeps_test.go +++ b/src/cmd/internal/moddeps/moddeps_test.go @@ -33,7 +33,7 @@ func findGorootModules(t *testing.T) []gorootModule { goBin := testenv.GoToolPath(t) goroot.once.Do(func() { - goroot.err = filepath.Walk(runtime.GOROOT(), func(path string, info fs.FileInfo, err error) error { + goroot.err = filepath.WalkDir(runtime.GOROOT(), func(path string, info fs.DirEntry, err error) error { if err != nil { return err } diff --git a/src/compress/gzip/issue14937_test.go b/src/compress/gzip/issue14937_test.go index 24db3641aa74a9..20da0b6824a6a9 100644 --- a/src/compress/gzip/issue14937_test.go +++ b/src/compress/gzip/issue14937_test.go @@ -31,7 +31,7 @@ func TestGZIPFilesHaveZeroMTimes(t *testing.T) { t.Fatal("error evaluating GOROOT: ", err) } var files []string - err = filepath.Walk(goroot, func(path string, info fs.FileInfo, err error) error { + err = filepath.WalkDir(goroot, func(path string, info fs.DirEntry, err error) error { if err != nil { return err } diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index bf1367355d1504..e9ed26aa5fdbff 100644 --- a/src/go/build/deps_test.go +++ b/src/go/build/deps_test.go @@ -510,8 +510,8 @@ func listStdPkgs(goroot string) ([]string, error) { var pkgs []string src := filepath.Join(goroot, "src") + string(filepath.Separator) - walkFn := func(path string, fi fs.FileInfo, err error) error { - if err != nil || !fi.IsDir() || path == src { + walkFn := func(path string, d fs.DirEntry, err error) error { + if err != nil || !d.IsDir() || path == src { return nil } @@ -528,7 +528,7 @@ func listStdPkgs(goroot string) ([]string, error) { pkgs = append(pkgs, strings.TrimPrefix(name, "vendor/")) return nil } - if err := filepath.Walk(src, walkFn); err != nil { + if err := filepath.WalkDir(src, walkFn); err != nil { return nil, err } return pkgs, nil diff --git a/src/go/doc/headscan.go b/src/go/doc/headscan.go index 8ea462366e045e..fe26a0ea842695 100644 --- a/src/go/doc/headscan.go +++ b/src/go/doc/headscan.go @@ -69,8 +69,8 @@ func main() { flag.Parse() fset := token.NewFileSet() nheadings := 0 - err := filepath.Walk(*root, func(path string, fi fs.FileInfo, err error) error { - if !fi.IsDir() { + err := filepath.WalkDir(*root, func(path string, info fs.DirEntry, err error) error { + if !info.IsDir() { return nil } pkgs, err := parser.ParseDir(fset, path, isGoFile, parser.ParseComments) diff --git a/src/index/suffixarray/suffixarray_test.go b/src/index/suffixarray/suffixarray_test.go index b6a81123b7406d..a11a98dae0de1a 100644 --- a/src/index/suffixarray/suffixarray_test.go +++ b/src/index/suffixarray/suffixarray_test.go @@ -503,7 +503,7 @@ func makeText(name string) ([]byte, error) { return nil, err } case "go": - err := filepath.Walk("../..", func(path string, info fs.FileInfo, err error) error { + err := filepath.WalkDir("../..", func(path string, info fs.DirEntry, err error) error { if err == nil && strings.HasSuffix(path, ".go") && !info.IsDir() { file, err := ioutil.ReadFile(path) if err != nil { diff --git a/test/run.go b/test/run.go index 672861c8d78136..4abf32d25c8b67 100644 --- a/test/run.go +++ b/test/run.go @@ -14,6 +14,7 @@ import ( "fmt" "hash/fnv" "io" + "io/fs" "io/ioutil" "log" "os" @@ -1793,7 +1794,7 @@ func overlayDir(dstRoot, srcRoot string) error { return err } - return filepath.Walk(srcRoot, func(srcPath string, info os.FileInfo, err error) error { + return filepath.WalkDir(srcRoot, func(srcPath string, d fs.DirEntry, err error) error { if err != nil || srcPath == srcRoot { return err } @@ -1804,14 +1805,16 @@ func overlayDir(dstRoot, srcRoot string) error { } dstPath := filepath.Join(dstRoot, suffix) - perm := info.Mode() & os.ModePerm - if info.Mode()&os.ModeSymlink != 0 { + var info fs.FileInfo + if d.Type()&os.ModeSymlink != 0 { info, err = os.Stat(srcPath) - if err != nil { - return err - } - perm = info.Mode() & os.ModePerm + } else { + info, err = d.Info() } + if err != nil { + return err + } + perm := info.Mode() & os.ModePerm // Always copy directories (don't symlink them). // If we add a file in the overlay, we don't want to add it in the original. diff --git a/test/winbatch.go b/test/winbatch.go index c3b48d385cecc8..54c2fff13463bd 100644 --- a/test/winbatch.go +++ b/test/winbatch.go @@ -27,11 +27,11 @@ func main() { // Walk the entire Go repository source tree (without GOROOT/pkg), // skipping directories that start with "." and named "testdata", // and ensure all .bat files found have exact CRLF line endings. - err := filepath.Walk(runtime.GOROOT(), func(path string, fi os.FileInfo, err error) error { + err := filepath.WalkDir(runtime.GOROOT(), func(path string, d os.DirEntry, err error) error { if err != nil { return err } - if fi.IsDir() && (strings.HasPrefix(fi.Name(), ".") || fi.Name() == "testdata") { + if d.IsDir() && (strings.HasPrefix(d.Name(), ".") || d.Name() == "testdata") { return filepath.SkipDir } if path == filepath.Join(runtime.GOROOT(), "pkg") { @@ -39,7 +39,7 @@ func main() { // Skip it to avoid false positives. (Also see golang.org/issue/37929.) return filepath.SkipDir } - if filepath.Ext(fi.Name()) == ".bat" { + if filepath.Ext(d.Name()) == ".bat" { enforceBatchStrictCRLF(path) } return nil