diff --git a/imports/fix.go b/imports/fix.go index cb2d3eb01d..4c0339305d 100644 --- a/imports/fix.go +++ b/imports/fix.go @@ -23,6 +23,8 @@ import ( "strings" "sync" "time" + "unicode" + "unicode/utf8" "golang.org/x/tools/go/ast/astutil" "golang.org/x/tools/go/packages" @@ -289,7 +291,7 @@ func (p *pass) importIdentifier(imp *importInfo) string { if known != nil && known.name != "" { return known.name } - return importPathToNameBasic(imp.importPath, p.srcDir) + return importPathToAssumedName(imp.importPath) } // load reads in everything necessary to run a pass, and reports whether the @@ -389,7 +391,7 @@ func (p *pass) fix() bool { } path := strings.Trim(imp.Path.Value, `""`) ident := p.importIdentifier(&importInfo{importPath: path}) - if ident != importPathToNameBasic(path, p.srcDir) { + if ident != importPathToAssumedName(path) { imp.Name = &ast.Ident{Name: ident, NamePos: imp.Pos()} } } @@ -648,7 +650,7 @@ func (r *goPackagesResolver) loadPackageNames(importPaths []string, srcDir strin if _, ok := names[path]; ok { continue } - names[path] = importPathToNameBasic(path, srcDir) + names[path] = importPathToAssumedName(path) } return names, nil @@ -741,18 +743,36 @@ func addExternalCandidates(pass *pass, refs references, filename string) error { return firstErr } -// importPathToNameBasic assumes the package name is the base of import path, -// except that if the path ends in foo/vN, it assumes the package name is foo. -func importPathToNameBasic(importPath, srcDir string) (packageName string) { +// notIdentifier reports whether ch is an invalid identifier character. +func notIdentifier(ch rune) bool { + return !('a' <= ch && ch <= 'z' || 'A' <= ch && ch <= 'Z' || + '0' <= ch && ch <= '9' || + ch == '_' || + ch >= utf8.RuneSelf && (unicode.IsLetter(ch) || unicode.IsDigit(ch))) +} + +// importPathToAssumedName returns the assumed package name of an import path. +// It does this using only string parsing of the import path. +// It picks the last element of the path that does not look like a major +// version, and then picks the valid identifier off the start of that element. +// It is used to determine if a local rename should be added to an import for +// clarity. +// This function could be moved to a standard package and exported if we want +// for use in other tools. +func importPathToAssumedName(importPath string) string { base := path.Base(importPath) if strings.HasPrefix(base, "v") { if _, err := strconv.Atoi(base[1:]); err == nil { dir := path.Dir(importPath) if dir != "." { - return path.Base(dir) + base = path.Base(dir) } } } + base = strings.TrimPrefix(base, "go-") + if i := strings.IndexFunc(base, notIdentifier); i >= 0 { + base = base[:i] + } return base } diff --git a/imports/fix_test.go b/imports/fix_test.go index d475e714d8..d34fef66e4 100644 --- a/imports/fix_test.go +++ b/imports/fix_test.go @@ -1373,13 +1373,13 @@ var ( // Test for correctly identifying the name of a vendored package when it // differs from its directory name. In this test, the import line -// "mypkg.com/mypkg.v1" would be removed if goimports wasn't able to detect +// "mypkg.com/mypkg_v1" would be removed if goimports wasn't able to detect // that the package name is "mypkg". func TestVendorPackage(t *testing.T) { const input = `package p import ( "fmt" - "mypkg.com/mypkg.v1" + "mypkg.com/mypkg_v1" ) var _, _ = fmt.Print, mypkg.Foo ` @@ -1389,7 +1389,7 @@ var _, _ = fmt.Print, mypkg.Foo import ( "fmt" - mypkg "mypkg.com/mypkg.v1" + mypkg "mypkg.com/mypkg_v1" ) var _, _ = fmt.Print, mypkg.Foo @@ -1400,7 +1400,7 @@ var _, _ = fmt.Print, mypkg.Foo module: packagestest.Module{ Name: "mypkg.com/outpkg", Files: fm{ - "vendor/mypkg.com/mypkg.v1/f.go": "package mypkg\nvar Foo = 123\n", + "vendor/mypkg.com/mypkg_v1/f.go": "package mypkg\nvar Foo = 123\n", "toformat.go": input, }, }, @@ -1626,28 +1626,43 @@ func TestAddNameToMismatchedImport(t *testing.T) { const input = `package main import ( +"foo.com/a.thing" "foo.com/surprise" "foo.com/v1" +"foo.com/other/v2" +"foo.com/other/v3" +"foo.com/go-thing" +"foo.com/go-wrong" ) -var _, _ = bar.X, v1.Y` +var _ = []interface{}{bar.X, v1.Y, a.A, v2.V2, other.V3, thing.Thing, gow.Wrong}` const want = `package main import ( + "foo.com/a.thing" + "foo.com/go-thing" + gow "foo.com/go-wrong" + v2 "foo.com/other/v2" + "foo.com/other/v3" bar "foo.com/surprise" v1 "foo.com/v1" ) -var _, _ = bar.X, v1.Y +var _ = []interface{}{bar.X, v1.Y, a.A, v2.V2, other.V3, thing.Thing, gow.Wrong} ` testConfig{ module: packagestest.Module{ Name: "foo.com", Files: fm{ + "a.thing/a.go": "package a \n const A = 1", "surprise/x.go": "package bar \n const X = 1", "v1/x.go": "package v1 \n const Y = 1", + "other/v2/y.go": "package v2 \n const V2 = 1", + "other/v3/z.go": "package other \n const V3 = 1", + "go-thing/b.go": "package thing \n const Thing = 1", + "go-wrong/b.go": "package gow \n const Wrong = 1", "test/t.go": input, }, },