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 <sean@liao.dev>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

diff --git a/src/cmd/gofmt/gofmt.go b/src/cmd/gofmt/gofmt.go
index d91a75b1050e2090e92bfbfec92e72edc61c7c99..bbb8b4fd15c2f74e64ac0cbeba6395cbf4856241 100644 (file)
--- a/src/cmd/gofmt/gofmt.go
+++ b/src/cmd/gofmt/gofmt.go
@@ -87,10 +87,8 @@ func initParserMode() {
        }
 }
 
-func isGoFile(f fs.DirEntry) bool {
-       // ignore non-Go files
-       name := f.Name()
-       return !strings.HasPrefix(name, ".") && strings.HasSuffix(name, ".go") && !f.IsDir()
+func isGoFilename(name string) bool {
+       return !strings.HasPrefix(name, ".") && strings.HasSuffix(name, ".go")
 }
 
 // A sequencer performs concurrent tasks that may write output, but emits that
@@ -411,34 +409,30 @@ func gofmtMain(s *sequencer) {
        }
 
        for _, arg := range args {
-               switch info, err := os.Stat(arg); {
-               case err != nil:
-                       s.AddReport(err)
-               case !info.IsDir():
-                       // Non-directory arguments are always formatted.
-                       arg := arg
-                       s.Add(fileWeight(arg, info), func(r *reporter) error {
-                               return processFile(arg, info, nil, r)
-                       })
-               default:
-                       // Directories are walked, ignoring non-Go files.
-                       err := filepath.WalkDir(arg, func(path string, f fs.DirEntry, err error) error {
-                               if err != nil || !isGoFile(f) {
-                                       return err
-                               }
-                               info, err := f.Info()
-                               if err != nil {
-                                       s.AddReport(err)
-                                       return nil
-                               }
-                               s.Add(fileWeight(path, info), func(r *reporter) error {
-                                       return processFile(path, info, nil, r)
-                               })
-                               return nil
-                       })
+               // Walk each given argument as a directory tree.
+               // If the argument is not a directory, it's always formatted as a Go file.
+               // If the argument is a directory, we walk it, ignoring non-Go files.
+               if err := filepath.WalkDir(arg, func(path string, d fs.DirEntry, err error) error {
+                       switch {
+                       case err != nil:
+                               return err
+                       case d.IsDir():
+                               return nil // simply recurse into directories
+                       case path == arg:
+                               // non-directories given as explicit arguments are always formatted
+                       case !isGoFilename(d.Name()):
+                               return nil // skip walked non-Go files
+                       }
+                       info, err := d.Info()
                        if err != nil {
-                               s.AddReport(err)
+                               return err
                        }
+                       s.Add(fileWeight(path, info), func(r *reporter) error {
+                               return processFile(path, info, nil, r)
+                       })
+                       return nil
+               }); err != nil {
+                       s.AddReport(err)
                }
        }
 }

diff --git a/src/cmd/gofmt/long_test.go b/src/cmd/gofmt/long_test.go
index 21a01196cf6cc285b4e36f99ef5269cce3a79d11..372e324387843da948dc14f23bbf801a6fd1e3f4 100644 (file)
--- a/src/cmd/gofmt/long_test.go
+++ b/src/cmd/gofmt/long_test.go
@@ -115,7 +115,7 @@ func genFilenames(t *testing.T, filenames chan<- string) {
                        return nil
                }
                // don't descend into testdata directories
-               if isGoFile(d) && !strings.Contains(filepath.ToSlash(filename), "/testdata/") {
+               if !d.IsDir() && isGoFilename(d.Name()) && !strings.Contains(filepath.ToSlash(filename), "/testdata/") {
                        filenames <- filename
                        nfiles++
                }