gopls/internal/lsp/cache: make IsIntermediateTestVariant a method
Metadata.IsIntermediateTestVariant can be derived from PkgPath and
ForTest, so make it a method. Along the way, document the easily missed
fact that intermediate test variants are not workspace packages.
For golang/go#55293
Change-Id: Ie03011aef9c91ebce89e8aad01ef39b65bdde09a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/438497
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/lsp/cache/load.go b/gopls/internal/lsp/cache/load.go
index 2bf4a33..287c310 100644
--- a/gopls/internal/lsp/cache/load.go
+++ b/gopls/internal/lsp/cache/load.go
@@ -17,11 +17,11 @@
"time"
"golang.org/x/tools/go/packages"
- "golang.org/x/tools/internal/event"
- "golang.org/x/tools/internal/gocommand"
- "golang.org/x/tools/internal/event/tag"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/source"
+ "golang.org/x/tools/internal/event"
+ "golang.org/x/tools/internal/event/tag"
+ "golang.org/x/tools/internal/gocommand"
"golang.org/x/tools/internal/packagesinternal"
"golang.org/x/tools/internal/span"
)
@@ -504,12 +504,6 @@
}
updates[id] = m
- // 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
@@ -686,6 +680,9 @@
case m.ForTest == m.PkgPath, m.ForTest+"_test" == m.PkgPath:
// The test variant of some workspace package or its x_test.
// To load it, we need to load the non-test variant with -test.
+ //
+ // Notably, this excludes intermediate test variants from workspace
+ // packages.
workspacePackages[m.ID] = m.ForTest
}
}
diff --git a/gopls/internal/lsp/cache/metadata.go b/gopls/internal/lsp/cache/metadata.go
index 711508c..7778996 100644
--- a/gopls/internal/lsp/cache/metadata.go
+++ b/gopls/internal/lsp/cache/metadata.go
@@ -39,34 +39,6 @@
// Config is the *packages.Config associated with the loaded package.
Config *packages.Config
-
- // IsIntermediateTestVariant reports whether the given package is an
- // intermediate test variant, e.g.
- // "golang.org/x/tools/gopls/internal/lsp/cache [golang.org/x/tools/gopls/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/gopls/internal/lsp/source" import.
- // The intermediate test variant exists to hold the test variant import:
- //
- // golang.org/x/tools/gopls/internal/lsp/source_test [golang.org/x/tools/gopls/internal/lsp/source.test]
- // | "golang.org/x/tools/gopls/internal/lsp/cache" -> golang.org/x/tools/gopls/internal/lsp/cache [golang.org/x/tools/gopls/internal/lsp/source.test]
- // | "golang.org/x/tools/gopls/internal/lsp/source" -> golang.org/x/tools/gopls/internal/lsp/source [golang.org/x/tools/gopls/internal/lsp/source.test]
- // | ...
- //
- // golang.org/x/tools/gopls/internal/lsp/cache [golang.org/x/tools/gopls/internal/lsp/source.test]
- // | "golang.org/x/tools/gopls/internal/lsp/source" -> golang.org/x/tools/gopls/internal/lsp/source [golang.org/x/tools/gopls/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
}
// Name implements the source.Metadata interface.
@@ -79,6 +51,40 @@
return string(m.PkgPath)
}
+// IsIntermediateTestVariant reports whether the given package is an
+// intermediate test variant, e.g. "net/http [net/url.test]".
+//
+// Such test variants arise when an x_test package (in this case net/url_test)
+// imports a package (in this case net/http) that itself imports the the
+// non-x_test package (in this case net/url).
+//
+// This is done so that the forward transitive closure of net/url_test has
+// only one package for the "net/url" import.
+// The intermediate test variant exists to hold the test variant import:
+//
+// net/url_test [net/url.test]
+//
+// | "net/http" -> net/http [net/url.test]
+// | "net/url" -> net/url [net/url.test]
+// | ...
+//
+// net/http [net/url.test]
+//
+// | "net/url" -> net/url [net/url.test]
+// | ...
+//
+// This restriction propagates throughout the import graph of net/http: for
+// every package imported by net/http that imports net/url, there must be an
+// intermediate test variant that instead imports "net/url [net/url.test]".
+//
+// As one can see from the example of net/url and net/http, intermediate test
+// variants can result in many additional packages that are essentially (but
+// not quite) identical. For this reason, we filter these variants wherever
+// possible.
+func (m *Metadata) IsIntermediateTestVariant() bool {
+ return m.ForTest != "" && m.ForTest != m.PkgPath && m.ForTest+"_test" != m.PkgPath
+}
+
// ModuleInfo implements the source.Metadata interface.
func (m *Metadata) ModuleInfo() *packages.Module {
return m.Module
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index d13f56c..fb4e78b 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -695,7 +695,7 @@
for _, id := range knownIDs {
// Filter out any intermediate test variants. We typically aren't
// interested in these packages for file= style queries.
- if m := s.getMetadata(id); m != nil && m.IsIntermediateTestVariant && !withIntermediateTestVariants {
+ if m := s.getMetadata(id); m != nil && m.IsIntermediateTestVariant() && !withIntermediateTestVariants {
continue
}
var parseModes []source.ParseMode
