go/packages: add Option.Tests bool, which affects pattern expansion

In the go build system, test packages and executables do not have a
name distinct from the package under test; they are implied, so
"go test fmt" means build those packages but "go build fmt" does not.

This change adds a Tests boolean option to indicate that implied
tests are desired during pattern expansion.
It has no effect on build systems that have explicit names
for tests, such as Blaze/Bazel.

The gopackages diagnostic tool now has a -test flag.

Change-Id: I424f343958c4286539e518d5f30067da19a57f3b
Reviewed-on: https://go-review.googlesource.com/123775
Reviewed-by: Michael Matloob <matloob@golang.org>

5 files changed, 63 insertions, 17 deletions

diff --git a/go/packages/golist.go b/go/packages/golist.go
index 62b282938..0af3a46de 100644
--- a/go/packages/golist.go
+++ b/go/packages/golist.go
@@ -25,7 +25,7 @@ type GoTooOldError struct{ error }
 
 // golistPackages uses the "go list" command to expand the
 // pattern words and return metadata for the specified packages.
-func golistPackages(ctx context.Context, gopath string, cgo, export bool, words []string) ([]*Package, error) {
+func golistPackages(ctx context.Context, gopath string, cgo, export, tests bool, words []string) ([]*Package, error) {
 	// Fields must match go list;
 	// see $GOROOT/src/cmd/go/internal/load/pkg.go.
 	type jsonPackage struct {
@@ -62,7 +62,7 @@ func golistPackages(ctx context.Context, gopath string, cgo, export bool, words
 	// Run "go list" for complete
 	// information on the specified packages.
 
-	buf, err := golist(ctx, gopath, cgo, export, words)
+	buf, err := golist(ctx, gopath, cgo, export, tests, words)
 	if err != nil {
 		return nil, err
 	}
@@ -176,19 +176,13 @@ func absJoin(dir string, fileses ...[]string) (res []string) {
 }
 
 // golist returns the JSON-encoded result of a "go list args..." query.
-func golist(ctx context.Context, gopath string, cgo, export bool, args []string) (*bytes.Buffer, error) {
+func golist(ctx context.Context, gopath string, cgo, export, tests bool, args []string) (*bytes.Buffer, error) {
 	out := new(bytes.Buffer)
-	if len(args) == 0 {
-		return out, nil
-	}
-
-	const test = true // TODO(adonovan): expose a flag for this.
-
 	cmd := exec.CommandContext(ctx, "go", append([]string{
 		"list",
 		"-e",
 		fmt.Sprintf("-cgo=%t", cgo),
-		fmt.Sprintf("-test=%t", test),
+		fmt.Sprintf("-test=%t", tests),
 		fmt.Sprintf("-export=%t", export),
 		"-deps",
 		"-json",
diff --git a/go/packages/gopackages/main.go b/go/packages/gopackages/main.go
index b864e821c..6be503df2 100644
--- a/go/packages/gopackages/main.go
+++ b/go/packages/gopackages/main.go
@@ -27,6 +27,7 @@ import (
 // flags
 var (
 	depsFlag = flag.Bool("deps", false, "show dependencies too")
+	testFlag = flag.Bool("test", false, "include any tests implied by the patterns")
 	cgoFlag  = flag.Bool("cgo", true, "process cgo files")
 	mode     = flag.String("mode", "metadata", "mode (one of metadata, typecheck, wholeprogram)")
 	private  = flag.Bool("private", false, "show non-exported declarations too")
@@ -119,6 +120,7 @@ func main() {
 	opts := &packages.Options{
 		Error:      func(error) {}, // we'll take responsibility for printing errors
 		DisableCgo: !*cgoFlag,
+		Tests:      *testFlag,
 	}
 	lpkgs, err := load(opts, flag.Args()...)
 	if err != nil {
diff --git a/go/packages/packages.go b/go/packages/packages.go
index 18e7460c2..2f2bb85b4 100644
--- a/go/packages/packages.go
+++ b/go/packages/packages.go
@@ -40,6 +40,19 @@ type Options struct {
 	// Replace with flags/cwd/environ pass-through.
 	GOPATH string
 
+	// The Tests flag causes the result to include any test packages
+	// implied by the patterns.
+	//
+	// For example, under 'go build', the "fmt" pattern ordinarily
+	// identifies a single importable package, but with the Tests
+	// flag it additionally denotes the "fmt.test" executable, which
+	// in turn depends on the variant of "fmt" augmented by its
+	// in-packages tests, and the "fmt_test" external test package.
+	//
+	// For build systems in which test names are explicit,
+	// this flag may have no effect.
+	Tests bool
+
 	// DisableCgo disables cgo-processing of files that import "C",
 	// and removes the 'cgo' build tag, which may affect source file selection.
 	// By default, TypeCheck, and WholeProgram queries process such
@@ -298,9 +311,13 @@ func (ld *loader) load(patterns ...string) ([]*Package, error) {
 		ld.GOPATH = os.Getenv("GOPATH")
 	}
 
+	if len(patterns) == 0 {
+		return nil, fmt.Errorf("no packages to load")
+	}
+
 	// Do the metadata query and partial build.
 	// TODO(adonovan): support alternative build systems at this seam.
-	list, err := golistPackages(ld.Context, ld.GOPATH, ld.cgo, ld.mode == typeCheck, patterns)
+	list, err := golistPackages(ld.Context, ld.GOPATH, ld.cgo, ld.mode == typeCheck, ld.Tests, patterns)
 	if err != nil {
 		return nil, err
 	}
@@ -320,7 +337,7 @@ func (ld *loader) load(patterns ...string) ([]*Package, error) {
 		}
 	}
 	if len(pkgs) == 0 {
-		return nil, fmt.Errorf("no packages to load")
+		return nil, fmt.Errorf("packages not found")
 	}
 
 	// Materialize the import graph.
diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go
index d24ff3b25..befd25e09 100644
--- a/go/packages/packages_test.go
+++ b/go/packages/packages_test.go
@@ -36,7 +36,6 @@ import (
 //   import error) will result in a JSON blob with no name and a
 //   nonexistent testmain file in GoFiles. Test that we handle this
 //   gracefully.
-// - import graph for synthetic testmain and "p [t.test]" packages.
 // - IsTest boolean
 //
 // TypeCheck & WholeProgram modes:
@@ -44,6 +43,7 @@ import (
 //   - Packages.Info is correctly set.
 //   - typechecker configuration is honored
 //   - import cycles are gracefully handled in type checker.
+//   - test typechecking of generated test main and cgo.
 
 func TestMetadataImportGraph(t *testing.T) {
 	tmp, cleanup := enterTree(t, map[string]string{
@@ -76,6 +76,34 @@ func TestMetadataImportGraph(t *testing.T) {
 * c
 * e
   errors
+* subdir/d
+  unsafe
+  b -> a
+  b -> errors
+  c -> b
+  c -> unsafe
+  e -> b
+  e -> c
+`[1:]
+
+	if graph != wantGraph {
+		t.Errorf("wrong import graph: got <<%s>>, want <<%s>>", graph, wantGraph)
+	}
+
+	opts.Tests = true
+	initial, err = packages.Metadata(opts, "c", "subdir/d", "e")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Check graph topology.
+	graph, all = importGraph(initial)
+	wantGraph = `
+  a
+  b
+* c
+* e
+  errors
   math/bits
 * subdir/d
   subdir/d [subdir/d.test]
@@ -114,7 +142,7 @@ func TestMetadataImportGraph(t *testing.T) {
 		{"e", "main", "command", "e.go e2.go"},
 		{"errors", "errors", "package", "errors.go"},
 		{"subdir/d", "d", "package", "d.go"},
-		// {"subdir/d.test", "main", "test command", "<hideous generated file name>"},
+		{"subdir/d.test", "main", "test command", "0.go"},
 		{"unsafe", "unsafe", "package", ""},
 	} {
 		p, ok := all[test.id]
@@ -489,8 +517,13 @@ func errorMessages(errors []error) []string {
 func srcs(p *packages.Package) (basenames []string) {
 	// Ideally we would show the root-relative portion (e.g. after
 	// src/) but vgo doesn't necessarily have a src/ dir.
-	for _, src := range p.Srcs {
-		basenames = append(basenames, filepath.Base(src))
+	for i, src := range p.Srcs {
+		if strings.Contains(src, ".cache/go-build") {
+			src = fmt.Sprintf("%d.go", i) // make cache names predictable
+		} else {
+			src = filepath.Base(src)
+		}
+		basenames = append(basenames, src)
 	}
 	return basenames
 }
diff --git a/go/packages/stdlib_test.go b/go/packages/stdlib_test.go
index d70518c00..4c3175d39 100644
--- a/go/packages/stdlib_test.go
+++ b/go/packages/stdlib_test.go
@@ -44,7 +44,7 @@ func TestStdlibMetadata(t *testing.T) {
 
 	t.Logf("Loaded %d packages", len(pkgs))
 	numPkgs := len(pkgs)
-	if want := 340; numPkgs < want {
+	if want := 186; numPkgs < want {
 		t.Errorf("Loaded only %d packages, want at least %d", numPkgs, want)
 	}