Skip to content

Commit 4c4cefc

Browse files
committed
cmd/gofmt: simplify logic to process arguments
Rather than stat-ing each argument and taking different code paths depending on whether it's a directory or not, we can leverage the fact that filepath.WalkDir works on regular files and already has to figure out whether each file it walks is a directory or not. We can then implement "always format non-directory arguments" by looking at whether the path we are walking is the original argument, meaning we are walking the top file. For full clarity, we expand the skipping logic with a switch, as before it was a bit confusing how we could `return err` on directories and other non-Go files. Given that we discard directories separately now, simplify isGoFile to just be about filenames. While here, also note that we called AddReport inside WalkDir; this is unnecessary, as we can return the error for the same effect. Change-Id: I50ab94710143f19bd8dd95a69e01a3dd228e397e Reviewed-on: https://go-review.googlesource.com/c/go/+/700115 Reviewed-by: Sean Liao <[email protected]> Reviewed-by: Michael Pratt <[email protected]> Reviewed-by: Cherry Mui <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 925a3cd commit 4c4cefc

File tree

2 files changed

+25
-31
lines changed

2 files changed

+25
-31
lines changed

src/cmd/gofmt/gofmt.go

Lines changed: 24 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,8 @@ func initParserMode() {
8787
}
8888
}
8989

90-
func isGoFile(f fs.DirEntry) bool {
91-
// ignore non-Go files
92-
name := f.Name()
93-
return !strings.HasPrefix(name, ".") && strings.HasSuffix(name, ".go") && !f.IsDir()
90+
func isGoFilename(name string) bool {
91+
return !strings.HasPrefix(name, ".") && strings.HasSuffix(name, ".go")
9492
}
9593

9694
// A sequencer performs concurrent tasks that may write output, but emits that
@@ -411,34 +409,30 @@ func gofmtMain(s *sequencer) {
411409
}
412410

413411
for _, arg := range args {
414-
switch info, err := os.Stat(arg); {
415-
case err != nil:
416-
s.AddReport(err)
417-
case !info.IsDir():
418-
// Non-directory arguments are always formatted.
419-
arg := arg
420-
s.Add(fileWeight(arg, info), func(r *reporter) error {
421-
return processFile(arg, info, nil, r)
422-
})
423-
default:
424-
// Directories are walked, ignoring non-Go files.
425-
err := filepath.WalkDir(arg, func(path string, f fs.DirEntry, err error) error {
426-
if err != nil || !isGoFile(f) {
427-
return err
428-
}
429-
info, err := f.Info()
430-
if err != nil {
431-
s.AddReport(err)
432-
return nil
433-
}
434-
s.Add(fileWeight(path, info), func(r *reporter) error {
435-
return processFile(path, info, nil, r)
436-
})
437-
return nil
438-
})
412+
// Walk each given argument as a directory tree.
413+
// If the argument is not a directory, it's always formatted as a Go file.
414+
// If the argument is a directory, we walk it, ignoring non-Go files.
415+
if err := filepath.WalkDir(arg, func(path string, d fs.DirEntry, err error) error {
416+
switch {
417+
case err != nil:
418+
return err
419+
case d.IsDir():
420+
return nil // simply recurse into directories
421+
case path == arg:
422+
// non-directories given as explicit arguments are always formatted
423+
case !isGoFilename(d.Name()):
424+
return nil // skip walked non-Go files
425+
}
426+
info, err := d.Info()
439427
if err != nil {
440-
s.AddReport(err)
428+
return err
441429
}
430+
s.Add(fileWeight(path, info), func(r *reporter) error {
431+
return processFile(path, info, nil, r)
432+
})
433+
return nil
434+
}); err != nil {
435+
s.AddReport(err)
442436
}
443437
}
444438
}

src/cmd/gofmt/long_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ func genFilenames(t *testing.T, filenames chan<- string) {
115115
return nil
116116
}
117117
// don't descend into testdata directories
118-
if isGoFile(d) && !strings.Contains(filepath.ToSlash(filename), "/testdata/") {
118+
if !d.IsDir() && isGoFilename(d.Name()) && !strings.Contains(filepath.ToSlash(filename), "/testdata/") {
119119
filenames <- filename
120120
nfiles++
121121
}

0 commit comments

Comments
 (0)