diff --git a/imports/fix.go b/imports/fix.go index 73477e26635..badcbb60cd7 100644 --- a/imports/fix.go +++ b/imports/fix.go @@ -644,27 +644,10 @@ func findImportGoPath(pkgName string, symbols map[string]bool, filename string) } var candidates []*pkg - for _, pkg := range dirScan { - if !strings.Contains(lastTwoComponents(pkg.importPathShort), pkgName) { - // Speed optimization to minimize disk I/O: - // the last two components on disk must contain the - // package name somewhere. - // - // This permits mismatch naming like directory - // "go-foo" being package "foo", or "pkg.v3" being "pkg", - // or directory "google.golang.org/api/cloudbilling/v1" - // being package "cloudbilling", but doesn't - // permit a directory "foo" to be package - // "bar", which is strongly discouraged - // anyway. There's no reason goimports needs - // to be slow just to accomodate that. - continue + if pkgIsCandidate(filename, pkgName, pkg) { + candidates = append(candidates, pkg) } - if !canUse(filename, pkg.dir) { - continue - } - candidates = append(candidates, pkg) } sort.Sort(byImportPathShortLength(candidates)) @@ -724,6 +707,76 @@ func findImportGoPath(pkgName string, symbols map[string]bool, filename string) return "", false, nil } +// pkgIsCandidate reports whether pkg is a candidate for satisfying the +// finding which package pkgIdent in the file named by filename is trying +// to refer to. +// +// This check is purely lexical and is meant to be as fast as possible +// because it's run over all $GOPATH directories to filter out poor +// candidates in order to limit the CPU and I/O later parsing the +// exports in candidate packages. +// +// filename is the file being formatted. +// pkgIdent is the package being searched for, like "client" (if +// searching for "client.New") +func pkgIsCandidate(filename, pkgIdent string, pkg *pkg) bool { + // Check "internal" and "vendor" visibility: + if !canUse(filename, pkg.dir) { + return false + } + + // Speed optimization to minimize disk I/O: + // the last two components on disk must contain the + // package name somewhere. + // + // This permits mismatch naming like directory + // "go-foo" being package "foo", or "pkg.v3" being "pkg", + // or directory "google.golang.org/api/cloudbilling/v1" + // being package "cloudbilling", but doesn't + // permit a directory "foo" to be package + // "bar", which is strongly discouraged + // anyway. There's no reason goimports needs + // to be slow just to accomodate that. + lastTwo := lastTwoComponents(pkg.importPathShort) + if strings.Contains(lastTwo, pkgIdent) { + return true + } + if hasHyphenOrUpperASCII(lastTwo) && !hasHyphenOrUpperASCII(pkgIdent) { + lastTwo = lowerASCIIAndRemoveHyphen(lastTwo) + if strings.Contains(lastTwo, pkgIdent) { + return true + } + } + + return false +} + +func hasHyphenOrUpperASCII(s string) bool { + for i := 0; i < len(s); i++ { + b := s[i] + if b == '-' || ('A' <= b && b <= 'Z') { + return true + } + } + return false +} + +func lowerASCIIAndRemoveHyphen(s string) (ret string) { + buf := make([]byte, 0, len(s)) + for i := 0; i < len(s); i++ { + b := s[i] + switch { + case b == '-': + continue + case 'A' <= b && b <= 'Z': + buf = append(buf, b+('a'-'A')) + default: + buf = append(buf, b) + } + } + return string(buf) +} + // canUse reports whether the package in dir is usable from filename, // respecting the Go "internal" and "vendor" visibility rules. func canUse(filename, dir string) bool { diff --git a/imports/fix_test.go b/imports/fix_test.go index 73e5de75119..1d72136de12 100644 --- a/imports/fix_test.go +++ b/imports/fix_test.go @@ -1333,3 +1333,141 @@ func strSet(ss []string) map[string]bool { } return m } + +func TestPkgIsCandidate(t *testing.T) { + tests := [...]struct { + filename string + pkgIdent string + pkg *pkg + want bool + }{ + // normal match + 0: { + filename: "/gopath/src/my/pkg/pkg.go", + pkgIdent: "client", + pkg: &pkg{ + dir: "/gopath/src/client", + importPath: "client", + importPathShort: "client", + }, + want: true, + }, + // not a match + 1: { + filename: "/gopath/src/my/pkg/pkg.go", + pkgIdent: "zzz", + pkg: &pkg{ + dir: "/gopath/src/client", + importPath: "client", + importPathShort: "client", + }, + want: false, + }, + // would be a match, but "client" appears too deep. + 2: { + filename: "/gopath/src/my/pkg/pkg.go", + pkgIdent: "client", + pkg: &pkg{ + dir: "/gopath/src/client/foo/foo/foo", + importPath: "client/foo/foo", + importPathShort: "client/foo/foo", + }, + want: false, + }, + // not an exact match, but substring is good enough. + 3: { + filename: "/gopath/src/my/pkg/pkg.go", + pkgIdent: "client", + pkg: &pkg{ + dir: "/gopath/src/foo/go-client", + importPath: "foo/go-client", + importPathShort: "foo/go-client", + }, + want: true, + }, + // "internal" package, and not visible + 4: { + filename: "/gopath/src/my/pkg/pkg.go", + pkgIdent: "client", + pkg: &pkg{ + dir: "/gopath/src/foo/internal/client", + importPath: "foo/internal/client", + importPathShort: "foo/internal/client", + }, + want: false, + }, + // "internal" package but visible + 5: { + filename: "/gopath/src/foo/bar.go", + pkgIdent: "client", + pkg: &pkg{ + dir: "/gopath/src/foo/internal/client", + importPath: "foo/internal/client", + importPathShort: "foo/internal/client", + }, + want: true, + }, + // "vendor" package not visible + 6: { + filename: "/gopath/src/foo/bar.go", + pkgIdent: "client", + pkg: &pkg{ + dir: "/gopath/src/other/vendor/client", + importPath: "other/vendor/client", + importPathShort: "client", + }, + want: false, + }, + // "vendor" package, visible + 7: { + filename: "/gopath/src/foo/bar.go", + pkgIdent: "client", + pkg: &pkg{ + dir: "/gopath/src/foo/vendor/client", + importPath: "other/foo/client", + importPathShort: "client", + }, + want: true, + }, + // Ignore hyphens. + 8: { + filename: "/gopath/src/foo/bar.go", + pkgIdent: "socketio", + pkg: &pkg{ + dir: "/gopath/src/foo/socket-io", + importPath: "foo/socket-io", + importPathShort: "foo/socket-io", + }, + want: true, + }, + // Ignore case. + 9: { + filename: "/gopath/src/foo/bar.go", + pkgIdent: "fooprod", + pkg: &pkg{ + dir: "/gopath/src/foo/FooPROD", + importPath: "foo/FooPROD", + importPathShort: "foo/FooPROD", + }, + want: true, + }, + // Ignoring both hyphens and case together. + 10: { + filename: "/gopath/src/foo/bar.go", + pkgIdent: "fooprod", + pkg: &pkg{ + dir: "/gopath/src/foo/Foo-PROD", + importPath: "foo/Foo-PROD", + importPathShort: "foo/Foo-PROD", + }, + want: true, + }, + } + for i, tt := range tests { + got := pkgIsCandidate(tt.filename, tt.pkgIdent, tt.pkg) + if got != tt.want { + t.Errorf("test %d. pkgIsCandidate(%q, %q, %+v) = %v; want %v", + i, tt.filename, tt.pkgIdent, *tt.pkg, got, tt.want) + } + } +}