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>
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index f45290d..2f6d8f1 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -162,9 +162,27 @@
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 {
diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index 38f1df7..3f70f85 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -52,23 +52,15 @@
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 @@
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 @@
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
diff --git a/internal/lsp/testdata/complit/complit.go.in b/internal/lsp/testdata/complit/complit.go.in
index 9c29c52..fec5aad 100644
--- a/internal/lsp/testdata/complit/complit.go.in
+++ b/internal/lsp/testdata/complit/complit.go.in
@@ -5,20 +5,48 @@
}
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)
Y: , //@complete(":", fieldY),complete(" ,", structPosition)
diff --git a/internal/lsp/testdata/nested_complit/nested_complit.go.in b/internal/lsp/testdata/nested_complit/nested_complit.go.in
new file mode 100644
index 0000000..3c3c88f
--- /dev/null
+++ b/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)
+ }
+}
diff --git a/internal/lsp/testdata/signature/signature.go b/internal/lsp/testdata/signature/signature.go
index 72351c4..7e826a0 100644
--- a/internal/lsp/testdata/signature/signature.go
+++ b/internal/lsp/testdata/signature/signature.go
@@ -28,12 +28,12 @@
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 @@
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)
}
diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go
index 196087d..bd36b82 100644
--- a/internal/lsp/tests/tests.go
+++ b/internal/lsp/tests/tests.go
@@ -27,7 +27,7 @@
// 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
