Skip to content

x/tools/gopls: follow-up to golang/vscode-go#2406 (analyzer print statement breaking gopls) #54459

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.

Sign up for GitHub

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
Open
2 of 5 tasks
findleyr opened this issue Aug 15, 2022 · 1 comment
Open
2 of 5 tasks

x/tools/gopls: follow-up to golang/vscode-go#2406 (analyzer print statement breaking gopls) #54459

findleyr opened this issue Aug 15, 2022 · 1 comment
Labels
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/backlog

Comments

@findleyr
Copy link
Member

findleyr commented Aug 15, 2022 •
edited
Loading

This is a follow up issue for golang/vscode-go#2406. That issue reports a rare but severe breakage that made it into the [email protected] release. Several processes broke down to let that happen, so this issue tracks improvements we can make to help avoid such regressions in the future.

Background: Gopls runs go vet analyzers for open packages. When new analyzers are added to vet (in the x/tools/go/analysis directory), they can be included in gopls via inclusion in internal/lsp/source.defaultAnalyzers. In this case, a new analyzer was added that accidentally included a print statement. Any print to stdout corrupts gopls' jsonrpc2 communication with the LSP client.

Action items (these are subject to change, as we go through our postmortem):

  • Release [email protected] containing the fix
  • Include an automated configuration diff in the gopls release process. This would have highlighted the new analyzer.
  • Add a meta test that sanity checks that all gopls analyzers are exercised by at least one test.
  • Consider redirecting os.Stdout in gopls, perhaps to os.Stderr.
  • Investigate whether we can improve the vscode-go error message when the jsonrpc2 stream gets corrupted.
@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 Aug 15, 2022
@gopherbot gopherbot added this to the Unreleased milestone Aug 15, 2022
@findleyr findleyr modified the milestones: Unreleased, gopls/later Aug 15, 2022
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Aug 22, 2022
@0-wiz-0
gopls: update to 0.9.4.
c151ff7 
This release fixes an unfortunate bug in a new vet analysis in the
[email protected] release. Specifically, a stray print statement in a
new analyzer for the invalid time format string "2006-02-01", which
corrupts gopls' communication over STDIN/STDOUT.

This only affects projects using that bad format string, and only
when a file in the affected package is open. Thanks to @Phadin for
accurately identifying this bug so quickly after it was introduced.
Issue golang/go#54459 tracks our follow-up to prevent similar
regressions from making it into future gopls releases.

On a positive note, here is the new vet analysis in action. Clearly
it will catch real bugs!
@findleyr findleyr mentioned this issue Sep 20, 2022
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/444799 mentions this issue: gopls/api-diff: simplify the api-diff implementation

gopherbot pushed a commit to golang/tools that referenced this issue Oct 31, 2022
@findleyr
gopls/api-diff: simplify the api-diff implementation
70a130e 
Simplify the api-diff implementation to use `go run` and cmp.Diff. The
latter is more maintainable and produces more readable output, due to
supporting line diffs for multi-line strings.

For golang/go#54459

Change-Id: I11c00e9728ce241aef8f9828f3840b4202294a9a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/444799
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
gopls-CI: kokoro <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
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/backlog
Development

No branches or pull requests
2 participants
@gopherbot @findleyr