Go: x/tools/gopls: autocompletion is appended to typed word

Created on 22 Apr 2020 · 19Comments · Source: golang/go

What did you do?

In VSCode, typed part of a word to autocomplete it, e.g. cancel, then selected cancelCtx.

What did you expect to see?

I expected the current word to be cancelCtx.

What did you see instead?

Current word became canccancelCtx.

gopls

Build info

Also getting this behaviour with v0.4.0.

golang.org/x/tools/gopls v0.3.4
    golang.org/x/tools/gopls@(devel)
    github.com/BurntSushi/[email protected] h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
    github.com/sergi/[email protected] h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
    golang.org/x/[email protected] h1:WG0RUwxtNT4qqaXX3DPA8zHFNm/D9xaBpxzHt1WcA/E=
    golang.org/x/[email protected] h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
    golang.org/x/[email protected] => ../
    golang.org/x/[email protected] h1:/atklqdjdhuosWIl6AIbOeHJjicWYPqR9bpxqxYG2pA=
    honnef.co/go/[email protected] h1:sXmLre5bzIR6ypkjXCDI3jHPssRhc8KD/Ome589sc3U=
    mvdan.cc/xurls/[email protected] h1:KaMb5GLhlcSX+e+qhbRJODnUUBvlw01jt4yrjFIHAuA=

Go info

go version go1.14 darwin/amd64

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/alex/Library/Caches/go-build"
GOENV="/Users/alex/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GONOPROXY="github.com/t11e/*,github.com/sanity-io/gradient"
GONOSUMDB="github.com/t11e/*,github.com/sanity-io/gradient"
GOOS="darwin"
GOPATH="/Users/alex/.go"
GOPRIVATE="github.com/t11e/*,github.com/sanity-io/gradient"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/opt/go/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/opt/go/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/alex/Code/golang-tools/gopls/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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/qs/wpmg19r12_9_nz7pvvs2_82r0000gn/T/go-build188118376=/tmp/go-build -gno-record-gcc-switches -fno-common"

VSCode

Version: 1.44.2
Commit: ff915844119ce9485abfe8aa9076ec76b5300ddd
Date: 2020-04-16T17:07:18.473Z
Electron: 7.1.11
Chrome: 78.0.3904.130
Node.js: 12.8.1
V8: 7.8.279.23-electron.0
OS: Darwin x64 19.3.0

VSCode-Go 0.14.1.

Log

gopls.log.gz

Tools gopls help wanted

Source

atombender

👍2

Most helpful comment

From what I can tell, GoLand's parser is more lenient, and will recover from such parsing errors. I'm not able to make it break on unexpected tokens. For example, if you do:

b := bytes.Buffer{}:=
b2 := bytes.Buffer{}

then doesn't understand the type of b anymore, but it will happily continue to parse the rest of the file, and b2 will be autocompletable. Gopls just stops working.

Looking at how parser errors are phrased, it seems GoLand has its own parser, and doesn't reuse Go's.

atombender on 24 Apr 2020

❤1 🎉1 👍1

All 19 comments

Thank you for filing a gopls issue! Please take a look at the Troubleshooting guide, and make sure that you have provided all of the relevant information here.

gopherbot on 22 Apr 2020

Thanks for the report! /cc @muirdm

stamblerre on 22 Apr 2020

I think this is because incrementalAnalyzer{} is a syntax error without extra parents around it. The composite literal curlies are parsed as the if block curlies, so the AST is all messed up.

I'm not sure it would be easy to work around this type of syntax error. Maybe a code action to add in the missing parens?

muirdm on 22 Apr 2020

@muirdm: No, that's not a syntax error.

atombender on 22 Apr 2020

Ah, I see what you're saying. It's not valid syntax in an if statement without parens.

atombender on 22 Apr 2020

Looks like it generally happens in the presence of syntax errors. For example:

expectErr := proto.QueryParseError{Description: "No query"})  // <-- extra parenthesis

If you try to autocomplete expectErr after this code snippet, the same thing happens.

atombender on 23 Apr 2020

Yup, syntax errors wreak all sorts of havoc. We try to work around "expected" syntax errors you get as you type code (e.g. as you type if fooBa), but unexpected syntax errors will still prevent completion from working properly due to various reasons.

muirdm on 23 Apr 2020

FWIW, GoLand does not have this issue.

atombender on 23 Apr 2020

There are two steps to improving this in gopls:

Manually extract the completion prefix when there are syntax errors instead of depending on AST. This will fix all simple cases of "fooBa" completing to "fooBafooBar". We haven't done this because you will still get a degraded completion experience (e.g. gopls won't be able to determine the expected type), so we wanted to address common cases with special AST fixup logic so completion works optimally.
Manually parse and type check the surrounding expression. For example:

b := bytes.Buffer{}) // syntax error
b.Wr

To offer b.Write() properly, we need to manually extract the selector expression b and type check it (since it is not in the AST). It is easy in this case, but the left side of the selector can be an arbitrarily complex expression, so it is non-trivial to pick out out dependably. Out of curiosity, does GoLand handle selector expressions like this as well?

muirdm on 23 Apr 2020

@muirdm An example from GoLand. Is the GIF relevant to your question?

W0kJj9Rags

I'm not sure if I'm understanding it correctly, but in GoLang, it does seem like

b := bytes.Buffer{}) // syntax error

is valid in a sense

b := bytes.Buffer{}

luciolas on 24 Apr 2020

From what I can tell, GoLand's parser is more lenient, and will recover from such parsing errors. I'm not able to make it break on unexpected tokens. For example, if you do:

b := bytes.Buffer{}:=
b2 := bytes.Buffer{}

then doesn't understand the type of b anymore, but it will happily continue to parse the rest of the file, and b2 will be autocompletable. Gopls just stops working.

Looking at how parser errors are phrased, it seems GoLand has its own parser, and doesn't reuse Go's.

atombender on 24 Apr 2020

❤1 🎉1 👍1

Thanks. It makes sense that GoLand has its own parser since the standard library parser wasn't designed to work in the face of syntax errors.

muirdm on 24 Apr 2020

It makes for _any language server_ to have to its own parser. 😉 Editors aren't compilers, after all.

As an aside, this extends beyond just accidental syntax. Losing autocompletion occurs, of course, while just typing code. Here's an example where temporarily missing parantheses cause breakage:

gopls2

That's unrelated to this bug, but it shows how the current parser is a problem.

atombender on 24 Apr 2020

Similar bug (the file has a syntax error some lines below the cursor):

gif

atombender on 29 Apr 2020

Another example.
The first one (res.var) was ok, and then the second one (res.err) was messed up. This inconsistency makes the completion feature more confusing.

completion

hyangah on 19 Jun 2020

I'm experiencing the same issue when typing a comment for documenting a method/function. Typing the first few characters of the method/function name and pressing tab/enter afterwards used to autocomplete properly, but now it just appends the name to the already typed characters.

kostaskoukouvis on 24 Jun 2020

@kostaskoukouvis: That is https://github.com/golang/go/issues/39262.

stamblerre on 24 Jun 2020

👍1

@kostaskoukouvis: That is #39262.

Oops! My bad! :/

kostaskoukouvis on 26 Jun 2020

I'm experiencing the same issue here. The syntax error in my case was this statement:
if a, b _ = objobject.dosomething
When I removed the underscore, the autocorrect goes back to working as intended.