gopls/internal/lsp/source: fix panic in quickParse
Fix a panic in quickParse resulting from a malformed synthetic selector
node created during completion.
Add a completion test in the new marker framework.
Fixes golang/go#59096
Change-Id: Icc3c3cec3084fb76aaecf88574bbd5cbee2a8afa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/477255
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
diff --git a/gopls/internal/lsp/regtest/marker.go b/gopls/internal/lsp/regtest/marker.go
index fb7454d..8b4c61e 100644
--- a/gopls/internal/lsp/regtest/marker.go
+++ b/gopls/internal/lsp/regtest/marker.go
@@ -22,6 +22,7 @@
"strings"
"testing"
+ "github.com/google/go-cmp/cmp"
"golang.org/x/tools/go/expect"
"golang.org/x/tools/gopls/internal/hooks"
"golang.org/x/tools/gopls/internal/lsp/cache"
@@ -115,6 +116,9 @@
//
// The following markers are supported within marker tests:
//
+// - complete(location, ...labels): specifies expected completion results at
+// the given location.
+//
// - diag(location, regexp): specifies an expected diagnostic matching the
// given regexp at the given location. The test runner requires
// a 1:1 correspondence between observed diagnostics and diag annotations
@@ -455,6 +459,7 @@
// Marker funcs should not mutate the test environment (e.g. via opening files
// or applying edits in the editor).
var markerFuncs = map[string]markerFunc{
+ "complete": makeMarkerFunc(completeMarker),
"def": makeMarkerFunc(defMarker),
"diag": makeMarkerFunc(diagMarker),
"hover": makeMarkerFunc(hoverMarker),
@@ -1091,6 +1096,26 @@
// ---- marker functions ----
+// completeMarker implements the @complete marker, running
+// textDocument/completion at the given src location and asserting that the
+// results match the expected results.
+//
+// TODO(rfindley): for now, this is just a quick check against the expected
+// completion labels. We could do more by assembling richer completion items,
+// as is done in the old marker tests. Does that add value? If so, perhaps we
+// should support a variant form of the argument, labelOrItem, which allows the
+// string form or item form.
+func completeMarker(mark marker, src protocol.Location, want ...string) {
+ list := mark.run.env.Completion(src)
+ var got []string
+ for _, item := range list.Items {
+ got = append(got, item.Label)
+ }
+ if diff := cmp.Diff(want, got); diff != "" {
+ mark.errorf("Completion(...) returned unexpect results (-want +got):\n%s", diff)
+ }
+}
+
// defMarker implements the @godef marker, running textDocument/definition at
// the given src location and asserting that there is exactly one resulting
// location, matching dst.
diff --git a/gopls/internal/lsp/source/completion/completion.go b/gopls/internal/lsp/source/completion/completion.go
index 315541e..e95c9eb 100644
--- a/gopls/internal/lsp/source/completion/completion.go
+++ b/gopls/internal/lsp/source/completion/completion.go
@@ -658,7 +658,11 @@
// recv.‸(arg)
case *ast.TypeAssertExpr:
// Create a fake selector expression.
- return c.selector(ctx, &ast.SelectorExpr{X: n.X})
+ //
+ // The name "_" is the convention used by go/parser to represent phantom
+ // selectors.
+ sel := &ast.Ident{NamePos: n.X.End() + token.Pos(len(".")), Name: "_"}
+ return c.selector(ctx, &ast.SelectorExpr{X: n.X, Sel: sel})
case *ast.SelectorExpr:
return c.selector(ctx, n)
// At the file scope, only keywords are allowed.
diff --git a/gopls/internal/regtest/marker/testdata/completion/issue59096.txt b/gopls/internal/regtest/marker/testdata/completion/issue59096.txt
new file mode 100644
index 0000000..44bb10a
--- /dev/null
+++ b/gopls/internal/regtest/marker/testdata/completion/issue59096.txt
@@ -0,0 +1,18 @@
+This test exercises the panic in golang/go#59096: completing at a syntactic
+type-assert expression was panicking because gopls was translating it into
+a (malformed) selector expr.
+
+-- go.mod --
+module example.com
+
+-- a/a.go --
+package a
+
+func _() {
+ b.(foo) //@complete(re"b.()", "B"), diag("b", re"(undefined|undeclared name): b")
+}
+
+-- b/b.go --
+package b
+
+const B = 0
