internal/lsp: improve composite literal completion

When the value of a composite literal key/value pair was unparsable,
you were getting completions for the composite literal keys instead of
values. For example "struct { foo int }{foo: []<>" was completing to
the field name "foo". This was because the leaf ast.Node at the cursor
was the composite literal itself, and our go-back-one-character logic
was not happening because the preceding character's node
was *ast.BadExpr, not *ast.Ident. Fix by always generating the ast
path for the character before the cursor's position. I couldn't find
any cases where this broke completion.

I also added expected type detection for the following composite
literal cases:
- array/slice literals
- struct literals (both implicit and explicit field names)
- map keys and values

Fixes golang/go#29153

Change-Id: If8cf678cbd743a970f52893fcf4a9b83ea06d7e9
GitHub-Last-Rev: f385705cc05eb98132e20561451dbb8c39b68519
GitHub-Pull-Request: golang/tools#86
Reviewed-on: https://go-review.googlesource.com/c/tools/+/173099
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/lsp/lsp_test.go

@@ -162,9 +162,27 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, items tests.Co
 			t.Fatalf("completion failed for %v: %v", src, err)
 		}
 		if diff := diffCompletionItems(t, src, want, got); diff != "" {
-			t.Errorf(diff)
+			t.Errorf("%s: %s", src, diff)
 		}
 	}
