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

Folded code unfolds on syntax error #1224

Closed
infalmo opened this issue Feb 12, 2021 · 12 comments
Closed

Folded code unfolds on syntax error #1224

infalmo opened this issue Feb 12, 2021 · 12 comments
Labels
FrozenDueToAge gopls gopls related issues upstream-tools Issues that are caused by problems in the tools that the extension depends on.
Milestone

Comments

@infalmo
Copy link

infalmo commented Feb 12, 2021

What version of Go, VS Code & VS Code Go extension are you using?

  • Run go version to get version of Go from the VS Code integrated terminal.
    go version go1.15.8 linux/amd64
  • Run gopls -v version to get version of Gopls from the VS Code integrated terminal.
Build info
----------
golang.org/x/tools/gopls v0.6.5
    golang.org/x/tools/gopls@v0.6.5 h1:kLt9rD/dWtVdvc8LmdcxagDFih6zxYXREpKSYYZu5KE=
    github.com/BurntSushi/toml@v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
    github.com/google/go-cmp@v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M=
    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    golang.org/x/mod@v0.4.0 h1:8pl+sMODzuvGJkmj2W4kZihvVb5mKm8pB/X44PIQHv8=
    golang.org/x/sync@v0.0.0-20201020160332-67f06af15bc9 h1:SQFwaSi55rU7vdNs9Yr0Z324VNlrF+0wMqRXT4St8ck=
    golang.org/x/sys@v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k=
    golang.org/x/tools@v0.1.1-0.20210201201750-4d4ee958a9b7 h1:/wdPW261t381NDQd8TBo63/FyvACfLICwtH8wMRoHJQ=
    golang.org/x/xerrors@v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
    honnef.co/go/tools@v0.0.1-2020.1.6 h1:W18jzjh8mfPez+AwGLxmOImucz/IFjpNlrKVnaj2YVc=
    mvdan.cc/gofumpt@v0.1.0 h1:hsVv+Y9UsZ/mFZTxJZuHVI6shSQCtzZ11h1JEFPAZLw=
    mvdan.cc/xurls/v2@v2.2.0 h1:NSZPykBXJFCetGZykLAxaL6SIpvbVy/UFEniIfHAa8A=
  • Run code -v or code-insiders -v to get version of VS Code or VS Code Insiders.
1.53.2
622cb03f7e070a9670c94bae1a45d78d7181fbd4
x64
  • Check your installed extensions to get the version of the VS Code Go extension
    0.22.1
  • Run Ctrl+Shift+P (Cmd+Shift+P on Mac OS) > Go: Locate Configured Go Tools command.
Checking configured tools....
GOBIN: undefined
toolsGopath: 
gopath: /home/infinitepro/go
GOROOT: /usr/lib/go
PATH: /home/infinitepro/.local/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl

   gopkgs: /home/infinitepro/go/bin/gopkgs installed
   go-outline: /home/infinitepro/go/bin/go-outline installed
   gotests: /home/infinitepro/go/bin/gotests installed
   gomodifytags: /home/infinitepro/go/bin/gomodifytags installed
   impl: /home/infinitepro/go/bin/impl installed
   goplay: /home/infinitepro/go/bin/goplay installed
   dlv: /home/infinitepro/go/bin/dlv installed
   golint: /home/infinitepro/go/bin/golint installed
   gopls: /home/infinitepro/go/bin/gopls installed

