Ignore extra packages in Gazelle (#386)

If Gazelle finds .go files in a directory with multiple distinct
package declarations, it will now only generate rules to build .go
files where the package declaration matches the directory name. If
there are multiple packages, and none match the directory name,
Gazelle will report a build.MultiplePackageError, as bove. If is a
single package that doesn't match the directory name, Gazelle will
still generate rules for it.

This change helps address #339. When we are no longer applying build
constraints by default, multiple packages are much more common; many
files are hidden with "// +build ignore".

Author: jayconrod

go/tools/gazelle/packages/walk.go

@@ -17,19 +17,33 @@ package packages
 
 import (
 	"go/build"
+	"go/parser"
+	"go/token"
+	"io/ioutil"
+	"log"
 	"os"
+	"path"
 	"path/filepath"
+	"sort"
+	"strings"
 )
 
 // A WalkFunc is a callback called by Walk for each package.
 type WalkFunc func(pkg *build.Package) error
 
-// Walk walks through Go packages under the given dir.
+// Walk walks through directories under "root".
 // It calls back "f" for each package.
 //
 // It is similar to "golang.org/x/tools/go/buildutil".ForEachPackage, but
 // it does not assume the standard Go tree because Bazel rules_go uses
 // go_prefix instead of the standard tree.
+//
+// If a directory contains no buildable Go code, "f" is not called. If a
+// directory contains one package with any name, "f" will be called with that
+// package. If a directory contains multiple packages and one of the package
+// names matches the directory name, "f" will be called on that package and the
+// other packages will be silently ignored. If none of the package names match
+// the directory name, a *build.MultiplePackageError error is returned.
 func Walk(bctx build.Context, root string, f WalkFunc) error {
 	return filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
 		if err != nil {
@@ -42,7 +56,7 @@ func Walk(bctx build.Context, root string, f WalkFunc) error {
 			return filepath.SkipDir
 		}
 
-		pkg, err := bctx.ImportDir(path, build.ImportComment)
+		pkg, err := findPackage(bctx, path)
 		if err != nil {
 			if _, ok := err.(*build.NoGoError); ok {
 				return nil
@@ -52,3 +66,106 @@ func Walk(bctx build.Context, root string, f WalkFunc) error {
 		return f(pkg)
 	})
 }
+
+func findPackage(bctx build.Context, dir string) (*build.Package, error) {
+	packageGoFiles, otherFiles, err := findPackageFiles(dir)
+	if err != nil {
+		return nil, err
+	}
+
+	packageName, err := selectPackageName(packageGoFiles, dir)
+	if err != nil {
+		return nil, err
+	}
+
+	var files []os.FileInfo
+	files = append(files, packageGoFiles[packageName]...)
+	files = append(files, otherFiles...)
+	sort.Slice(files, func(i, j int) bool {
+		return files[i].Name() < files[j].Name()
+	})
+	bctx.ReadDir = func(dir string) ([]os.FileInfo, error) {
+		return files, nil
+	}
+	return bctx.ImportDir(dir, build.ImportComment)
+}
+
+func findPackageFiles(dir string) (packageGoFiles map[string][]os.FileInfo, otherFiles []os.FileInfo, err error) {
+	files, err := ioutil.ReadDir(dir)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	packageGoFiles = make(map[string][]os.FileInfo)
+	for _, file := range files {
+		if file.IsDir() {
+			continue
+		}
+
+		name := file.Name()
+		filename := filepath.Join(dir, name)
+		ext := path.Ext(name)
+		isGo := ext == ".go"
+
+		if !isGo {
+			otherFiles = append(otherFiles, file)
+			continue
+		}
+		fset := token.NewFileSet()
+		ast, err := parser.ParseFile(fset, filename, nil, parser.PackageClauseOnly)
+		if err != nil {
+			log.Printf("%s: error parsing package clause: %v", filename, err)
+			continue
+		}
+
+		packageName := ast.Name.Name
+		if packageName == "documentation" {
+			// go/build ignores this package.
+			continue
+		}
+		if strings.HasSuffix(packageName, "_test") {
+			packageName = packageName[:len(packageName)-len("_test")]
+		}
+		packageGoFiles[packageName] = append(packageGoFiles[packageName], file)
+	}
+	return packageGoFiles, otherFiles, nil
+}
+
+func defaultPackageName(dir string) string {
+	pname := filepath.Base(dir)
+	if pname == "." || pname == "/" {
+		// We'll only use this name at the root of the filesystem.
+		return "unnamed"
+	}
+	return pname
+}
+
+func selectPackageName(packageGoFiles map[string][]os.FileInfo, dir string) (string, error) {
+	if len(packageGoFiles) == 0 {
+		return "", &build.NoGoError{Dir: dir}
+	}
+
+	if len(packageGoFiles) == 1 {
+		var packageName string
+		for name, _ := range packageGoFiles {
+			packageName = name
+		}
+		return packageName, nil
+	}
+
+	defaultName := defaultPackageName(dir)
+	if _, ok := packageGoFiles[defaultName]; ok {
+		return defaultName, nil
+	}
+
+	err := &build.MultiplePackageError{Dir: dir}
+	for name, files := range packageGoFiles {
+		// Add the first file for each package for the error message.
+		// Error() method expects these lists to be the same length. File
+		// lists must be non-empty. These lists are only created by
+		// findPackageFiles for packages with .go files present.
+		err.Packages = append(err.Packages, name)
+		err.Files = append(err.Files, files[0].Name())
+	}
+	return "", err
+}

go/tools/gazelle/packages/walk_test.go

@@ -21,7 +21,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
-	"sort"
+	"strings"
 	"testing"
 
 	"github.com/bazelbuild/rules_go/go/tools/gazelle/packages"
@@ -31,78 +31,159 @@ func tempDir() (string, error) {
 	return ioutil.TempDir(os.Getenv("TEST_TMPDIR"), "walk_test")
 }
 
-func TestWalkSimple(t *testing.T) {
-	dir, err := tempDir()
+type fileSpec struct {
+	path, content string
+}
+
+func checkFiles(t *testing.T, files []fileSpec, want []*build.Package) {
+	dir, err := createFiles(files)
 	if err != nil {
-		t.Fatalf("tempDir() failed with %v; want success", err)
+		t.Fatalf("createFiles() failed with %v; want success", err)
 	}
 	defer os.RemoveAll(dir)
 
-	fname := filepath.Join(dir, "lib.go")
-	if err := ioutil.WriteFile(fname, []byte("package lib"), 0600); err != nil {
-		t.Fatalf(`ioutil.WriteFile(%q, "package lib", 0600) failed with %v; want success`, fname, err)
+	got, err := walkPackages(dir)
+	if err != nil {
+		t.Errorf("walkPackages(%q) failed with %v; want success", dir, err)
 	}
+	checkPackages(t, got, want)
+}
 
-	var n int
-	err = packages.Walk(build.Default, dir, func(pkg *build.Package) error {
-		if got, want := pkg.Name, "lib"; got != want {
-			t.Errorf("pkg.Name = %q; want %q", got, want)
+func createFiles(files []fileSpec) (string, error) {
+	dir, err := tempDir()
+	if err != nil {
+		return "", err
+	}
+	for _, f := range files {
+		path := filepath.Join(dir, f.path)
+		if strings.HasSuffix(f.path, "/") {
+			if err := os.MkdirAll(path, 0700); err != nil {
+				return dir, err
+			}
+			continue
+		}
+		if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil {
+			return "", err
+		}
+		if err := ioutil.WriteFile(path, []byte(f.content), 0600); err != nil {
+			return "", err
 		}
-		n++
+	}
+	return dir, nil
+}
+
+func walkPackages(root string) ([]*build.Package, error) {
+	var pkgs []*build.Package
+	err := packages.Walk(build.Default, root, func(pkg *build.Package) error {
+		pkgs = append(pkgs, pkg)
 		return nil
 	})
 	if err != nil {
-		t.Errorf("packages.Walk(build.Default, %q, func) failed with %v; want success", dir, err)
+		return nil, err
 	}
-	if got, want := n, 1; got != want {
-		t.Errorf("n = %d; want %d", got, want)
+	return pkgs, nil
+}
+
+func checkPackages(t *testing.T, got []*build.Package, want []*build.Package) {
+	if len(got) != len(want) {
+		t.Fatalf("got %d packages; want %d", len(got), len(want))
+	}
+	for i := 0; i < len(got); i++ {
+		checkPackage(t, got[i], want[i])
 	}
 }
 
-func TestWalkNested(t *testing.T) {
-	dir, err := tempDir()
-	if err != nil {
-		t.Fatalf("tempDir() failed with %v; want success", err)
+func checkPackage(t *testing.T, got, want *build.Package) {
+	if !reflect.DeepEqual(got.Name, want.Name) {
+		t.Errorf("got package name %q; want %q", got.Name, want.Name)
 	}
-	defer os.RemoveAll(dir)
+	if !reflect.DeepEqual(got.GoFiles, want.GoFiles) {
+		t.Errorf("in package %q, got GoFiles %v; want %v", got.Name, got.GoFiles, want.GoFiles)
+	}
+	if !reflect.DeepEqual(got.CgoFiles, want.CgoFiles) {
+		t.Errorf("in package %q, got CgoFiles %v; want %v", got.Name, got.CgoFiles, want.CgoFiles)
+	}
+	if !reflect.DeepEqual(got.CFiles, want.CFiles) {
+		t.Errorf("in package %q, got CFiles %v; want %v", got.Name, got.CFiles, want.CFiles)
+	}
+	if !reflect.DeepEqual(got.TestGoFiles, want.TestGoFiles) {
+		t.Errorf("in package %q, got TestGoFiles %v; want %v", got.Name, got.TestGoFiles, want.TestGoFiles)
+	}
+	if !reflect.DeepEqual(got.XTestGoFiles, want.XTestGoFiles) {
+		t.Errorf("in package %q, got XTestGoFiles %v; want %v", got.Name, got.XTestGoFiles, want.XTestGoFiles)
+	}
+}
 
-	for _, p := range []struct {
-		path, content string
-	}{
+func TestWalkEmpty(t *testing.T) {
+	files := []fileSpec{
+		{path: "a/foo.c"},
+		{path: "b/"},
+	}
+	want := []*build.Package{}
+	checkFiles(t, files, want)
+}
+
+func TestWalkSimple(t *testing.T) {
+	files := []fileSpec{{path: "lib.go", content: "package lib"}}
+	want := []*build.Package{
+		{
+			Name:    "lib",
+			GoFiles: []string{"lib.go"},
+		},
+	}
+	checkFiles(t, files, want)
+}
+
+func TestWalkNested(t *testing.T) {
+	files := []fileSpec{
 		{path: "a/foo.go", content: "package a"},
 		{path: "b/c/bar.go", content: "package c"},
 		{path: "b/d/baz.go", content: "package main"},
-	} {
-		path := filepath.Join(dir, p.path)
-		if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil {
-			t.Fatalf("os.MkdirAll(%q, 0700) failed with %v; want success", filepath.Dir(path), err)
-		}
-		if err := ioutil.WriteFile(path, []byte(p.content), 0600); err != nil {
-			t.Fatalf("ioutil.WriteFile(%q, %q, 0600) failed with %v; want success", path, p.content, err)
-		}
 	}
+	want := []*build.Package{
+		{
+			Name:    "a",
+			GoFiles: []string{"foo.go"},
+		},
+		{
+			Name:    "c",
+			GoFiles: []string{"bar.go"},
+		},
+		{
+			Name:    "main",
+			GoFiles: []string{"baz.go"},
+		},
+	}
+	checkFiles(t, files, want)
+}
 
-	var dirs, pkgs []string
-	err = packages.Walk(build.Default, dir, func(pkg *build.Package) error {
-		rel, err := filepath.Rel(dir, pkg.Dir)
-		if err != nil {
-			t.Errorf("filepath.Rel(%q, %q) failed with %v; want success", dir, pkg.Dir, err)
-			return err
-		}
-		dirs = append(dirs, filepath.ToSlash(rel))
-		pkgs = append(pkgs, pkg.Name)
-		return nil
-	})
-	if err != nil {
-		t.Errorf("packages.Walk(build.Default, %q, func) failed with %v; want success", dir, err)
+func TestMultiplePackagesWithDefault(t *testing.T) {
+	files := []fileSpec{
+		{path: "a/a.go", content: "package a"},
+		{path: "a/b.go", content: "package b"},
 	}
+	want := []*build.Package{
+		{
+			Name:    "a",
+			GoFiles: []string{"a.go"},
+		},
+	}
+	checkFiles(t, files, want)
+}
 
-	sort.Strings(dirs)
-	if got, want := dirs, []string{"a", "b/c", "b/d"}; !reflect.DeepEqual(got, want) {
-		t.Errorf("pkgs = %q; want %q", got, want)
+func TestMultiplePackagesWithoutDefault(t *testing.T) {
+	files := []fileSpec{
+		{path: "a/b.go", content: "package b"},
+		{path: "a/c.go", content: "package c"},
+	}
+	dir, err := createFiles(files)
+	if err != nil {
+		t.Fatalf("createFiles() failed with %v; want success", err)
 	}
-	sort.Strings(pkgs)
-	if got, want := pkgs, []string{"a", "c", "main"}; !reflect.DeepEqual(got, want) {
-		t.Errorf("pkgs = %q; want %q", got, want)
+	defer os.RemoveAll(dir)
+
+	_, err = walkPackages(dir)
+	if _, ok := err.(*build.MultiplePackageError); !ok {
+		t.Errorf("got %v; want MultiplePackageError", err)
 	}
 }