-
Notifications
You must be signed in to change notification settings - Fork 752
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
Folded code unfolds on syntax error #1224
Comments
@infinitepr0 Thanks for the report. @stamblerre This looks like a duplicate of golang/go#41281. I folded
gopls trace.
|
This is caused by the fundamental issue with the Go parser, which is that it doesn't recover well from syntax errors. This is the same reason that you won't get completion results for code that is below a syntax error. I believe @findleyr can speak to plans for parser improvements, but at the earliest, this could be fixed for Go 1.17. |
Thanks for the explanation. Maybe I am too naive - what will happen if gopls returns an error when the parse error is detected? I just tried to make FoldingRange query fail if pgf.ParseError != nil in https://github.com/golang/tools/blob/9eba6e1578c0e266628eefd47f0b750a6a63cf07/internal/lsp/source/folding_range.go#L23, and it seems like vscode preserves the folding range. But I am not sure what side-effect this would cause eventually.
|
Oh that's totally fine if you want to send that CL. I tried having it return empty results, but forgot about the error 😄 . The only possibility is that other LSP clients don't handle this gracefully and constantly pop-up an error to the user. |
Change https://golang.org/cl/291569 mentions this issue: |
This is something I encounter all the time. I often make some change that makes my code invalid until I'm done, and then edit code elsewhere in my file. A recent example I remember was editing log lines. I could have really used code completion for all the variables I wanted to add to |
Change https://golang.org/cl/299569 mentions this issue: |
In the presence of parse errors, gopls may fail to compute foldingRanges correctly and return only partial results. Partial results can confuse editors and cause to drop still valid, previously known folding ranges information. Currently, LSP does not allow a way for the server to tell clients the server is not ready for answering to the request. VSCode foldingRange API allows registered foldingRange provider to opt out of the folding range computation by providing undefined. https://code.visualstudio.com/api/references/vscode-api#FoldingRangeProvider "Returns a list of folding ranges or null and undefined if the provider does not want to participate or was cancelled." But, LSP currently does not disginguish undefined/null and empty results. Raising an error may be an option, but some LSP clients may not handle errors in user-friendly ways. Thus gopls devs want to avoid raising errors here. With https://go-review.googlesource.com/c/tools/+/291569, gopls will return an empty foldingRange response when encountering parse errors instead of returning incomplete results. This CL adds provideFoldingRanges in the middleware, that converts an empty foldingRange response with 'undefined' if the document is not empty. Manually tested. Testing it in the integration test setup is currently tricky until gopls with the change is released. Updates #1224 Updates golang/go#41281 Change-Id: I32cfb30d7e6a9874b9d1cb6c7e5309f19ee081e5 Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/299569 Trust: Hyang-Ah Hana Kim <hyangah@gmail.com> Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com> TryBot-Result: kokoro <noreply+kokoro@google.com> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When parse errors occur, go's parse package cannot recover nicely. gopls tried to compute folding ranges based on the partial info in this case, but returning partial folding range info confuses editors (vscode) and results in dropping previous folding range info from the region after the parse error location. This CL makes gopls not to return anything - so the editor can tell the result is not believable and ignore it. The ideal solution is to return a response explicitly surfacing this case, but currently LSP (3.16, as of today) does not have a way to describe this condition. See the discussion in microsoft/language-server-protocol#1200. We also tried to make gopls return an error. While it worked nicely in VSCode, we are not sure about how other editors handle errors from foldingRange. So, instead, we just let gopls return an empty result - since foldingRange is already broken in this case, we hope it doesn't add a lot of noise to existing users. VSCode Go will check the response from the middleware. If the response is empty but the file is not empty, VSCode Go will ignore the response. (https://go-review.googlesource.com/c/vscode-go/+/299569) Updates golang/vscode-go#1224 Updates golang/go#41281 Change-Id: I917d6667508aabbca1906137eb5e21a97a6cfdaf Reviewed-on: https://go-review.googlesource.com/c/tools/+/291569 Trust: Hyang-Ah Hana Kim <hyangah@gmail.com> Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.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/300290 mentions this issue: |
I believe the fix was released with v0.23.1 & gopls v0.6.7. |
It would appear that this error has returned. Currently on gopls v0.7.5, but I believe I updated a few versions last time, so I'm not sure when this was reintroduced. Please reopen issue. Here are the errors output to the console with filenames removed. The extra spaces in the first 2 are intentional
|
@thebenjhoward Can you please file a new bug report with a reproducer? |
What version of Go, VS Code & VS Code Go extension are you using?
go version
to get version of Go from the VS Code integrated terminal.go version go1.15.8 linux/amd64
gopls -v version
to get version of Gopls from the VS Code integrated terminal.code -v
orcode-insiders -v
to get version of VS Code or VS Code Insiders.0.22.1
Go: Locate Configured Go Tools
command.Describe the bug
Screenshots or recordings
Screencast.from.12-02-21.05_12_30.PM.IST.mp4
The text was updated successfully, but these errors were encountered: