diff --git a/go/ast/astutil/imports.go b/go/ast/astutil/imports.go index 4a77c782..83f196cd 100644 --- a/go/ast/astutil/imports.go +++ b/go/ast/astutil/imports.go @@ -49,6 +49,8 @@ func AddNamedImport(fset *token.FileSet, f *ast.File, name, ipath string) (added lastImport = -1 // index in f.Decls of the file's final import decl impDecl *ast.GenDecl // import decl containing the best match impIndex = -1 // spec index in impDecl containing the best match + + isThirdPartyPath = isThirdParty(ipath) ) for i, decl := range f.Decls { gen, ok := decl.(*ast.GenDecl) @@ -65,15 +67,27 @@ func AddNamedImport(fset *token.FileSet, f *ast.File, name, ipath string) (added impDecl = gen } - // Compute longest shared prefix with imports in this group. + // Compute longest shared prefix with imports in this group and find best + // matched import spec. + // 1. Always prefer import spec with longest shared prefix. + // 2. While match length is 0, + // - for stdlib package: prefer first import spec. + // - for third party package: prefer first third party import spec. + // We cannot use last import spec as best match for third party package + // because grouped imports are usually placed last by goimports -local + // flag. + // See issue #19190. + seenAnyThirdParty := false for j, spec := range gen.Specs { impspec := spec.(*ast.ImportSpec) - n := matchLen(importPath(impspec), ipath) - if n > bestMatch { + p := importPath(impspec) + n := matchLen(p, ipath) + if n > bestMatch || (bestMatch == 0 && !seenAnyThirdParty && isThirdPartyPath) { bestMatch = n impDecl = gen impIndex = j } + seenAnyThirdParty = seenAnyThirdParty || isThirdParty(p) } } } @@ -175,6 +189,12 @@ func AddNamedImport(fset *token.FileSet, f *ast.File, name, ipath string) (added return true } +func isThirdParty(importPath string) bool { + // Third party package import path usually contains "." (".com", ".org", ...) + // This logic is taken from golang.org/x/tools/imports package. + return strings.Contains(importPath, ".") +} + // DeleteImport deletes the import path from the file f, if present. func DeleteImport(fset *token.FileSet, f *ast.File, path string) (deleted bool) { return DeleteNamedImport(fset, f, "", path) diff --git a/go/ast/astutil/imports_test.go b/go/ast/astutil/imports_test.go index 21682674..8bc34808 100644 --- a/go/ast/astutil/imports_test.go +++ b/go/ast/astutil/imports_test.go @@ -140,6 +140,95 @@ import ( "d/f" ) +`, + }, + { + name: "issue #19190", + pkg: "x.org/y/z", + in: `package main + +// Comment +import "C" + +import ( + "bytes" + "os" + + "d.com/f" +) +`, + out: `package main + +// Comment +import "C" + +import ( + "bytes" + "os" + + "d.com/f" + "x.org/y/z" +) +`, + }, + { + name: "issue #19190 with existing grouped import packages", + pkg: "x.org/y/z", + in: `package main + +// Comment +import "C" + +import ( + "bytes" + "os" + + "c.com/f" + "d.com/f" + + "y.com/a" + "y.com/b" + "y.com/c" +) +`, + out: `package main + +// Comment +import "C" + +import ( + "bytes" + "os" + + "c.com/f" + "d.com/f" + "x.org/y/z" + + "y.com/a" + "y.com/b" + "y.com/c" +) +`, + }, + { + name: "issue #19190 - match score is still respected", + pkg: "y.org/c", + in: `package main + +import ( + "x.org/a" + + "y.org/b" +) +`, + out: `package main + +import ( + "x.org/a" + + "y.org/b" + "y.org/c" +) `, }, { diff --git a/imports/fix_test.go b/imports/fix_test.go index ffab3907..89c7e7bc 100644 --- a/imports/fix_test.go +++ b/imports/fix_test.go @@ -800,6 +800,70 @@ import ( ) var _ = fmt.Sprintf +`, + }, + + { + name: "issue #19190 1", + in: `package main + +import ( + "time" +) + +func main() { + _ = snappy.Encode + _ = p.P + _ = time.Parse +} +`, + out: `package main + +import ( + "time" + + "code.google.com/p/snappy-go/snappy" + "rsc.io/p" +) + +func main() { + _ = snappy.Encode + _ = p.P + _ = time.Parse +} +`, + }, + + { + name: "issue #19190 2", + in: `package main + +import ( + "time" + + "code.google.com/p/snappy-go/snappy" +) + +func main() { + _ = snappy.Encode + _ = p.P + _ = time.Parse +} +`, + out: `package main + +import ( + "time" + + "code.google.com/p/snappy-go/snappy" + "rsc.io/p" +) + +func main() { + _ = snappy.Encode + _ = p.P + _ = time.Parse +} `, }, }