revert "imports: create named imports for name/path mismatches"

This reverts CL 145699 (commit 864069c)

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 <[email protected]>

Author: heschi

imports/fix.go

@@ -224,7 +224,7 @@ func fixImports(fset *token.FileSet, f *ast.File, filename string) (added []stri
 			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 @@ 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() {
@@ -680,34 +772,14 @@ 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,
+		// 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.

imports/fix_test.go

@@ -7,6 +7,7 @@ package imports
 import (
 	"fmt"
 	"go/build"
+	"path/filepath"
 	"runtime"
 	"strings"
 	"sync"
@@ -1349,15 +1350,9 @@ var (
 `
 
 	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 @@ 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 "mypkg.com/mypkg.v1"
+	"mypkg.com/mypkg.v1"
 )
 
-var _, _ = fmt.Print, mypkg.Foo
+var (
+	_ = fmt.Print
+	_ = mypkg.Foo
+)
 `
-
 	testConfig{
 		gopathOnly: true,
 		module: packagestest.Module{
@@ -1395,7 +1384,7 @@ var _, _ = fmt.Print, mypkg.Foo
 				"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,23 +1562,25 @@ 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.
+// 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
 `
 	testConfig{
 		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 @@ 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