imports: do less I/O finding the package name of an import

When goimports was run on a file like:

    package main

    import (
        "example.net/foo"
        "example.net/bar"
    )

    var _, _ = foo.Foo, bar.Bar

... even though there looks to be no work to do, it still needs to
verify that "example.net/foo" is really package "foo" (even though it
looks like it) and "example.net/bar" is really package
"bar". (Packages in the standard library are hard-coded since the
previous commit and not verified for consistency since they're always consistent)

To do that verification for non-std packages, go/build.Import was
being used before, but Import reads all files in the directory to make
sure they're consistent. That's unnecessary. Instead, stop after the
first file. If example.net/foo has foo.go with "package foo" and
just_kidding.go with "package other", we never read that far to find
the inconsistency. Oh well. Prefer speed.

Updates golang/go#16367

Change-Id: I9fc3fefbee0e8a6bc287bf2a565257fb9523fd5c
Reviewed-on: https://go-review.googlesource.com/24948
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>

imports/fix.go

@@ -187,29 +187,72 @@ func importPathToNameGoPath(importPath, srcDir string) (packageName string) {
 		return pkg
 	}
 
-	// TODO(bradfitz): build.Import does too much work, and
+	pkgName, err := importPathToNameGoPathParse(importPath, srcDir)
-	// doesn't cache either in-process or long-lived (anything
-	// found in the first pass from explicit imports aren't used
-	// again when scanning all directories). Also, it opens+reads
-	// *_test.go files too. As a baby step, use a cheaper
-	// mechanism to start (build.FindOnly), and then just read a
-	// single file (like parser.ParseFile in loadExportsGoPath) and
-	// skip only "documentation" but otherwise trust the first matching
-	// file's package name.
 	if Debug {
-		log.Printf("build.Import(ip=%q, srcDir=%q) ...", importPath, srcDir)
+		log.Printf("importPathToNameGoPathParse(%q, srcDir=%q) = %q, %v", importPath, srcDir, pkgName, err)
 	}
-	if buildPkg, err := build.Import(importPath, srcDir, 0); err == nil {
+	if err == nil {
-		if Debug {
+		return pkgName
-			log.Printf("build.Import(%q, srcDir=%q) = %q", importPath, srcDir, buildPkg.Name)
-		}
-		return buildPkg.Name
-	} else {
-		if Debug {
-			log.Printf("build.Import(%q, srcDir=%q) error: %v", importPath, srcDir, err)
-		}
-		return importPathToNameBasic(importPath, srcDir)
 	}
+	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"

imports/fix_test.go

@@ -1280,6 +1280,30 @@ func TestIgnoreDocumentationPackage(t *testing.T) {
 	})
 }
 
+// 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{
+		gopathFiles: map[string]string{
+			"example.net/pkg/doc.go": "package documentation\n", // ignored
+			"example.net/pkg/gen.go": "package main\n",          // also ignored
+			"example.net/pkg/pkg.go": "package the_pkg_name_to_find\n  and this syntax error is ignored because of parser.PackageClauseOnly",
+			"example.net/pkg/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 strSet(ss []string) map[string]bool {
 	m := make(map[string]bool)
 	for _, s := range ss {