revert "imports: create named imports for name/path mismatches"
This reverts CL 145699 (commit 864069cfd1b106326ae556b3ca5c1d6b94645987)
Reason for revert: If the mismatch is bad enough that goimports can't find
it again, then the import is just removed, even if the user adds it back again.
Fixes #28645.
Change-Id: I6c8fc5434c2b56c73b260bcec2c12d8746fac4ad
Reviewed-on: https://go-review.googlesource.com/c/148237
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/imports/fix.go b/imports/fix.go
index ed702f4..187bc7d 100644
--- a/imports/fix.go
+++ b/imports/fix.go
@@ -224,7 +224,7 @@
if ipath == "C" {
break
}
- local := path.Base(ipath)
+ local := importPathToName(ipath, srcDir)
decls[local] = v
case *ast.SelectorExpr:
xident, ok := v.X.(*ast.Ident)
@@ -363,6 +363,98 @@
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() {
@@ -680,34 +772,14 @@
if pkg == nil {
continue
}
- // If the package name in the source doesn't match the import path,
+ // If the package name in the source doesn't match the import path's base,
// return true so the rewriter adds a name (import foo "github.com/bar/go-foo")
- // Module-style version suffixes are allowed.
- lastSeg := path.Base(pkg.importPath)
- if isVersionSuffix(lastSeg) {
- lastSeg = path.Base(path.Dir(pkg.importPath))
- }
- needsRename := lastSeg != pkgName
+ needsRename := path.Base(pkg.importPath) != 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 0acb478..7d71ecd 100644
--- a/imports/fix_test.go
+++ b/imports/fix_test.go
@@ -7,6 +7,7 @@
import (
"fmt"
"go/build"
+ "path/filepath"
"runtime"
"strings"
"sync"
@@ -1349,15 +1350,9 @@
`
testConfig{
- 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"},
- },
+ module: packagestest.Module{
+ Name: "mypkg.com/outpkg",
+ Files: fm{"toformat.go": input},
},
}.processTest(t, "mypkg.com/outpkg", "toformat.go", nil, nil, input)
}
@@ -1368,24 +1363,18 @@
// 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 "mypkg.com/mypkg.v1"
+var (
+ _ = fmt.Print
+ _ = mypkg.Foo
)
-
-var _, _ = fmt.Print, mypkg.Foo
`
-
testConfig{
gopathOnly: true,
module: packagestest.Module{
@@ -1395,7 +1384,7 @@
"toformat.go": input,
},
},
- }.processTest(t, "mypkg.com/outpkg", "toformat.go", nil, nil, want)
+ }.processTest(t, "mypkg.com/outpkg", "toformat.go", nil, nil, input)
}
func TestInternal(t *testing.T) {
@@ -1573,14 +1562,16 @@
}
// Tests that added imports are renamed when the import path's base doesn't
-// match its package name.
+// match its package name. For example, we want to generate:
+//
+// import cloudbilling "google.golang.org/api/cloudbilling/v1"
func TestRenameWhenPackageNameMismatch(t *testing.T) {
const input = `package main
const Y = bar.X`
const want = `package main
-import bar "foo.com/foo/bar/baz"
+import bar "foo.com/foo/bar/v1"
const Y = bar.X
`
@@ -1588,8 +1579,8 @@
module: packagestest.Module{
Name: "foo.com",
Files: fm{
- "foo/bar/baz/x.go": "package bar \n const X = 1",
- "test/t.go": input,
+ "foo/bar/v1/x.go": "package bar \n const X = 1",
+ "test/t.go": input,
},
},
}.processTest(t, "foo.com", "test/t.go", nil, nil, want)
@@ -1734,6 +1725,34 @@
}.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
