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>
diff --git a/go/packages/golist_overlay.go b/go/packages/golist_overlay.go
index 941b0f4..308e79c 100644
--- a/go/packages/golist_overlay.go
+++ b/go/packages/golist_overlay.go
@@ -38,10 +38,10 @@
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 @@
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 @@
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 @@
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 @@
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
diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go
index dc75f15..4d10d41 100644
--- a/go/packages/packages_test.go
+++ b/go/packages/packages_test.go
@@ -881,10 +881,11 @@
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 @@
{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 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{{
