internal/lsp/cache: always compute IsIntermediateTestVariant
It is inconsistent to only compute IsIntermediateTestVariant for
workspace packages, and could be a source of bugs. Always compute it.
Change-Id: I16f40fe44a1145a74ef77fee4b7fd813abe603cb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/340732
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index 1d4921a..96c2a07 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -441,6 +441,12 @@
depsErrors: packagesinternal.GetDepsErrors(pkg),
}
+ // Identify intermediate test variants for later filtering. See the
+ // documentation of IsIntermediateTestVariant for more information.
+ if m.ForTest != "" && m.ForTest != m.PkgPath && m.ForTest+"_test" != m.PkgPath {
+ m.IsIntermediateTestVariant = true
+ }
+
for _, err := range pkg.Errors {
// Filter out parse errors from go list. We'll get them when we
// actually parse, and buggy overlay support may generate spurious
@@ -532,9 +538,6 @@
// The test variant of some workspace package or its x_test.
// To load it, we need to load the non-test variant with -test.
s.workspacePackages[m.ID] = m.ForTest
- default:
- // A test variant of some intermediate package. We don't care about it.
- m.IsIntermediateTestVariant = true
}
}
return m, nil
diff --git a/internal/lsp/cache/metadata.go b/internal/lsp/cache/metadata.go
index 618578d..c2a2196 100644
--- a/internal/lsp/cache/metadata.go
+++ b/internal/lsp/cache/metadata.go
@@ -43,6 +43,29 @@
// IsIntermediateTestVariant reports whether the given package is an
// intermediate test variant, e.g.
// "golang.org/x/tools/internal/lsp/cache [golang.org/x/tools/internal/lsp/source.test]".
+ //
+ // Such test variants arise when an x_test package (in this case source_test)
+ // imports a package (in this case cache) that itself imports the the
+ // non-x_test package (in this case source).
+ //
+ // This is done so that the forward transitive closure of source_test has
+ // only one package for the "golang.org/x/tools/internal/lsp/source" import.
+ // The intermediate test variant exists to hold the test variant import:
+ //
+ // golang.org/x/tools/internal/lsp/source_test [golang.org/x/tools/internal/lsp/source.test]
+ // | "golang.org/x/tools/internal/lsp/cache" -> golang.org/x/tools/internal/lsp/cache [golang.org/x/tools/internal/lsp/source.test]
+ // | "golang.org/x/tools/internal/lsp/source" -> golang.org/x/tools/internal/lsp/source [golang.org/x/tools/internal/lsp/source.test]
+ // | ...
+ //
+ // golang.org/x/tools/internal/lsp/cache [golang.org/x/tools/internal/lsp/source.test]
+ // | "golang.org/x/tools/internal/lsp/source" -> golang.org/x/tools/internal/lsp/source [golang.org/x/tools/internal/lsp/source.test]
+ // | ...
+ //
+ // We filter these variants out in certain places. For example, there is
+ // generally no reason to run diagnostics or analysis on them.
+ //
+ // TODO(rfindley): this can probably just be a method, since it is derived
+ // from other fields.
IsIntermediateTestVariant bool
}
