Skip to content

Commit

Permalink
fully format generated files if given explicitly
Browse files Browse the repository at this point in the history
Otherwise it's very hard to apply gofumpt rules to generated code.
It still makes sense to skip the added rules with "gofumpt -w .",
because more often than not code generators only obey gofmt,
and it's often unnecessary to force generated code to follow gofumpt.

The behavior is similar to how we treat vendor directories,
with the difference that we do fully skip vendor directories by default,
whereas for generated Go files we still apply gofmt.
The difference is that changing vendored files may upset cmd/go,
and walking the vendor directory can unnecessarily waste time.

While here, tweak the README to better explain these special cases.

Fixes #180.
  • Loading branch information
mvdan committed Feb 20, 2022
1 parent 9685193 commit d74ca43
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 42 deletions.
12 changes: 6 additions & 6 deletions README.md
Expand Up @@ -11,14 +11,14 @@ replacement. Running `gofmt` after `gofumpt` should be a no-op. For example:
gofumpt -l -w .

Some of the Go source files in this repository belong to the Go project.
The added formatting rules are in the `format` package.
The [added formatting rules](#Added-rules) are implemented in the `format` package.

Beyond the [added rules below](#Added-rules), the tool differs from gofmt in the
following ways:
Note that vendor directories are skipped unless given as explicit arguments.
Similarly, the added rules do not apply to generated Go files unless they are
given as explicit arguments.

* Vendor directories are skipped unless given as explicit arguments
* The added rules are not applied to generated Go files
* The `-r` rewrite feature is removed in favor of `gofmt -r`
Finally, note that the `-r` rewrite flag is removed in favor of `gofmt -r`,
and the `-s` flag is hidden as it is always enabled.

### Added rules

Expand Down
13 changes: 0 additions & 13 deletions format/format.go
Expand Up @@ -69,25 +69,12 @@ func Source(src []byte, opts Options) ([]byte, error) {
return buf.Bytes(), nil
}

var rxCodeGenerated = regexp.MustCompile(`^// Code generated .* DO NOT EDIT\.$`)

// File modifies a file and fset in place to follow gofumpt's format. The
// changes might include manipulating adding or removing newlines in fset,
// modifying the position of nodes, or modifying literal values.
func File(fset *token.FileSet, file *ast.File, opts Options) {
simplify(file)

for _, cg := range file.Comments {
if cg.Pos() > file.Package {
break
}
for _, line := range cg.List {
if rxCodeGenerated.MatchString(line.Text) {
return
}
}
}

if opts.LangVersion == "" {
opts.LangVersion = "v1"
} else if opts.LangVersion[0] != 'v' {
Expand Down
35 changes: 27 additions & 8 deletions gofmt.go
Expand Up @@ -18,6 +18,7 @@ import (
"io/fs"
"os"
"path/filepath"
"regexp"
"runtime"
"runtime/pprof"
"strings"
Expand Down Expand Up @@ -103,8 +104,24 @@ func isGoFile(f fs.DirEntry) bool {
return !f.IsDir() && !strings.HasPrefix(name, ".") && strings.HasSuffix(name, ".go")
}

var rxCodeGenerated = regexp.MustCompile(`^// Code generated .* DO NOT EDIT\.$`)

func isGenerated(file *ast.File) bool {
for _, cg := range file.Comments {
if cg.Pos() > file.Package {
return false
}
for _, line := range cg.List {
if rxCodeGenerated.MatchString(line.Text) {
return true
}
}
}
return false
}

// If in == nil, the source is the contents of the file with the given filename.
func processFile(filename string, in io.Reader, out io.Writer, stdin bool) error {
func processFile(filename string, in io.Reader, out io.Writer, stdin, explicit bool) error {
var perm os.FileMode = 0o644
if in == nil {
f, err := os.Open(filename)
Expand Down Expand Up @@ -143,10 +160,12 @@ func processFile(filename string, in io.Reader, out io.Writer, stdin bool) error
}
}

gformat.File(fileSet, file, gformat.Options{
LangVersion: *langVersion,
ExtraRules: *extraRules,
})
if explicit || !isGenerated(file) {
gformat.File(fileSet, file, gformat.Options{
LangVersion: *langVersion,
ExtraRules: *extraRules,
})
}

res, err := format(fileSet, file, sourceAdj, indentAdj, src, printer.Config{Mode: printerMode, Tabwidth: tabWidth})
if err != nil {
Expand Down Expand Up @@ -198,7 +217,7 @@ func visitFile(path string, f fs.DirEntry, err error) error {
if err != nil || !isGoFile(f) {
return err
}
if err := processFile(path, nil, os.Stdout, false); err != nil {
if err := processFile(path, nil, os.Stdout, false, false); err != nil {
report(err)
}
return nil
Expand Down Expand Up @@ -255,7 +274,7 @@ func gofumptMain() {
exitCode = 2
return
}
if err := processFile("<standard input>", os.Stdin, os.Stdout, true); err != nil {
if err := processFile("<standard input>", os.Stdin, os.Stdout, true, true); err != nil {
report(err)
}
return
Expand All @@ -267,7 +286,7 @@ func gofumptMain() {
report(err)
case !info.IsDir():
// Non-directory arguments are always formatted.
if err := processFile(arg, nil, os.Stdout, false); err != nil {
if err := processFile(arg, nil, os.Stdout, false, true); err != nil {
report(err)
}
default:
Expand Down
32 changes: 17 additions & 15 deletions testdata/scripts/generated.txt
@@ -1,27 +1,29 @@
cp foo.go foo.go.orig
cp foo.go foo.go.golden
exec gofmt -w foo.go.golden
gofumpt foo.go
cmp stdout foo.go.golden

gofumpt -w foo.go
cmp foo.go foo.go.golden

cp foo.go.orig foo.go
gofumpt -w .
cmp foo.go foo.go.golden
gofumpt -l .
! stdout .
! stderr .

-- foo.go --
// foo is a package about bar.

// Code generated by foo. DO NOT EDIT.
// versions:
// foo v1.26.0

package foo

type i interface {
func f() {

println("body")

}
-- foo.go.golden --
// foo is a package about bar.

a(x int) int
// Code generated by foo. DO NOT EDIT.

b(x int) int
package foo

c(x int) int
func f() {
println("body")
}

0 comments on commit d74ca43

Please sign in to comment.