imports: drop anything after a non identifier rune in package names
A package name cannot contain a '.' anyway, so this is a mostly likely
internal package name in the presence of a '.' in the import path.
This stops goimports from adding a local alias in places where it is not
needed, for instance in gopkg.in conventions.
Fixes golang/go#29556
Change-Id: I0ab11f2852d7f1dae14457995692760077201c8e
Reviewed-on: https://go-review.googlesource.com/c/157357
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/imports/fix.go b/imports/fix.go
index cb2d3eb..4c03393 100644
--- a/imports/fix.go
+++ b/imports/fix.go
@@ -23,6 +23,8 @@
"strings"
"sync"
"time"
+ "unicode"
+ "unicode/utf8"
"golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/go/packages"
@@ -289,7 +291,7 @@
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 @@
}
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 @@
if _, ok := names[path]; ok {
continue
}
- names[path] = importPathToNameBasic(path, srcDir)
+ names[path] = importPathToAssumedName(path)
}
return names, nil
@@ -741,18 +743,36 @@
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 d475e71..d34fef6 100644
--- a/imports/fix_test.go
+++ b/imports/fix_test.go
@@ -1373,13 +1373,13 @@
// 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 @@
import (
"fmt"
- mypkg "mypkg.com/mypkg.v1"
+ mypkg "mypkg.com/mypkg_v1"
)
var _, _ = fmt.Print, mypkg.Foo
@@ -1400,7 +1400,7 @@
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 @@
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,
},
},
