internal/lsp: always ParseFull in-workspace dependencies
When searching for implementations we look at all packages in the
workspace. We do a full parse since we need to look for non-exported
types and look in functions for type declarations. However, we always
type check a package's dependencies in export-only mode to save work.
This leads to what I call the "two world" syndrome where you have both
the export-only and full-parse versions of a package in play at once.
This is problematic because mirror objects in each version do not
compare equal.
For example:
-- a/a.go --
package a
type Breed int
const Mutt Breed = 0
type Dog interface{ Breed() Breed }
-- b/b.go --
package b
import "a"
type dog struct{}
func (dog) Breed() a.Breed { return a.Mutt }
---
In this situation, the problem is "b" loads its dependency "a" in
export only mode so it gets one version of the "a.Breed" type. The
user opens package "a" directly so it gets fully type checked and has
a second version of "a.Breed". The user searches for "a.Dog"
implementations, but "b.dog" does not implement the fully-loaded
"a.Dog" because it returns the export-only version of the "a.Breed"
type.
Fix it by always loading in-workspace dependencies in full parse mode.
We need to load them in full parse mode anyway if the user does find
references or find implementations.
In writing a test I fixed an incorrect import in the testdata. This
uncovered an unrelated bug which made a different implementation test
very flaky. I disabled it for now since I couldn't see a fix simple
enough to slip into this commit.
Fixes golang/go#35857.
Change-Id: I01509f57d54d593e62c895c7ecb93eb5f780bec7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209759
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index ef547bf..8bd21b8 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -136,7 +136,11 @@
// Begin computing the key by getting the depKeys for all dependencies.
var depKeys [][]byte
for _, depID := range depList {
- depHandle, err := s.packageHandle(ctx, depID, source.ParseExported)
+ mode := source.ParseExported
+ if s.workspacePackages[depID] {
+ mode = source.ParseFull
+ }
+ depHandle, err := s.packageHandle(ctx, depID, mode)
if err != nil {
log.Error(ctx, "no dep handle", err, telemetry.Package.Of(depID))
diff --git a/internal/lsp/testdata/implementation/implementation.go b/internal/lsp/testdata/implementation/implementation.go
index 2b10701..b4a98cc 100644
--- a/internal/lsp/testdata/implementation/implementation.go
+++ b/internal/lsp/testdata/implementation/implementation.go
@@ -1,6 +1,6 @@
package implementation
-import "implementation/other"
+import "golang.org/x/tools/internal/lsp/implementation/other"
type ImpP struct{} //@ImpP
@@ -25,5 +25,9 @@
}
type U interface {
- U() //@mark(IntU, "U"),implementations("U", ImpU),
+ U() //TODO: fix flaky @implementations("U", ImpU)
}
+
+type cryer int
+
+func (cryer) Cry(other.CryType) {} //@mark(CryImpl, "Cry")
diff --git a/internal/lsp/testdata/implementation/other/other.go b/internal/lsp/testdata/implementation/other/other.go
index e8410af..6fe16d4 100644
--- a/internal/lsp/testdata/implementation/other/other.go
+++ b/internal/lsp/testdata/implementation/other/other.go
@@ -19,3 +19,11 @@
func (Foo) U() { //@mark(ImpU, "U")
}
+
+type CryType int
+
+const Sob CryType = 1
+
+type Cryer interface {
+ Cry(CryType) //@implementations("Cry", CryImpl)
+}
