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.

Sign up for GitHub

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

Jump to bottom

x/tools/gopls: TestXxx() completions not working when location followed by newline, due to parser error recovery #61371

Open
muchzill4 opened this issue Jul 15, 2023 · 2 comments
Open

x/tools/gopls: TestXxx() completions not working when location followed by newline, due to parser error recovery #61371

muchzill4 opened this issue Jul 15, 2023 · 2 comments
Labels
gopls/completion Issues related to auto-completion in gopls. gopls/parsing Issues related to parsing / poor parser recovery. gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.17.0

Comments

@muchzill4
Copy link

muchzill4 commented Jul 15, 2023 •
edited

gopls version

Affects versions >=0.12.0, including current master.

go env

GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/muchzill4/Library/Caches/go-build"
GOENV="/Users/muchzill4/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/muchzill4/.asdf/installs/golang/1.20.5/packages/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/muchzill4/.asdf/installs/golang/1.20.5/packages"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/muchzill4/.asdf/installs/golang/1.20.5/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/muchzill4/.asdf/installs/golang/1.20.5/go/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.20.5"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/muchzill4/Dev/vcs/go-tools/gopls/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/8b/9q2y2mm51yb6qmjm0pv9sjhm0000gn/T/go-build2972635718=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

When triggering completions in test files, such as:

package main

import "testing"

func T<caret, where completion should be triggered>

func TestSomething(t *testing.T) {
}

What did you expect to see?

TestXxx as one of the completions.

What did you see instead?

TestMain and Test

Editor and settings

n/a

Logs

[Trace - 11:49:34.710 AM] Sending request 'textDocument/completion - (581)'.
Params: {"textDocument":{"uri":"file:///Users/muchzill4/Dev/my/ttt/foo_test.go"},"position":{"line":4,"character":6},"context":{"triggerKind":1}}
...
[Trace - 11:49:34.711 AM] Received response 'textDocument/completion - (581)' in 1ms.
Result: {"isIncomplete":true,"items":[{"label":"TestMain","kind":3,"documentation":{"kind":"markdown","value":"complete the function name\n"},"preselect":true,"sortText":"00000","filterText":"TestMain","insertTextFormat":2,"textEdit":{"range":{"start":{"line":4,"character":5},"end":{"line":4,"character":6}},"newText":"TestMain"}},{"label":"Test","kind":3,"documentation":{"kind":"markdown","value":"complete the function name\n"},"sortText":"00001","filterText":"Test","insertTextFormat":2,"textEdit":{"range":{"start":{"line":4,"character":5},"end":{"line":4,"character":6}},"newText":"Test"}}]}

Debugging journey

My debugging indicates that this started happening since https://go-review.googlesource.com/c/tools/+/459559.
It seems that t.Closing != t.Opening unless the completion is triggered at the end of the file.

To replicate with existing tests:

