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: empty diagnostics overwrites earlier diagnostics with same version #42837
Comments
Another example of duplicate [Trace - 09:19:01.307 AM] Sending request 'initialize - (1)'. Params: {"processId":0,"clientInfo":{"name":""},"rootUri":"file:///artefacts/govim/cmd/govim/scenario_default/script-gofmt","capabilities":{"Workspace":{"workspaceEdit":{},"didChangeConfiguration":{"dynamicRegistration":true},"didChangeWatchedFiles":{"dynamicRegistration":true},"symbol":{"symbolKind":{},"tagSupport":{"valueSet":null}},"executeCommand":{},"semanticTokens":{},"configuration":true},"textDocument":{"synchronization":{},"completion":{"completionItem":{"tagSupport":{"valueSet":null},"resolveSupport":{"properties":null}},"completionItemKind":{}},"hover":{"contentFormat":["plaintext"]},"signatureHelp":{"signatureInformation":{"parameterInformation":{}}},"declaration":{},"definition":{},"typeDefinition":{},"implementation":{},"references":{},"documentHighlight":{},"documentSymbol":{"symbolKind":{},"tagSupport":{"valueSet":null}},"codeAction":{"codeActionLiteralSupport":{"codeActionKind":{"valueSet":null}},"resolveSupport":{"properties":null}},"codeLens":{},"documentLink":{},"colorProvider":{},"formatting":{},"rangeFormatting":{},"onTypeFormatting":{},"rename":{},"foldingRange":{},"selectionRange":{},"publishDiagnostics":{"tagSupport":{"valueSet":null}},"callHierarchy":{},"semanticTokens":{"requests":{},"tokenTypes":null,"tokenModifiers":null,"formats":null}},"Window":{"workDoneProgress":true}},"initializationOptions":{"symbolMatcher":"fuzzy","symbolStyle":"full"},"workspaceFolders":null} |
Change https://golang.org/cl/274241 mentions this issue: |
A guard against publishing stale diagnostics was removed in CL 269677, because the signficance of diagnoseDetached was overlooked. Restore it. For golang/go#42837 Change-Id: I911c4707a9c18a632ce32e0c25c43ae2e92135b2 Reviewed-on: https://go-review.googlesource.com/c/tools/+/274241 Run-TryBot: Robert Findley <rfindley@google.com> Trust: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Change https://golang.org/cl/275238 mentions this issue: |
Change https://golang.org/cl/275239 mentions this issue: |
There was a race to the background context that allowed work for earlier snapshots to run on the background context of later snapshots. For example, the following sequence is possible and has been observed in tests (admittedly at unrealistic time scales): - snapshot N is created - view.cancelBackground() is called - snapshot N+1 is created - snapshot N+1 is diagnosed - snapshot N is diagnosed - snapshot N is destroyed Fix this by pinning the background context to the snapshot. This also makes more sense; I'm guessing the background context being on view is a vestige of when more work was done on the view. For golang/go#42837 Change-Id: If9cbad7361de823da9bfd2f55cb2f8b3aed95075 Reviewed-on: https://go-review.googlesource.com/c/tools/+/275239 Run-TryBot: Robert Findley <rfindley@google.com> Trust: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The LSP server tracks the snapshot ID for which diagnostics were last published, but this ID was not being updated if the publish was suppressed when there are no changes, which can cause incorrect diagnostics when snapshots are published out-of-order (a separate bug). Fix this to update the snapshot ID when the current published diagnostics match the latest computed diagnostics. Fixes golang/go#42837 Change-Id: I3634e6f351a479aefa5c9423956720431590d814 Reviewed-on: https://go-review.googlesource.com/c/tools/+/275238 Run-TryBot: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Robert Findley <rfindley@google.com> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
There was a race to the background context that allowed work for earlier snapshots to run on the background context of later snapshots. For example, the following sequence is possible and has been observed in tests (admittedly at unrealistic time scales): - snapshot N is created - view.cancelBackground() is called - snapshot N+1 is created - snapshot N+1 is diagnosed - snapshot N is diagnosed - snapshot N is destroyed Fix this by pinning the background context to the snapshot. This also makes more sense; I'm guessing the background context being on view is a vestige of when more work was done on the view. For golang/go#42837 Change-Id: If9cbad7361de823da9bfd2f55cb2f8b3aed95075 Reviewed-on: https://go-review.googlesource.com/c/tools/+/275239 Run-TryBot: Robert Findley <rfindley@google.com> Trust: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
What version of Go are you using (
go version
)?What did you do?
This was found during
govim
CI tests with race-detection enabled where several different test cases failed, https://github.com/govim/govim/actions/runs/382858463.I looked at two different failures and the common thing was that
gopls
did sendpublishDiagnostics
without any diagnostics after sending real diagnostics.e.g. this log where
"version": 5
is sent twice, empty the second time:Another example where an empty
0
-version is sent after a correct"version": 1
:What did you expect to see?
In these two cases, no second
publishDiagnostics
with empty diagnostics.//cc @findleyr
The text was updated successfully, but these errors were encountered: