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

x/tools/gopls: missing metadata for import #40685

Closed
stamblerre opened this issue Aug 11, 2020 · 7 comments
Closed

x/tools/gopls: missing metadata for import #40685

stamblerre opened this issue Aug 11, 2020 · 7 comments
Labels
Milestone

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Aug 11, 2020

I've noticed a few times that after adding and removing log.Printfs that I get an error saying that there is no metadata for the log package. This seems like something is going wrong with our metadata invalidation logic in (*snapshot).clone.

@stamblerre stamblerre added this to the gopls/v1.0.0 milestone Aug 11, 2020
@stamblerre stamblerre added this to To Do in gopls/v1.0.0 Aug 26, 2020
@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.5.0 Sep 2, 2020
@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Sep 2, 2020

I've been noticing this again and actually saw it specifically when I manually added imports from a quickfix.

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Sep 2, 2020

Ok yep, this is easily reproducible by adding a symbol from an unimported package and accepting the quick fix. Will investigate.

Edit: Less easily reproducible than I thought. I reproduced it by adding os.Stat("") to golang.org/x/tools/internal/lsp/mod/code_lens.go at 5b9ef244dc36f978f990312c29463d66e655f263. Note that the only other import of "os" is in the mod_test.go file, which is an in-package test.

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 2, 2020

Change https://golang.org/cl/252681 mentions this issue: internal/lsp/regtest: reproduce golang/go#40685

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Sep 2, 2020

To clarify, while this is reproducible with a quick fix, I have also noticed it when running goimports on save, rather than with a quick fix.

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Sep 5, 2020

Just noticed that my repro case test failed for all but Go 1.14, so this may be a Go command bug, which would explain its recent appearance.

Edit: Confirming that I cannot reproduce with Go 1.14.

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Sep 5, 2020

Looks like this is a consequence of #36857--an overlaid file in a package that has a test variant really needs to be added both to the original package and its test variant. We currently don't handle this, but I think the ordering of the pre-1.15 go list output resulted in the file getting added to the non-test variant package.

Now it's being added to the test variant, which results in incorrect gopls diagnostics. The complete fix would be fixing #36857, but since native overlays are coming to the Go command, I'll just apply a simple "band-aid" fix.

@stamblerre stamblerre removed this from Bugs in gopls/v1.0.0 Sep 9, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 9, 2020

Change https://golang.org/cl/253800 mentions this issue: internal/lsp, go/packages: fix Go 1.15-related overlay bug

gopherbot pushed a commit to golang/tools that referenced this issue Sep 9, 2020
…ated overlay bug

Something about the ordering of `go list` results must have changed in
Go 1.15, so overlays with test variants were failing to get the correct
imports. Technically, the correct solution would have been to support
overlays for a package *and* its test variant, but for the purposes of
gopls, it's actually more important that the overlays apply to the
package itself (rather than its test variant).

Fixes golang/go#40685

Change-Id: I3282557502f7f30bf484e1e6c17b90db824bc7d0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/253800
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.