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>

gopls/internal/lsp/regtest/marker.go

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.

gopls/internal/lsp/source/completion/completion.go

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.

gopls/internal/regtest/marker/testdata/completion/issue59096.txt

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