[gopls-release-branch.0.1] go/packages: fix overlay deps for packages…

… with test variants

This change handles the case when an import gets added in an overlay a
package with a test variant. Previously, we would only add that
dependency to the test variant of the package.

Fixes golang/go#34379

Change-Id: I82c3d72d7c2d0b970fb27c1aea5be71783b83764
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196259
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
(cherry picked from commit 2c1181b)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196279
Reviewed-by: Ian Cottrell <iancottrell@google.com>

go/packages/golist_overlay.go

@@ -38,10 +38,10 @@ func processGolistOverlay(cfg *Config, response *responseDeduper, rootDirs func(
 	for opath, contents := range cfg.Overlay {
 		base := filepath.Base(opath)
 		dir := filepath.Dir(opath)
-		var pkg *Package
+		var pkg *Package           // if opath belongs to both a package and its test variant, this will be the test variant
 		var testVariantOf *Package // if opath is a test file, this is the package it is testing
 		var fileExists bool
-		isTest := strings.HasSuffix(opath, "_test.go")
+		isTestFile := strings.HasSuffix(opath, "_test.go")
 		pkgName, ok := extractPackageName(opath, contents)
 		if !ok {
 			// Don't bother adding a file that doesn't even have a parsable package statement
@@ -57,13 +57,26 @@ func processGolistOverlay(cfg *Config, response *responseDeduper, rootDirs func(
 				if !sameFile(filepath.Dir(f), dir) {
 					continue
 				}
-				if isTest && !hasTestFiles(p) {
+				// Make sure to capture information on the package's test variant, if needed.
+				if isTestFile && !hasTestFiles(p) {
 					// TODO(matloob): Are there packages other than the 'production' variant
 					// of a package that this can match? This shouldn't match the test main package
 					// because the file is generated in another directory.
 					testVariantOf = p
 					continue nextPackage
 				}
+				if pkg != nil && p != pkg && pkg.PkgPath == p.PkgPath {
+					// If we've already seen the test variant,
+					// make sure to label which package it is a test variant of.
+					if hasTestFiles(pkg) {
+						testVariantOf = p
+						continue nextPackage
+					}
+					// If we have already seen the package of which this is a test variant.
+					if hasTestFiles(p) {
+						testVariantOf = pkg
+					}
+				}
 				pkg = p
 				if filepath.Base(f) == base {
 					fileExists = true
@@ -99,7 +112,7 @@ func processGolistOverlay(cfg *Config, response *responseDeduper, rootDirs func(
 				pkgPath += "_test"
 			}
 			id := pkgPath
-			if isTest && !isXTest {
+			if isTestFile && !isXTest {
 				id = fmt.Sprintf("%s [%s.test]", pkgPath, pkgPath)
 			}
 			// Try to reclaim a package with the same id if it exists in the response.
@@ -115,7 +128,7 @@ func processGolistOverlay(cfg *Config, response *responseDeduper, rootDirs func(
 				response.addPackage(pkg)
 				havePkgs[pkg.PkgPath] = id
 				// Add the production package's sources for a test variant.
-				if isTest && !isXTest && testVariantOf != nil {
+				if isTestFile && !isXTest && testVariantOf != nil {
 					pkg.GoFiles = append(pkg.GoFiles, testVariantOf.GoFiles...)
 					pkg.CompiledGoFiles = append(pkg.CompiledGoFiles, testVariantOf.CompiledGoFiles...)
 				}
@@ -138,12 +151,16 @@ func processGolistOverlay(cfg *Config, response *responseDeduper, rootDirs func(
 			if !found {
 				overlayAddsImports = true
 				// TODO(matloob): Handle cases when the following block isn't correct.
-				// These include imports of test variants, imports of vendored packages, etc.
+				// These include imports of vendored packages, etc.
 				id, ok := havePkgs[imp]
 				if !ok {
 					id = imp
 				}
 				pkg.Imports[imp] = &Package{ID: id}
+				// Add dependencies to the non-test variant version of this package as wel.
+				if testVariantOf != nil {
+					testVariantOf.Imports[imp] = &Package{ID: id}
+				}
 			}
 		}
 		continue

go/packages/packages_test.go

@@ -881,10 +881,11 @@ func testOverlay(t *testing.T, exporter packagestest.Exporter) {
 	exported := packagestest.Export(t, exporter, []packagestest.Module{{
 		Name: "golang.org/fake",
 		Files: map[string]interface{}{
-			"a/a.go": `package a; import "golang.org/fake/b"; const A = "a" + b.B`,
-			"b/b.go": `package b; import "golang.org/fake/c"; const B = "b" + c.C`,
-			"c/c.go": `package c; const C = "c"`,
-			"d/d.go": `package d; const D = "d"`,
+			"a/a.go":      `package a; import "golang.org/fake/b"; const A = "a" + b.B`,
+			"b/b.go":      `package b; import "golang.org/fake/c"; const B = "b" + c.C`,
+			"c/c.go":      `package c; const C = "c"`,
+			"c/c_test.go": `package c; import "testing"; func TestC(t *testing.T) {}`,
+			"d/d.go":      `package d; const D = "d"`,
 		}}})
 	defer exported.Cleanup()
 
@@ -899,6 +900,8 @@ func testOverlay(t *testing.T, exporter packagestest.Exporter) {
 		{map[string][]byte{exported.File("golang.org/fake", "b/b.go"): []byte(`package b; import "golang.org/fake/c"; const B = "B" + c.C`)}, `"aBc"`, nil},
 		// Overlay with an existing file in an existing package adding a new import.
 		{map[string][]byte{exported.File("golang.org/fake", "b/b.go"): []byte(`package b; import "golang.org/fake/d"; const B = "B" + d.D`)}, `"aBd"`, nil},
+		// Overlay with an existing file in an existing package.
+		{map[string][]byte{exported.File("golang.org/fake", "c/c.go"): []byte(`package c; import "net/http"; const C = http.MethodGet`)}, `"abGET"`, nil},
 		// Overlay with a new file in an existing package.
 		{map[string][]byte{
 			exported.File("golang.org/fake", "c/c.go"):                                               []byte(`package c;`),
@@ -941,6 +944,43 @@ func testOverlay(t *testing.T, exporter packagestest.Exporter) {
 	}
 }
 
+func TestOverlayDeps(t *testing.T) { packagestest.TestAll(t, testOverlayDeps) }
+func testOverlayDeps(t *testing.T, exporter packagestest.Exporter) {
+	exported := packagestest.Export(t, exporter, []packagestest.Module{{
+		Name: "golang.org/fake",
+		Files: map[string]interface{}{
+			"c/c.go":      `package c; const C = "c"`,
+			"c/c_test.go": `package c; import "testing"; func TestC(t *testing.T) {}`,
+		},
+	}})
+	defer exported.Cleanup()
+
+	exported.Config.Overlay = map[string][]byte{exported.File("golang.org/fake", "c/c.go"): []byte(`package c; import "net/http"; const C = http.MethodGet`)}
+	exported.Config.Mode = packages.NeedName |
+		packages.NeedFiles |
+		packages.NeedCompiledGoFiles |
+		packages.NeedImports |
+		packages.NeedDeps |
+		packages.NeedTypesSizes
+	initial, err := packages.Load(exported.Config, fmt.Sprintf("file=%s", exported.File("golang.org/fake", "c/c.go")))
+	if err != nil {
+		t.Error(err)
+	}
+	contains := func(imports map[string]*packages.Package, wantImport string) bool {
+		for imp := range imports {
+			if imp == wantImport {
+				return true
+			}
+		}
+		return false
+	}
+	for _, pkg := range initial {
+		if !contains(pkg.Imports, "net/http") {
+			t.Errorf("expected %s import in %s", "net/http", pkg.ID)
+		}
+	}
+}
+
 func TestNewPackagesInOverlay(t *testing.T) { packagestest.TestAll(t, testNewPackagesInOverlay) }
 func testNewPackagesInOverlay(t *testing.T, exporter packagestest.Exporter) {
 	exported := packagestest.Export(t, exporter, []packagestest.Module{{