Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/imports: add FormatOnly option #14500

Closed
moorereason opened this issue Feb 25, 2016 · 2 comments
Closed

x/tools/imports: add FormatOnly option #14500

moorereason opened this issue Feb 25, 2016 · 2 comments
Milestone

Comments

@moorereason
Copy link

Proposal

Add a FormatOnly field to the imports.Options type. Setting FormatOnly to true will bypass the "fix" functionality so that no imports will be inserted or deleted. Default would be false and backward compatible.

Thanks for your consideration.

Rationale

I'm working on a Go code generator and would like for the generated import stanza to be formatted according to the standard goimports style. Some projects enforce goimports formatting such that improperly formatted files will cause a CI build failure.

However, we don't want the import-insertion logic of the imports package to execute. If our generator creates code with missing imports, we consider that a bug in the generator and don't want to mask the bugs by using imports.Process. We've also seen goimports insert the wrong package and want to avoid that scenario, too.

This proposal would also disable the deletion of unused imports, but that functionality is easily replicated with astutil.UsesImport. In our project, we already delete unused imports manually and would continue doing so prior to using imports.Process.

Alternatives

Add an option to go/format

An option could to be added to go/format to enable goimports formatting, but I'm doubtful that the Go team would care to alter go/format for this use. go/format does one thing (with no options): canonical gofmt formatting. Unless we're willing to change the canon (blasphemy!), this is a dead end.

Copy code

We looked at copying the necessary code out of x/tools/imports, but we didn't like that option. It ended up being more code than we cared to copy and added a lot of complexity that we felt was unnecessary.

Proof of Concept

diff --git a/imports/imports.go b/imports/imports.go
index e30946b..5342801 100644
--- a/imports/imports.go
+++ b/imports/imports.go
@@ -31,6 +31,8 @@ type Options struct {
        Comments  bool // Print comments (true if nil *Options provided)
        TabIndent bool // Use tabs for indent (true if nil *Options provided)
        TabWidth  int  // Tab width (8 if nil *Options provided)
+
+       FormatOnly bool // Disable the insertion and deletion of imports
 }

 // Process formats and adjusts imports for the provided file.
@@ -46,9 +48,11 @@ func Process(filename string, src []byte, opt *Options) ([]byte, error) {
                return nil, err
        }

-       _, err = fixImports(fileSet, file)
-       if err != nil {
-               return nil, err
+       if !opt.FormatOnly {
+               _, err = fixImports(fileSet, file)
+               if err != nil {
+                       return nil, err
+               }
        }

        sortImports(fileSet, file)
@bradfitz bradfitz self-assigned this Feb 25, 2016
@bradfitz bradfitz added this to the Go1.7 milestone Feb 25, 2016
@bradfitz
Copy link
Contributor

I created this mess (goimports having an opinion on import blocks, where gofmt doesn't) so I'll take this bug. No promises, but I promise to at least think about it.

@bketelsen
Copy link
Contributor

Yay!

@golang golang locked and limited conversation to collaborators Apr 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants