internal/imports: merge import declarations
When an import is added to the ast, the import declarations are merged
together into the first import declaration. Since this is a part of
the formatting functionality of goimports, do this during formatting
regardless.
The merging pass was added to astutil.AddNamedImport in order to address
issue golang/go#14075. This joined imports from other blocks into the first
import declaration, so that a single block of imports isn't split across
multiple blocks.
This functionality is more of a formatting change than a fix imports
change, in line with sorting the imports, which occurs even when
FormatOnly. The formatting was only applied when an import was added
(not renamed or deleted). This change makes formatting by goimports
more consistent across runs and is not dependent on the exact fixes
that need to be applied.
Change-Id: Icb90bf694ff35e2d6405a3d477cf82fcd3e697e0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/189941
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/imports/fix_test.go b/internal/imports/fix_test.go
index d15b6dd..fca6156 100644
--- a/internal/imports/fix_test.go
+++ b/internal/imports/fix_test.go
@@ -372,7 +372,37 @@
}
`,
},
+ // Merge import blocks, even when no additions are required.
+ {
+ name: "merge_import_blocks_no_fix",
+ in: `package foo
+import (
+ "fmt"
+)
+import "os"
+
+import (
+ "rsc.io/p"
+)
+
+var _, _ = os.Args, fmt.Println
+var _, _ = snappy.ErrCorrupt, p.P
+`,
+ out: `package foo
+
+import (
+ "fmt"
+ "os"
+
+ "github.com/golang/snappy"
+ "rsc.io/p"
+)
+
+var _, _ = os.Args, fmt.Println
+var _, _ = snappy.ErrCorrupt, p.P
+`,
+ },
// Delete existing empty import block
{
name: "delete_empty_import_block",
diff --git a/internal/imports/imports.go b/internal/imports/imports.go
index 82d02f0..9a132cd 100644
--- a/internal/imports/imports.go
+++ b/internal/imports/imports.go
@@ -136,6 +136,7 @@
}
func formatFile(fileSet *token.FileSet, file *ast.File, src []byte, adjust func(orig []byte, src []byte) []byte, opt *Options) ([]byte, error) {
+ mergeImports(opt.Env, fileSet, file)
sortImports(opt.Env, fileSet, file)
imps := astutil.Imports(fileSet, file)
var spacesBefore []string // import paths we need spaces before
diff --git a/internal/imports/sortimports.go b/internal/imports/sortimports.go
index 0a156fe..2262794 100644
--- a/internal/imports/sortimports.go
+++ b/internal/imports/sortimports.go
@@ -58,6 +58,53 @@
}
}
+// mergeImports merges all the import declarations into the first one.
+// Taken from golang.org/x/tools/ast/astutil.
+func mergeImports(env *ProcessEnv, fset *token.FileSet, f *ast.File) {
+ if len(f.Decls) <= 1 {
+ return
+ }
+
+ // Merge all the import declarations into the first one.
+ var first *ast.GenDecl
+ for i := 0; i < len(f.Decls); i++ {
+ decl := f.Decls[i]
+ gen, ok := decl.(*ast.GenDecl)
+ if !ok || gen.Tok != token.IMPORT || declImports(gen, "C") {
+ continue
+ }
+ if first == nil {
+ first = gen
+ continue // Don't touch the first one.
+ }
+ // We now know there is more than one package in this import
+ // declaration. Ensure that it ends up parenthesized.
+ first.Lparen = first.Pos()
+ // Move the imports of the other import declaration to the first one.
+ for _, spec := range gen.Specs {
+ spec.(*ast.ImportSpec).Path.ValuePos = first.Pos()
+ first.Specs = append(first.Specs, spec)
+ }
+ f.Decls = append(f.Decls[:i], f.Decls[i+1:]...)
+ i--
+ }
+}
+
+// declImports reports whether gen contains an import of path.
+// Taken from golang.org/x/tools/ast/astutil.
+func declImports(gen *ast.GenDecl, path string) bool {
+ if gen.Tok != token.IMPORT {
+ return false
+ }
+ for _, spec := range gen.Specs {
+ impspec := spec.(*ast.ImportSpec)
+ if importPath(impspec) == path {
+ return true
+ }
+ }
+ return false
+}
+
func importPath(s ast.Spec) string {
t, err := strconv.Unquote(s.(*ast.ImportSpec).Path.Value)
if err == nil {
