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: uncleared diagnostics when toggling gc_details #44826

Closed
findleyr opened this issue Mar 6, 2021 · 1 comment
Closed

x/tools/gopls: uncleared diagnostics when toggling gc_details #44826

findleyr opened this issue Mar 6, 2021 · 1 comment

Comments

@findleyr
Copy link
Contributor

@findleyr findleyr commented Mar 6, 2021

Reported by @leitzler in Slack, govim CI is experiencing some flakes related to gc_details diagnostics when upgrading its gopls version. Capturing this as an issue as a reminder to investigate.

We are experiencing some flakes in CI for a test that toggles GC Details on and off. It looks like gopls isn’t sending an updated textDocument/publishDiagnostics were the GC Details diags are removed after toggling it off sometimes. It started to happen after updating from v0.1.1-0.20210227203037-f5a4005dda57 to v0.1.1-0.20210303215420-376db57240db. The test toggles GC Details on and then off with the following setup:

-- go.mod --
module mod.com
go 1.12
-- main.go --
package main
func main() {
	fn()
}
func fn() {}

And sometimes during the -race tests gopls doesn’t send any empty diags back.
Here is a gist with gopls logs, both when it works and when it doesn’t work: https://gist.github.com/leitzler/345d34f5fdc313ec27dc9513ab7cc176
This is the specific one I’m missing when it doesn’t work: https://gist.github.com/leitzler/345d34f5fdc313ec27dc9513ab7cc176#file-working-log-L145-L146

This sounds potentially related to @heschik's recent changes to diagnostics. Here are the new changes in the later version:

> git log --oneline f5a4005dda57..376db57240db
376db572 internal/lsp: use pre-existing quick fixes for analysis diagnostics
144d5ced internal/lsp: run type error analyzers as part of diagnostics
24439e3c internal/lsp/source: eliminate GetTypeCheckDiagnostics
dafbee50 internal/lsp: show human-readable const time.Duration as a comment
9c452d85 internal/lsp/cache: don't rely on related diagnostics if unsupported
2ac05c83 internal/lsp: key GC details off package ID
303d8bbb gopls/internal/hooks: compile URL regexp once
94327d32 internal/lsp/cache: show type errors on parse errors in other files
16b2c870 gopls/internal/regtest: add a failing test for issue 44736
78002535 internal/lsp/cache: split up sourceDiagnostics
47985cf3 internal/lsp/cache: refactor Go file parsing
6422c5c8 internal/lsp/cache: invalidate metadata on magic comment changes
f9c628b1 gopls/internal/regtest: add test for bad embed rules
89a9cb6e internal/lsp/cache: parse filenames from go list errors correctly
eb48d3f6 internal/lsp/cache: refactor diagnostic suppression
7a079fcd internal/lsp/cache: fix related error processing

I haven't tried to repro locally yet.

CC @stamblerre

@gopherbot
Copy link

@gopherbot gopherbot commented Mar 23, 2021

Change https://golang.org/cl/304169 mentions this issue: internal/lsp: additional debugging to diagnose golang/go#44826

leitzler added a commit to leitzler/tools that referenced this issue Mar 23, 2021
DO NOT REVIEW

Change-Id: I7b9108a829c98a84360c9012c1b60f4990839b5a
leitzler added a commit to leitzler/tools that referenced this issue Mar 24, 2021
DO NOT REVIEW

Change-Id: I7b9108a829c98a84360c9012c1b60f4990839b5a
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
3 participants