diff --git a/imports/fix.go b/imports/fix.go index 187bc7d3..ed702f45 100644 --- a/imports/fix.go +++ b/imports/fix.go @@ -224,7 +224,7 @@ func fixImports(fset *token.FileSet, f *ast.File, filename string) (added []stri if ipath == "C" { break } - local := importPathToName(ipath, srcDir) + local := path.Base(ipath) decls[local] = v case *ast.SelectorExpr: xident, ok := v.X.(*ast.Ident) @@ -363,98 +363,6 @@ func fixImports(fset *token.FileSet, f *ast.File, filename string) (added []stri return added, nil } -// importPathToName returns the package name for the given import path. -var importPathToName func(importPath, srcDir string) (packageName string) = importPathToNameGoPath - -// 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) { - 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) - } - } - } - return base -} - -// importPathToNameGoPath finds out the actual package name, as declared in its .go files. -// If there's a problem, it falls back to using importPathToNameBasic. -func importPathToNameGoPath(importPath, srcDir string) (packageName string) { - // Fast path for standard library without going to disk. - if pkg, ok := stdImportPackage[importPath]; ok { - return pkg - } - - pkgName, err := importPathToNameGoPathParse(importPath, srcDir) - if Debug { - log.Printf("importPathToNameGoPathParse(%q, srcDir=%q) = %q, %v", importPath, srcDir, pkgName, err) - } - if err == nil { - return pkgName - } - return importPathToNameBasic(importPath, srcDir) -} - -// importPathToNameGoPathParse is a faster version of build.Import if -// the only thing desired is the package name. It uses build.FindOnly -// to find the directory and then only parses one file in the package, -// trusting that the files in the directory are consistent. -func importPathToNameGoPathParse(importPath, srcDir string) (packageName string, err error) { - buildPkg, err := build.Import(importPath, srcDir, build.FindOnly) - if err != nil { - return "", err - } - d, err := os.Open(buildPkg.Dir) - if err != nil { - return "", err - } - names, err := d.Readdirnames(-1) - d.Close() - if err != nil { - return "", err - } - sort.Strings(names) // to have predictable behavior - var lastErr error - var nfile int - for _, name := range names { - if !strings.HasSuffix(name, ".go") { - continue - } - if strings.HasSuffix(name, "_test.go") { - continue - } - nfile++ - fullFile := filepath.Join(buildPkg.Dir, name) - - fset := token.NewFileSet() - f, err := parser.ParseFile(fset, fullFile, nil, parser.PackageClauseOnly) - if err != nil { - lastErr = err - continue - } - pkgName := f.Name.Name - if pkgName == "documentation" { - // Special case from go/build.ImportDir, not - // handled by ctx.MatchFile. - continue - } - if pkgName == "main" { - // Also skip package main, assuming it's a +build ignore generator or example. - // Since you can't import a package main anyway, there's no harm here. - continue - } - return pkgName, nil - } - if lastErr != nil { - return "", lastErr - } - return "", fmt.Errorf("no importable package found in %d Go files", nfile) -} - var stdImportPackage = map[string]string{} // "net/http" => "http" func init() { @@ -772,14 +680,34 @@ func findImportGoPath(ctx context.Context, pkgName string, symbols map[string]bo if pkg == nil { continue } - // If the package name in the source doesn't match the import path's base, + // If the package name in the source doesn't match the import path, // return true so the rewriter adds a name (import foo "github.com/bar/go-foo") - needsRename := path.Base(pkg.importPath) != pkgName + // Module-style version suffixes are allowed. + lastSeg := path.Base(pkg.importPath) + if isVersionSuffix(lastSeg) { + lastSeg = path.Base(path.Dir(pkg.importPath)) + } + needsRename := lastSeg != pkgName return pkg.importPathShort, needsRename, nil } return "", false, nil } +// isVersionSuffix reports whether the path segment seg is a semantic import +// versioning style major version suffix. +func isVersionSuffix(seg string) bool { + if seg == "" { + return false + } + if seg[0] != 'v' { + return false + } + if _, err := strconv.Atoi(seg[1:]); err != nil { + return false + } + return true +} + // 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. diff --git a/imports/fix_test.go b/imports/fix_test.go index 7d71ecd7..0acb4789 100644 --- a/imports/fix_test.go +++ b/imports/fix_test.go @@ -7,7 +7,6 @@ package imports import ( "fmt" "go/build" - "path/filepath" "runtime" "strings" "sync" @@ -1350,9 +1349,15 @@ var ( ` testConfig{ - module: packagestest.Module{ - Name: "mypkg.com/outpkg", - Files: fm{"toformat.go": input}, + modules: []packagestest.Module{ + { + Name: "mypkg.com/outpkg", + Files: fm{"toformat.go": input}, + }, + { + Name: "github.com/foo/v2", + Files: fm{"x.go": "package foo\n func Foo(){}\n"}, + }, }, }.processTest(t, "mypkg.com/outpkg", "toformat.go", nil, nil, input) } @@ -1363,18 +1368,24 @@ var ( // that the package name is "mypkg". func TestVendorPackage(t *testing.T) { const input = `package p +import ( + "fmt" + "mypkg.com/mypkg.v1" +) +var _, _ = fmt.Print, mypkg.Foo +` + + const want = `package p import ( "fmt" - "mypkg.com/mypkg.v1" + mypkg "mypkg.com/mypkg.v1" ) -var ( - _ = fmt.Print - _ = mypkg.Foo -) +var _, _ = fmt.Print, mypkg.Foo ` + testConfig{ gopathOnly: true, module: packagestest.Module{ @@ -1384,7 +1395,7 @@ var ( "toformat.go": input, }, }, - }.processTest(t, "mypkg.com/outpkg", "toformat.go", nil, nil, input) + }.processTest(t, "mypkg.com/outpkg", "toformat.go", nil, nil, want) } func TestInternal(t *testing.T) { @@ -1562,16 +1573,14 @@ func (t *goimportTest) process(module, file string, contents []byte, opts *Optio } // Tests that added imports are renamed when the import path's base doesn't -// match its package name. For example, we want to generate: -// -// import cloudbilling "google.golang.org/api/cloudbilling/v1" +// match its package name. func TestRenameWhenPackageNameMismatch(t *testing.T) { const input = `package main const Y = bar.X` const want = `package main -import bar "foo.com/foo/bar/v1" +import bar "foo.com/foo/bar/baz" const Y = bar.X ` @@ -1579,8 +1588,8 @@ const Y = bar.X module: packagestest.Module{ Name: "foo.com", Files: fm{ - "foo/bar/v1/x.go": "package bar \n const X = 1", - "test/t.go": input, + "foo/bar/baz/x.go": "package bar \n const X = 1", + "test/t.go": input, }, }, }.processTest(t, "foo.com", "test/t.go", nil, nil, want) @@ -1725,34 +1734,6 @@ const Y = foo.X }.processTest(t, "foo.com", "x/x.go", nil, nil, want) } -// Tests importPathToNameGoPathParse and in particular that it stops -// after finding the first non-documentation package name, not -// reporting an error on inconsistent package names (since it should -// never make it that far). -func TestImportPathToNameGoPathParse(t *testing.T) { - testConfig{ - gopathOnly: true, - module: packagestest.Module{ - Name: "example.net/pkg", - Files: fm{ - "doc.go": "package documentation\n", // ignored - "gen.go": "package main\n", // also ignored - "pkg.go": "package the_pkg_name_to_find\n and this syntax error is ignored because of parser.PackageClauseOnly", - "z.go": "package inconsistent\n", // inconsistent but ignored - }, - }, - }.test(t, func(t *goimportTest) { - got, err := importPathToNameGoPathParse("example.net/pkg", filepath.Join(t.gopath, "src", "other.net")) - if err != nil { - t.Fatal(err) - } - const want = "the_pkg_name_to_find" - if got != want { - t.Errorf("importPathToNameGoPathParse(..) = %q; want %q", got, want) - } - }) -} - func TestIgnoreConfiguration(t *testing.T) { const input = `package x