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

[atombender]

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.

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/toml@v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
    github.com/sergi/go-diff@v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
    golang.org/x/mod@v0.1.1-0.20191105210325-c90efee705ee h1:WG0RUwxtNT4qqaXX3DPA8zHFNm/D9xaBpxzHt1WcA/E=
    golang.org/x/sync@v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
    golang.org/x/tools@v0.0.0-20200316194252-fafb6e2e8a4a => ../
    golang.org/x/xerrors@v0.0.0-20191011141410-1b5146add898 h1:/atklqdjdhuosWIl6AIbOeHJjicWYPqR9bpxqxYG2pA=
    honnef.co/go/tools@v0.0.1-2020.1.3 h1:sXmLre5bzIR6ypkjXCDI3jHPssRhc8KD/Ome589sc3U=
    mvdan.cc/xurls/v2@v2.1.0 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

[gopherbot]

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.

[stamblerre]

Thanks for the report! /cc @muirdm

[muirdm]

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?

[atombender]

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

[atombender]

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

[atombender]

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.

[muirdm]

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.

[atombender]

FWIW, GoLand does not have this issue.

[muirdm]

There are two steps to improving this in gopls:

1. 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.
2. 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?

[luciolas]

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

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{}