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

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

Open
atombender opened this issue Apr 22, 2020 · 19 comments
Open

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

atombender opened this issue Apr 22, 2020 · 19 comments

Comments

@atombender
Copy link

@atombender atombender commented Apr 22, 2020

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/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 gopherbot added this to the Unreleased milestone Apr 22, 2020
@gopherbot gopherbot added the Tools label Apr 22, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Apr 22, 2020

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 gopherbot added the gopls label Apr 22, 2020
@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Apr 22, 2020

Thanks for the report! /cc @muirdm

@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.5.0 Apr 22, 2020
@muirdm
Copy link

@muirdm muirdm commented Apr 22, 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?

@atombender
Copy link
Author

@atombender atombender commented Apr 22, 2020

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

@atombender
Copy link
Author

@atombender atombender commented Apr 22, 2020

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

@atombender
Copy link
Author

@atombender atombender commented Apr 23, 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.

@muirdm
Copy link

@muirdm muirdm commented Apr 23, 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.

@atombender
Copy link
Author

@atombender atombender commented Apr 23, 2020

FWIW, GoLand does not have this issue.

@muirdm
Copy link

@muirdm muirdm commented Apr 23, 2020

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
Copy link

@luciolas luciolas commented Apr 24, 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{}

@atombender
Copy link
Author

@atombender atombender commented Apr 24, 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.

@muirdm
Copy link

@muirdm muirdm commented Apr 24, 2020

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.

@atombender
Copy link
Author

@atombender atombender commented Apr 24, 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.

@stamblerre stamblerre changed the title x/tools/gopls: Autocompletion is appended to typed word x/tools/gopls: autocompletion is appended to typed word Apr 28, 2020
@atombender
Copy link
Author

@atombender atombender commented Apr 28, 2020

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

gif

@hyangah
Copy link
Contributor

@hyangah hyangah commented Jun 19, 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

@stamblerre stamblerre removed this from the gopls/v0.5.0 milestone Jun 24, 2020
@kostaskoukouvis
Copy link

@kostaskoukouvis kostaskoukouvis commented Jun 24, 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.

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Jun 24, 2020

@kostaskoukouvis: That is #39262.

@kostaskoukouvis
Copy link

@kostaskoukouvis kostaskoukouvis commented Jun 26, 2020

@kostaskoukouvis: That is #39262.

Oops! My bad! :/

@jonathan-alvaro
Copy link

@jonathan-alvaro jonathan-alvaro commented Jul 17, 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.