Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/tools/gopls: reconsider the experimental feature to use invalid metadata #54180

Open
findleyr opened this issue Aug 1, 2022 · 1 comment
Open

x/tools/gopls: reconsider the experimental feature to use invalid metadata #54180

findleyr opened this issue Aug 1, 2022 · 1 comment
Labels
gopls Tools
Milestone
gopls/later

Comments

@findleyr
Copy link
Contributor

findleyr commented Aug 1, 2022

Discovered while investigating #54147: with "experimentalUseInvalidMetadata" set, we may diagnose a package with invalid metadata, and not re-diagnose when new metadata arrives. This can lead to diagnostics which are stale until you begin typing again.

We could fix this by re-diagnosing after each metadata load, but more generally we should reconsider whether it is worth landing this feature:

Pros:

  • when the workspace is totally broken, gopls will continue to work

Cons:

  • though gopls will continue to work, it may have inconsistent or confusing state
  • users should generally fix broken workspaces before doing anything else
  • we really don't want users to have to restart gopls, so in all other respects we are trying to avoid places where gopls "remembers history"
@findleyr findleyr added this to the gopls/later milestone Aug 1, 2022
@gopherbot gopherbot added Tools gopls labels Aug 1, 2022
@gopherbot
Copy link

gopherbot commented Aug 3, 2022

Change https://go.dev/cl/420956 mentions this issue: internal/lsp/cache: invalid packages should not be workspace packages

gopherbot pushed a commit to golang/tools that referenced this issue Aug 4, 2022
@findleyr
internal/lsp/cache: invalid packages should not be workspace packages
01c9ff0 
To support the experimentalUseInvalidMetadata mode, we keep around
invalid metadata. This can help gopls features work when, for example,
the go.mod file is broken. It is debatable whether this feature is worth
supporting (see golang/go#54180), but in the meantime there is a very
negative side-effect when module paths are changed in the go.mod file:
we keep around a bunch of workspace packages with a stale module path.
As a result we can be left with a lots of extra type-checked packages
in memory, and many inaccurate diagnostics.

Fix this by skipping packages with invalid metadata when computing
workspace packages. While we may want to use invalid metadata when
finding the best package for a file, it does not make sense to re-
type-check and diagnose all those stale packages.

Fixes golang/go#43186

Change-Id: Id73b47ea138ec80a9de63b03dae41d4e509b8d5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420956
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
gopls Tools
Projects
None yet
Milestone
gopls/later
Development

No branches or pull requests
2 participants
@gopherbot @findleyr