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: compiler errors disappearing and reappearing in unopened test files #38267

Closed
zikaeroh opened this issue Apr 6, 2020 · 11 comments
Closed

x/tools/gopls: compiler errors disappearing and reappearing in unopened test files #38267

zikaeroh opened this issue Apr 6, 2020 · 11 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.4.0

Comments

@zikaeroh
Copy link
Contributor

zikaeroh commented Apr 6, 2020

Please answer these questions before submitting your issue. Thanks!

What did you do?

I was working on one file in a project that fully compiles and runs. I can publish it if needed.

What did you expect to see?

No errors; the code compiles.

What did you see instead?

Randomly, I'd see that one of the files had a bunch of compiler errors. But, I'd go open the file and they'd disappear. Close the file, and they'd come back. I could modify another file in the package and they'd disappear and come back some time in the future. This happens reliably in this project.

LSP logs: https://gist.github.com/zikaeroh/89ddc6f1b444b756cc44195f607481ad

Screenshots of what VS Code is showing me in one file, versus moving to the file with the errors:

image

image

Build info

golang.org/x/tools/gopls master
    golang.org/x/tools/gopls@v0.1.8-0.20200403190813-44a64ad78b9b h1:VVxELAabDbd0ccBLh29rD/dcK4BkwiQUTgNJrzUqezQ=
    github.com/BurntSushi/toml@v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    golang.org/x/mod@v0.2.0 h1:KU7oHjnv3XNWfa5COkzUifxZmxp1TyI7ImMXqFxLwvQ=
    golang.org/x/sync@v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY=
    golang.org/x/tools@v0.0.0-20200403190813-44a64ad78b9b h1:AFZdJUT7jJYXQEC29hYH/WZkoV7+KhwxQGmdZ19yYoY=
    golang.org/x/xerrors@v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
    honnef.co/go/tools@v0.0.1-2020.1.3 h1:sXmLre5bzIR6ypkjXCDI3jHPssRhc8KD/Ome589sc3U=
    mvdan.cc/xurls/v2@v2.1.0 h1:KaMb5GLhlcSX+e+qhbRJODnUUBvlw01jt4yrjFIHAuA=

Go info

go version go1.14.1 linux/amd64
@gopherbot gopherbot added this to the Unreleased milestone Apr 6, 2020
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Apr 6, 2020
@golang golang deleted a comment from gopherbot Apr 6, 2020
@stamblerre
Copy link
Contributor

Thanks for the issue report and the log! Some quick observations:

  • The test file in question is not open till the end of the log (only open for 1 version).
  • It seems to alternate between diagnostics from analyses and diagnostics from the compiler.

From this information, my first guess is that this is an issue with diagnostics caching, since the file is not changing, but the diagnostics are seemingly changing and being resent. I'll see if I can come up with a regression test for this case.

A question for you: Were the diagnostics ever correct? Are the analysis diagnostics accurate or are they also outdated?

@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.5.0 Apr 6, 2020
@zikaeroh
Copy link
Contributor Author

zikaeroh commented Apr 6, 2020

I restarted gopls to get these logs; the original session's were 6MB and probably contain a lot of info. I can likely include that if you want.

Were the diagnostics ever correct?

I'm not sure which diagnostics you mean, sorry. Do you mean the "cannot use" ones? I don't think those were correct; the package builds and I can edit the file without trouble.

Are the analysis diagnostics accurate or are they also outdated?

Which diagnostics are the analysis ones? Most (if not all) of the warnings are from golangci-lint (as I don't run staticcheck live). I'll have to disable it to get a better view on things, since all of these tools display their diagnostics with the underlying tool's/analysis' names. I know that this project has a bunch of places without gofmt -s changes, which get auto-fixed on save, so I guess those are analysis diagnistics.

The repro is this repo on the selected branch, if you want to test this more easily: https://github.com/zikaeroh/sqlboiler/tree/with-gomod

@stamblerre
Copy link
Contributor

stamblerre commented Apr 6, 2020 •
edited

Thanks for sharing the repro - I didn't realize it repro'd this easily. I should be able to make some progress off of the repro.

Edit: This does not reproduce with gopls/v0.3.4.

@zikaeroh
Copy link
Contributor Author

zikaeroh commented Apr 6, 2020

Edit: This does not reproduce with gopls/v0.3.4.

I'm assuming that's a note to self, given I'm running at the current master build and not 0.3.4 (so probably something in between). 🙂

@stamblerre
Copy link
Contributor

Yep 😄Sorry about that - just wanted to take notes as I investigate. I think I've isolated this to some weirdness with the new gofmt -s analyzers + diagnostics caching.

@stamblerre stamblerre modified the milestones: gopls/v0.5.0, gopls/v0.4.0 Apr 6, 2020
@gopherbot
Copy link

Change https://golang.org/cl/227299 mentions this issue: internal/lsp: handle unexpected compiler errors

@myitcv
Copy link
Member

myitcv commented Apr 6, 2020

FWIW I'm also seeing instances of the "cannot use &(X literal) (value of type *X) as *X value in assignment" with -remote, against commit 9fc00b0.

@stamblerre - do you have enough information here or would you like further log files?

@stamblerre
Copy link
Contributor

Can you try patching in https://golang.org/cl/227299 and seeing if that fixes the issue?

myitcv pushed a commit to myitcvforks/tools that referenced this issue Apr 6, 2020
@stamblerre @myitcv
internal/lsp: make sure that gofmt -s analyses don't modify AST
4bd6e5b 
The code for `gofmt -s` directly modifies the AST, since the ASTs are
not long-lived. Some of this code made it into our analysis
implementations, causing very strange bugs to manifest. Added a
regression test for this specific case.

Fixes golang/go#38267

Change-Id: I4c90cbd686f5e7e635a94abe40463feb5911fa2d
@gopherbot
Copy link

Change https://golang.org/cl/227354 mentions this issue: internal/lsp: make sure that gofmt -s analyses don't modify AST

@zikaeroh
Copy link
Contributor Author

zikaeroh commented Apr 6, 2020

Fixed it for me, thank you!

gopherbot pushed a commit to golang/tools that referenced this issue Apr 6, 2020
@stamblerre
internal/lsp: make sure that gofmt -s analyses don't modify AST
b2e6919 
The code for `gofmt -s` directly modifies the AST, since the ASTs are
not long-lived. Some of this code made it into our analysis
implementations, causing very strange bugs to manifest. Added a
regression test for this specific case.

Fixes golang/go#38267

Change-Id: I235620adcbf2bbc7027c6d83ff2c7fe74729062e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227299
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
(cherry picked from commit 1fd9766)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227354
Reviewed-by: Robert Findley <rfindley@google.com>
@myitcv
Copy link
Member

myitcv commented Apr 7, 2020

@stamblerre

Can you try patching in https://golang.org/cl/227299 and seeing if that fixes the issue?

I didn't have a repro in this case, so totally happy to go with you and @zikaeroh on this one. Will open a new issue if I spot this again.

@myitcv myitcv mentioned this issue Apr 13, 2020
Closed
@golang golang locked and limited conversation to collaborators Apr 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.4.0
Development

No branches or pull requests
4 participants
@myitcv @stamblerre @gopherbot @zikaeroh