diff --git a/gopls/internal/regtest/completion/completion_test.go b/gopls/internal/regtest/completion/completion_test.go
index 117e940e0..e3e62b47e 100644
--- a/gopls/internal/regtest/completion/completion_test.go
+++ b/gopls/internal/regtest/completion/completion_test.go
@@ -729,7 +729,7 @@ package foo
 		env.OpenFile(fname)
 		env.Await(env.DoneWithOpen())
 		for _, test := range tests {
-			env.SetBufferContent(fname, "package foo\n"+test.line)
+			env.SetBufferContent(fname, "package foo\n"+test.line+"\n")
 			loc := env.RegexpSearch(fname, test.pat)
 			loc.Range.Start.Character += uint32(protocol.UTF16Len([]byte(test.pat)))
 			completions := env.Completion(loc)
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jul 15, 2023
@gopherbot gopherbot added this to the Unreleased milestone Jul 15, 2023
@jamalc jamalc modified the milestones: Unreleased, gopls/v0.12.5 Jul 20, 2023
@jamalc jamalc added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 20, 2023
@adonovan
Copy link
Member

This is yet another case of poor parser error recovery. The parser parses the input above to

0 *ast.FuncDecl {
1 . Doc: nil
2 . Recv: nil
3 . Name: *ast.Ident {
4 . . NamePos: play.go:5:6
5 . . Name: "T"
6 . . Obj: *ast.Object {
7 . . . Kind: func
8 . . . Name: "T"
9 . . . Decl: *(obj @ 0)
10 . . . Data: nil
11 . . . Type: nil
12 . . }
13 . }
14 . Type: *ast.FuncType {
15 . . Func: play.go:5:1
16 . . TypeParams: nil
17 . . Params: *ast.FieldList {
18 . . . Opening: play.go:5:7
19 . . . List: []*ast.Field (len = 1) {
20 . . . . 0: *ast.Field {
21 . . . . . Doc: nil
22 . . . . . Names: nil
23 . . . . . Type: *ast.FuncType {
24 . . . . . . Func: play.go:7:1
25 . . . . . . TypeParams: nil
26 . . . . . . Params: *ast.FieldList {
27 . . . . . . . Opening: play.go:7:6
28 . . . . . . . List: []*ast.Field (len = 1) {
29 . . . . . . . . 0: *ast.Field {
30 . . . . . . . . . Doc: nil
31 . . . . . . . . . Names: nil
32 . . . . . . . . . Type: *ast.ParenExpr {
33 . . . . . . . . . . Lparen: play.go:7:19
34 . . . . . . . . . . X: *ast.Ident {
35 . . . . . . . . . . . NamePos: play.go:7:20
36 . . . . . . . . . . . Name: "t"
37 . . . . . . . . . . . Obj: nil
38 . . . . . . . . . . }
39 . . . . . . . . . . Rparen: play.go:7:22
40 . . . . . . . . . }
41 . . . . . . . . . Tag: nil
42 . . . . . . . . . Comment: nil
43 . . . . . . . . }
44 . . . . . . . }
45 . . . . . . . Closing: play.go:7:32
46 . . . . . . }
47 . . . . . . Results: nil
48 . . . . . }
49 . . . . . Tag: nil
50 . . . . . Comment: nil
51 . . . . }
52 . . . }
53 . . . Closing: play.go:8:2
54 . . }
55 . . Results: nil
56 . }
57 . Body: nil
58 }

which pretty-prints to:

func T(

func(t),
)

In other words, the first func decl swallowed the second.

See also: gopls/parsing Issues related to parsing / poor parser recovery.

@adonovan adonovan added the gopls/parsing Issues related to parsing / poor parser recovery. label Jul 25, 2023
@findleyr
Copy link
Contributor

Thanks for investigating. With forward compatibility in particular, I think we should fix this upstream (in go/parser).

That will take longer. Moving this to the next milestone.

@findleyr findleyr modified the milestones: gopls/v0.12.5, gopls/v0.13.0 Jul 25, 2023
@findleyr findleyr modified the milestones: gopls/v0.14.0, gopls/v0.15.0 Sep 27, 2023
@adonovan adonovan changed the title x/tools/gopls: TestXxx() completions not working when location followed by newline x/tools/gopls: TestXxx() completions not working when location followed by newline, due to parser error recovery Dec 1, 2023
@adonovan adonovan added the gopls/completion Issues related to auto-completion in gopls. label Dec 1, 2023
@findleyr findleyr modified the milestones: gopls/v0.15.0, gopls/v0.16.0 Dec 12, 2023
@findleyr findleyr modified the milestones: gopls/v0.16.0, gopls/v0.17.0 Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
gopls/completion Issues related to auto-completion in gopls. gopls/parsing Issues related to parsing / poor parser recovery. gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.17.0
Development

No branches or pull requests
5 participants
@muchzill4 @adonovan @gopherbot @jamalc @findleyr