go env
Workspace Folder (cpt-lib): /home/infinitepro/Desktop/GitHub/cp-tools/cpt-lib
	GO111MODULE=""
	GOARCH="amd64"
	GOBIN=""
	GOCACHE="/home/infinitepro/.cache/go-build"
	GOENV="/home/infinitepro/.config/go/env"
	GOEXE=""
	GOFLAGS=""
	GOHOSTARCH="amd64"
	GOHOSTOS="linux"
	GOINSECURE=""
	GOMODCACHE="/home/infinitepro/go/pkg/mod"
	GONOPROXY=""
	GONOSUMDB=""
	GOOS="linux"
	GOPATH="/home/infinitepro/go"
	GOPRIVATE=""
	GOPROXY="https://proxy.golang.org,direct"
	GOROOT="/usr/lib/go"
	GOSUMDB="sum.golang.org"
	GOTMPDIR=""
	GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
	GCCGO="gccgo"
	AR="ar"
	CC="gcc"
	CXX="g++"
	CGO_ENABLED="1"
	GOMOD="/home/infinitepro/Desktop/GitHub/cp-tools/cpt-lib/go.mod"
	CGO_CFLAGS="-g -O2"
	CGO_CPPFLAGS=""
	CGO_CXXFLAGS="-g -O2"
	CGO_FFLAGS="-g -O2"
	CGO_LDFLAGS="-g -O2"
	PKG_CONFIG="pkg-config"
	GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build229485445=/tmp/go-build -gno-record-gcc-switches"

Describe the bug

  • Write some foldable code (loop, conditional statements etc).
  • Fold the code blocks.
  • Start writing a new line of code and stop midway (ensure there is a syntax error)
  • Have the previously folded code automatically unfolded.

Screenshots or recordings

Screencast.from.12-02-21.05_12_30.PM.IST.mp4
@hyangah
Copy link
Contributor

hyangah commented Feb 12, 2021

@infinitepr0 Thanks for the report.

@stamblerre This looks like a duplicate of golang/go#41281.

I folded F below. As soon as I entered if, gopls detected the incomplete statement as an error and sent out the folding range response that doesn't include ranges below the problematic statement. (message 9)
The issue 41281 is currently classified as 'unplanned'. Folding action is not stored and hard for users to recover the lost folding range when it is removed due to the missing foldingRange info caused by (transient) errors.

package main

func main() {
	for i := 0; i < 10; i++ {
		println(F())
	}
	if
}

// F is ... folded
func F() int {
	x++
	return x
}

gopls trace.

...

[Trace - 09:31:36.893 AM] Sending request 'textDocument/foldingRange - (7)'.
Params: {"textDocument":{"uri":"file:///Users/hakim/projects/google/w/main.go"}}


[Trace - 09:31:36.893 AM] Received response 'textDocument/foldingRange - (7)' in 0ms.
Result: [{"startLine":2,"startCharacter":13,"endLine":5,"endCharacter":2},{"startLine":3,"startCharacter":26,"endLine":4,"endCharacter":14},{"startLine":10,"startCharacter":14,"endLine":12,"endCharacter":9}]

..

[Trace - 09:31:37.042 AM] Sending notification 'textDocument/didChange'.
Params: {"textDocument":{"uri":"file:///Users/hakim/projects/google/w/main.go","version":3},"contentChanges":[{"range":{"start":{"line":6,"character":1},"end":{"line":6,"character":1}},"rangeLength":0,"text":"i"}]}


[Trace - 09:31:37.044 AM] Received notification 'textDocument/publishDiagnostics'.
Params: {"uri":"file:///Users/hakim/projects/google/w/main.go","version":3,"diagnostics":[{"range":{"start":{"line":6,"character":1},"end":{"line":6,"character":2}},"severity":1,"source":"compiler","message":"undeclared name: i"}]}


[Trace - 09:31:37.153 AM] Sending notification 'textDocument/didChange'.
Params: {"textDocument":{"uri":"file:///Users/hakim/projects/google/w/main.go","version":4},"contentChanges":[{"range":{"start":{"line":6,"character":2},"end":{"line":6,"character":2}},"rangeLength":0,"text":"f"}]}


[Trace - 09:31:37.156 AM] Received notification 'textDocument/publishDiagnostics'.
Params: {"uri":"file:///Users/hakim/projects/google/w/main.go","version":4,"diagnostics":[{"range":{"start":{"line":7,"character":0},"end":{"line":7,"character":0}},"severity":1,"source":"syntax","message":"expected operand, found '}'"}]}


