Skip to content

Commit

Permalink
all: update to use filepath.WalkDir instead of filepath.Walk
Browse files Browse the repository at this point in the history
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 <[email protected]>
Run-TryBot: Russ Cox <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Rob Pike <[email protected]>
  • Loading branch information
rsc committed Dec 2, 2020
1 parent 0433845 commit c32140f
Show file tree
Hide file tree
Showing 17 changed files with 65 additions and 45 deletions.
3 changes: 2 additions & 1 deletion src/cmd/compile/fmt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import (
"go/types"
"internal/testenv"
"io"
"io/fs"
"io/ioutil"
"log"
"os"
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/cmd/dist/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions src/cmd/fix/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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")
Expand Down
9 changes: 3 additions & 6 deletions src/cmd/go/go_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down
9 changes: 5 additions & 4 deletions src/cmd/go/internal/modfetch/fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()})
}
}
Expand All @@ -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
}
Expand Down
12 changes: 10 additions & 2 deletions src/cmd/go/internal/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/go/testdata/addmod.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/go/testdata/savedir.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -60,7 +60,7 @@ func main() {
}
return nil
}
if !info.Mode().IsRegular() {
if !info.Type().IsRegular() {
return nil
}
data, err := ioutil.ReadFile(path)
Expand Down
6 changes: 3 additions & 3 deletions src/cmd/gofmt/gofmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
}
Expand All @@ -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() {
Expand Down
17 changes: 13 additions & 4 deletions src/cmd/gofmt/long_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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++
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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 }
2 changes: 1 addition & 1 deletion src/cmd/internal/moddeps/moddeps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion src/compress/gzip/issue14937_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
6 changes: 3 additions & 3 deletions src/go/build/deps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/go/doc/headscan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/index/suffixarray/suffixarray_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
17 changes: 10 additions & 7 deletions test/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"fmt"
"hash/fnv"
"io"
"io/fs"
"io/ioutil"
"log"
"os"
Expand Down Expand Up @@ -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
}
Expand All @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions test/winbatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,19 @@ 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") {
// GOROOT/pkg is known to contain generated artifacts, not source code.
// 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
Expand Down

0 comments on commit c32140f

Please sign in to comment.