+
+	// Make sure we don't crash completing the first position in file set.
+	firstFile := r.data.Config.Fset.Position(1).Filename
+
+	_, err := r.server.Completion(context.Background(), &protocol.CompletionParams{
+		TextDocumentPositionParams: protocol.TextDocumentPositionParams{
+			TextDocument: protocol.TextDocumentIdentifier{
+				URI: protocol.NewURI(span.FileURI(firstFile)),
+			},
+			Position: protocol.Position{
+				Line:      0,
+				Character: 0,
+			},
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
 }
 
 func isBuiltin(item protocol.CompletionItem) bool {

internal/lsp/source/completion.go

@@ -52,23 +52,15 @@ func Completion(ctx context.Context, f File, pos token.Pos) (items []CompletionI
 	if pkg.IsIllTyped() {
 		return nil, "", fmt.Errorf("package for %s is ill typed", f.URI())
 	}
-	path, _ := astutil.PathEnclosingInterval(file, pos, pos)
+
+	// Completion is based on what precedes the cursor.
+	// To understand what we are completing, find the path to the
+	// position before pos.
+	path, _ := astutil.PathEnclosingInterval(file, pos-1, pos-1)
 	if path == nil {
 		return nil, "", fmt.Errorf("cannot find node enclosing position")
 	}
 
-	// If the position is not an identifier but immediately follows
-	// an identifier or selector period (as is common when
-	// requesting a completion), use the path to the preceding node.
-	if _, ok := path[0].(*ast.Ident); !ok {
-		if p, _ := astutil.PathEnclosingInterval(file, pos-1, pos-1); p != nil {
-			switch p[0].(type) {
-			case *ast.Ident, *ast.SelectorExpr:
-				path = p // use preceding ident/selector
-			}
-		}
-	}
-
 	// Skip completion inside comment blocks or string literals.
 	switch lit := path[0].(type) {
 	case *ast.File, *ast.BlockStmt:
@@ -411,6 +403,43 @@ func complit(path []ast.Node, pos token.Pos, pkg *types.Package, info *types.Inf
 	return items, prefix, false
 }
 
+// enclosingCompLit returns the composite literal and key value expression, if
+// any, enclosing the given position.
+func enclosingCompLit(pos token.Pos, path []ast.Node) (*ast.CompositeLit, *ast.KeyValueExpr) {
+	var keyVal *ast.KeyValueExpr
+
+	for _, n := range path {
+		switch n := n.(type) {
+		case *ast.CompositeLit:
+			// pos isn't part of the composite literal unless it falls within the curly
+			// braces (e.g. "foo.Foo<>Struct{}").
+			if n.Lbrace <= pos && pos <= n.Rbrace {
+				if keyVal == nil {
+					if i := exprAtPos(pos, n.Elts); i < len(n.Elts) {
+						keyVal, _ = n.Elts[i].(*ast.KeyValueExpr)
+					}
+				}
+
+				return n, keyVal
+			}
+
+			return nil, nil
+		case *ast.KeyValueExpr:
+			keyVal = n
+		case *ast.FuncType, *ast.CallExpr, *ast.TypeAssertExpr:
+			// These node types break the type link between the leaf node and
+			// the composite literal. The type of the leaf node becomes unrelated
+			// to the type of the composite literal, so we return nil to avoid
+			// inappropriate completions. For example, "Foo{Bar: x.Baz(<>)}"
+			// should complete as a function argument to Baz, not part of the Foo
+			// composite literal.
+			return nil, nil
+		}
+	}
+
+	return nil, nil
+}
+
 // formatCompletion creates a completion item for a given types.Object.
 func formatCompletion(obj types.Object, qualifier types.Qualifier, score float64, isParam func(*types.Var) bool) CompletionItem {
 	label := obj.Name()
@@ -578,8 +607,77 @@ func enclosingFunction(path []ast.Node, pos token.Pos, info *types.Info) *types.
 	return nil
 }
 
+func expectedCompLitType(cl *ast.CompositeLit, kv *ast.KeyValueExpr, pos token.Pos, info *types.Info) types.Type {
+	// Get the type of the *ast.CompositeLit we belong to.
+	clType, ok := info.Types[cl]
+	if !ok {
+		return nil
+	}
+
+	switch t := clType.Type.Underlying().(type) {
+	case *types.Slice:
+		return t.Elem()
+	case *types.Array:
+		return t.Elem()
+	case *types.Map:
+		// If pos isn't in a key/value expression or it is on the left side
+		// of a key/value colon, a key must be entered next.
+		if kv == nil || pos <= kv.Colon {
+			return t.Key()
+		}
+
+		return t.Elem()
+	case *types.Struct:
+		// pos is in a key/value expression
+		if kv != nil {
+			// If pos is to left of the colon, it is a struct field name,
+			// so there is no expected type.
+			if pos <= kv.Colon {
+				return nil
+			}
+
+			if keyIdent, ok := kv.Key.(*ast.Ident); ok {
+				// Find the type of the struct field whose name matches the key.
+				for i := 0; i < t.NumFields(); i++ {
+					if field := t.Field(i); field.Name() == keyIdent.Name {
+						return field.Type()
+					}
+				}
+			}
+
+			return nil
+		}
+
+		hasKeys := false // true if the composite literal has any key/value pairs
+		for _, el := range cl.Elts {
+			if _, ok := el.(*ast.KeyValueExpr); ok {
+				hasKeys = true
+				break
+			}
+		}
+
+		// The struct literal is using field names, but pos is not in a key/value
+		// pair. A field name must be entered next, so there is no expected type.
+		if hasKeys {
+			return nil
+		}
+
+		// The order of the literal fields must match the order in the struct definition.
+		// Find the element pos falls in and use the corresponding field's type.
+		if i := exprAtPos(pos, cl.Elts); i < t.NumFields() {
+			return t.Field(i).Type()
+		}
+	}
+
+	return nil
+}
+
 // expectedType returns the expected type for an expression at the query position.
 func expectedType(path []ast.Node, pos token.Pos, info *types.Info) types.Type {
+	if compLit, keyVal := enclosingCompLit(pos, path); compLit != nil {
+		return expectedCompLitType(compLit, keyVal, pos, info)
+	}
+
 	for i, node := range path {
 		if i == 2 {
 			break

internal/lsp/testdata/complit/complit.go.in

@@ -5,19 +5,47 @@ type position struct { //@item(structPosition, "position", "struct{...}", "struc
 }
 
 func _() {
-	_ := position{
+	_ = position{
 		//@complete("", fieldX, fieldY, structPosition)
 	}
-	_ := position{
+	_ = position{
 		X: 1,
 		//@complete("", fieldY)
 	}
-	_ := position{
+	_ = position{
 		//@complete("", fieldX)
 		Y: 1,
 	}
 }
 
+func _() {
+	var (
+		aa string //@item(aaVar, "aa", "string", "var")
+		ab int    //@item(abVar, "ab", "int", "var")
+	)
+
+	_ = map[int]int{
+		a: a, //@complete(":", abVar, aaVar),complete(",", abVar, aaVar)
+	}
+
+	_ = map[int]int{
+		//@complete("", abVar, aaVar, structPosition)
+	}
+
+	_ = position{X: a} //@complete("}", abVar, aaVar)
+	_ = position{a}    //@complete("}", abVar, aaVar)
+
+	_ = []int{a}  //@complete("a", abVar, aaVar, structPosition)
+	_ = [1]int{a} //@complete("a", abVar, aaVar, structPosition)
+
+	var s struct {
+		AA int    //@item(fieldAA, "AA", "int", "field")
+		AB string //@item(fieldAB, "AB", "string", "field")
+	}
+	_ = map[int]string{1: "" + s.A} //@complete("}", fieldAB, fieldAA)
+	_ = map[int]string{1: (func(i int) string { return "" })(s.A)} //@complete(")}", fieldAA, fieldAB)
+}
+
 func _() {
 	_ := position{
 		X: 1, //@complete("X", fieldX),complete(" 1", structPosition)

internal/lsp/testdata/nested_complit/nested_complit.go.in

@@ -0,0 +1,13 @@
+package nested_complit
+
+type ncFoo struct {} //@item(structNCFoo, "ncFoo", "struct{...}", "struct")
+
+type ncBar struct { //@item(structNCBar, "ncBar", "struct{...}", "struct")
+	baz []ncFoo
+}
+
+func _() {
+	_ := ncBar{
+		baz: [] //@complete(" //", structNCBar, structNCFoo)
+	}
+}

internal/lsp/testdata/signature/signature.go

@@ -28,12 +28,12 @@ func Qux() {
 	Foo("foo", 123) //@signature(" 1", "Foo(a string, b int) (c bool)", 1)
 	Foo("foo", 123) //@signature(")", "Foo(a string, b int) (c bool)", 1)
 
-	Bar(13.37, 0x1337)     //@signature("13.37", "Bar(float64, ...byte)", 0)
-	Bar(13.37, 0x1337)     //@signature("0x1337", "Bar(float64, ...byte)", 1)
+	Bar(13.37, 0x13)       //@signature("13.37", "Bar(float64, ...byte)", 0)
+	Bar(13.37, 0x37)       //@signature("0x37", "Bar(float64, ...byte)", 1)
 	Bar(13.37, 1, 2, 3, 4) //@signature("4", "Bar(float64, ...byte)", 1)
 
 	fn := func(hi, there string) func(i int) rune {
-		return 0
+		return func(int) rune { return 0 }
 	}
 
 	fn("hi", "there")    //@signature("hi", "fn(hi string, there string) func(i int) rune", 0)
@@ -53,9 +53,10 @@ func Qux() {
 	var ms myStruct
 	ms.foo(nil) //@signature("nil", "foo(e *json.Decoder) (*big.Int, error)", 0)
 
-	panic("oops!")    //@signature("oops", "panic(interface{})", 0)
-	make([]int, 1, 2) //@signature("2", "make([]int, int, int) []int", 2)
+	_ = make([]int, 1, 2) //@signature("2", "make([]int, int, int) []int", 2)
 
 	Foo(myFunc(123), 456) //@signature("myFunc", "Foo(a string, b int) (c bool)", 0)
 	Foo(myFunc(123), 456) //@signature("123", "myFunc(foo int) string", 0)
+
+	panic("oops!") //@signature("oops", "panic(interface{})", 0)
 }

internal/lsp/tests/tests.go

@@ -27,7 +27,7 @@ import (
 // We hardcode the expected number of test cases to ensure that all tests
 // are being executed. If a test is added, this number must be changed.
 const (
-	ExpectedCompletionsCount     = 65
+	ExpectedCompletionsCount     = 75
 	ExpectedDiagnosticsCount     = 16
 	ExpectedFormatCount          = 4
 	ExpectedDefinitionsCount     = 21