[Trace - 09:31:37.351 AM] Sending request 'textDocument/foldingRange - (9)'.
Params: {"textDocument":{"uri":"file:///Users/hakim/projects/google/w/main.go"}}


[Trace - 09:31:37.352 AM] Received response 'textDocument/foldingRange - (9)' in 0ms.
Result: [{"startLine":3,"startCharacter":26,"endLine":4,"endCharacter":14}]

@stamblerre
Copy link
Contributor

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.

@hyangah
Copy link
Contributor

hyangah commented Feb 12, 2021

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.

[Trace - 11:19:57.825 AM] Sending request 'textDocument/foldingRange - (14)'.
Params: {"textDocument":{"uri":"file:///Users/hakim/projects/google/w/main.go"}}


[Error - Received] 11:19:57.826 AM #14 /Users/hakim/projects/google/w/main.go:13:1: expected operand, found '}' (and 3 more errors)


[Error - 11:19:57 AM] Request textDocument/foldingRange failed.
  Message: /Users/hakim/projects/google/w/main.go:13:1: expected operand, found '}' (and 3 more errors)
  Code: 0 
[Trace - 11:19:58.357 AM] Sending request 'textDocument/codeLens - (15)'.
Params: {"textDocument":{"uri":"file:///Users/hakim/projects/google/w/main.go"}}

@stamblerre
Copy link
Contributor

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.

@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/291569 mentions this issue: internal/lsp/source: error for foldingRange in case of parse error

@mattwelke
Copy link

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.

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 fmt.Printf's arguments after the string. Having code completion work even when I have a syntax error would be great.

@hyangah hyangah added gopls gopls related issues upstream-tools Issues that are caused by problems in the tools that the extension depends on. labels Feb 14, 2021
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/299569 mentions this issue: src/goLanguageServer: replace [] foldingRange response with undefined

gopherbot pushed a commit that referenced this issue Mar 8, 2021
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>
gopherbot pushed a commit to golang/tools that referenced this issue Mar 8, 2021
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>
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/300290 mentions this issue: [release] src/goLanguageServer: replace [] foldingRange response with undefined

@hyangah hyangah added this to the v0.23.1 milestone Mar 10, 2021
@hyangah
Copy link
Contributor

hyangah commented Apr 13, 2021

I believe the fix was released with v0.23.1 & gopls v0.6.7.
Please reopen or update the issue if it's still not working. Thanks!

@hyangah hyangah closed this as completed Apr 13, 2021
@mattwelke
Copy link

Fixed for me for latest go and gopls installed into VS Code today:

image

Stopping typing and saving both don't unfold code.

@thebenjhoward
Copy link

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

[Error - 4:17:36 PM] 2022/02/06 16:17:36 unable to compute positions for type errors: no parsed file for  in [redacted]
	package="[redacted]"

[Error - 4:17:37 PM] 2022/02/06 16:17:37 unable to compute positions for type errors: no parsed file for  in [redacted]
	package="[redacted]"

[Error - 4:17:37 PM] 2022/02/06 16:17:37 DocumentSymbols failed: invalid pos
	URI=file:///[redacted].go

@hyangah hyangah reopened this Feb 12, 2022
@hyangah
Copy link
Contributor

hyangah commented Feb 12, 2022

@thebenjhoward Can you please file a new bug report with a reproducer?
The maintainers don't read closed issues regularly, and it can be a different issue (for example, I see the example in #1224 (comment) works as expected, but some other cases not working as expected).

@hyangah hyangah closed this as completed Feb 12, 2022
@golang golang locked and limited conversation to collaborators Feb 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls gopls related issues upstream-tools Issues that are caused by problems in the tools that the extension depends on.
Projects
None yet
Development

No branches or pull requests

6 participants