From ca6481ae56504398949d597084558e50ad07117a Mon Sep 17 00:00:00 2001
From: Michael Matloob <matloob@golang.org>
Date: Tue, 7 Aug 2018 14:48:55 -0400
Subject: [PATCH] go/packages/golist: fix golist fallback selection

The fallback was being reset to the incorrect value
after it was set. This change fixes it.

Also add a test that triggers the fallback (a query with both
a contains: line and a regular package path).

Change-Id: I49a9aeb3a0c7d7cc308ac56f4985545315a5bfd2
Reviewed-on: https://go-review.googlesource.com/128356
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
---
 go/packages/golist/golist.go | 17 ++++++++++-------
 go/packages/packages_test.go | 30 ++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/go/packages/golist/golist.go b/go/packages/golist/golist.go
index 222a0f47..a41c049d 100644
--- a/go/packages/golist/golist.go
+++ b/go/packages/golist/golist.go
@@ -45,7 +45,7 @@ func LoadRaw(ctx context.Context, cfg *raw.Config, patterns ...string) ([]string
 	// Determine files requested in contains patterns
 	var containFiles []string
 	{
-		restPatterns := patterns[:0]
+		restPatterns := make([]string, 0, len(patterns))
 		for _, pattern := range patterns {
 			if containFile := strings.TrimPrefix(pattern, "contains:"); containFile != pattern {
 				containFiles = append(containFiles, containFile)
@@ -63,7 +63,7 @@ func LoadRaw(ctx context.Context, cfg *raw.Config, patterns ...string) ([]string
 		roots, pkgs, err := golistPackages(ctx, cfg, patterns...)
 		if _, ok := err.(goTooOldError); ok {
 			listfunc = golistPackagesFallback
-			roots, pkgs, err = listfunc(ctx, cfg, patterns...)
+			return listfunc(ctx, cfg, patterns...)
 		}
 		listfunc = golistPackages
 		return roots, pkgs, err
@@ -82,6 +82,7 @@ func LoadRaw(ctx context.Context, cfg *raw.Config, patterns ...string) ([]string
 
 	// Run go list for contains: patterns.
 	seenPkgs := make(map[string]bool) // for deduplication. different containing queries could produce same packages
+	seenRoots := make(map[string]bool)
 	if len(containFiles) > 0 {
 		for _, pkg := range pkgs {
 			seenPkgs[pkg.ID] = true
@@ -96,21 +97,23 @@ func LoadRaw(ctx context.Context, cfg *raw.Config, patterns ...string) ([]string
 			return nil, nil, err
 		}
 		// Deduplicate and set deplist to set of packages requested files.
-		dedupedList := cList[:0] // invariant: only packages that haven't been seen before
 		for _, pkg := range cList {
-			if seenPkgs[pkg.ID] {
+			if seenRoots[pkg.ID] {
 				continue
 			}
-			seenPkgs[pkg.ID] = true
-			dedupedList = append(dedupedList, pkg)
 			for _, pkgFile := range pkg.GoFiles {
 				if filepath.Base(f) == filepath.Base(pkgFile) {
+					seenRoots[pkg.ID] = true
 					roots = append(roots, pkg.ID)
 					break
 				}
 			}
+			if seenPkgs[pkg.ID] {
+				continue
+			}
+			seenPkgs[pkg.ID] = true
+			pkgs = append(pkgs, pkg)
 		}
-		pkgs = append(pkgs, dedupedList...)
 	}
 	return roots, pkgs, nil
 }
diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go
index 46efa8bd..1ea1284b 100644
--- a/go/packages/packages_test.go
+++ b/go/packages/packages_test.go
@@ -862,6 +862,36 @@ func TestContains(t *testing.T) {
 	}
 }
 
+// TestContains_FallbackSticks ensures that when there are both contains and non-contains queries
+// the decision whether to fallback to the pre-1.11 go list sticks across both sets of calls to
+// go list.
+func TestContains_FallbackSticks(t *testing.T) {
+	tmp, cleanup := makeTree(t, map[string]string{
+		"src/a/a.go": `package a; import "b"`,
+		"src/b/b.go": `package b; import "c"`,
+		"src/c/c.go": `package c`,
+	})
+	defer cleanup()
+
+	opts := &packages.Config{Env: append(os.Environ(), "GOPATH="+tmp), Dir: tmp, Mode: packages.LoadImports}
+	initial, err := packages.Load(opts, "a", "contains:src/b/b.go")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	graph, _ := importGraph(initial)
+	wantGraph := `
+* a
+* b
+  c
+  a -> b
+  b -> c
+`[1:]
+	if graph != wantGraph {
+		t.Errorf("wrong import graph: got <<%s>>, want <<%s>>", graph, wantGraph)
+	}
+}
+
 func errorMessages(errors []error) []string {
 	var msgs []string
 	for _, err := range errors {