x/tools/cmd/goimports: add a flag to skip generated files

[adambabik]

I use goimports as follows:

goimports -local="github.com/my/package" -w -l .

It works as expected, going through the given directory and formatting all files.

The problem is that the directory contains generated files, i.e. files with the first line // Code generated by XXX. DO NOT EDIT., and there is no flag to skip such files.

[adambabik]

I looked at the possible implementation and the gist of it is this:

diff --git a/internal/imports/imports.go b/internal/imports/imports.go
index 2215a1288..e6da58c75 100644
--- a/internal/imports/imports.go
+++ b/internal/imports/imports.go
@@ -42,6 +42,8 @@ type Options struct {
        TabWidth  int  // Tab width (8 if nil *Options provided)

        FormatOnly bool // Disable the insertion and deletion of imports
+
+       SkipGenerated bool // Skip processing of generated files
 }

 // Process implements golang.org/x/tools/imports.Process with explicit context in opt.Env.
@@ -59,7 +61,7 @@ func Process(filename string, src []byte, opt *Options) (formatted []byte, err e
                return nil, err
        }

-       if !opt.FormatOnly {
+       if !opt.FormatOnly && (!opt.SkipGenerated || opt.SkipGenerated && !ast.IsGenerated(file)) {
                if err := fixImports(fileSet, file, filename, opt.Env); err != nil {
                        return nil, err
                }

It's backwards compatible, and the overhead is only added when the flag is used.

[seankhliao]

Do we really need to expose this, rather than encourage tool authors to generate properly formatted files?

[seankhliao]

Or format it regardless, the warning is for humans to recognize that changes they make won't persist, but goimports can be mechanically applied every time there's a change

[dmitshur]

CC @golang/tools-team.

[xieyuschen]

I don't think goimports should check whether some files are generated or not as there is no unified marker.

But we may expose a new flag to allow users to filter folders. For example, in cmd/deadcode, there is a flag filter to specify a filter.

We can add a new flag filter in goimports, which will skip all files don't satisfy the filter.

[adonovan]

I don't think goimports should check whether some files are generated or not as there is no unified marker.

FYI: since 2021 there has been a standard form of comment to identify generated file; see https://pkg.go.dev/go/ast#IsGenerated.

But I agree with @seankhliao: code generators should generate properly formatted code. They can call goimports if they want, but they should not emit code with surplus or missing imports or non-canonical formatting.

I'm going to close this issue.

[cyrilc-pro]

@adonovan Although I agree with the conclusion that the generator tool should generate properly formatted code, I think it is not always possible: indeed, when you run goimports with the -local flag, there is not way for the generator tool to be aware of it and to generate the code accordingly.
Hence, I believe this feature request could/should be considered valid.

[adonovan]

Why would a code generator need to run goimports (as opposed to plain go fmt, or the equivalent effect in process)? Presumably it knows what imports are needed, and it really ought not to be a property of the local environment (as the effects of goimport are).