diff --git a/internal/lsp/completion_test.go b/internal/lsp/completion_test.go index 04dc35e841..340dd1cc25 100644 --- a/internal/lsp/completion_test.go +++ b/internal/lsp/completion_test.go @@ -15,13 +15,10 @@ func (r *runner) Completion(t *testing.T, src span.Span, test tests.Completion, opts.DeepCompletion = false opts.Matcher = source.CaseInsensitive opts.UnimportedCompletion = false - opts.InsertTextFormat = protocol.PlainTextTextFormat - // Only enable literal completions if in the completion literals tests. - // TODO(rstambler): Separate out literal completion tests. - if strings.Contains(string(src.URI()), "literal") { - opts.InsertTextFormat = protocol.SnippetTextFormat + opts.InsertTextFormat = protocol.SnippetTextFormat + if !strings.Contains(string(src.URI()), "literal") { + opts.LiteralCompletions = false } - }) got = tests.FilterBuiltins(src, got) want := expected(t, test, items) diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index fe3ba16e17..d9f11b5e12 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -78,6 +78,10 @@ type CompletionItem struct { // Documentation is the documentation for the completion item. Documentation string + + // obj is the object from which this candidate was derived, if any. + // obj is for internal use only. + obj types.Object } // Snippet is a convenience returns the snippet if available, otherwise @@ -501,7 +505,7 @@ func Completion(ctx context.Context, snapshot Snapshot, fh FileHandle, protoPos documentation: opts.CompletionDocumentation, fullDocumentation: opts.HoverKind == FullDocumentation, placeholders: opts.Placeholders, - literal: opts.InsertTextFormat == protocol.SnippetTextFormat, + literal: opts.LiteralCompletions && opts.InsertTextFormat == protocol.SnippetTextFormat, budget: opts.CompletionBudget, }, // default to a matcher that always matches @@ -553,10 +557,6 @@ func Completion(ctx context.Context, snapshot Snapshot, fh FileHandle, protoPos return c.items, c.getSurrounding(), nil } - // Statement candidates offer an entire statement in certain - // contexts, as opposed to a single object. - c.addStatementCandidates() - if c.emptySwitchStmt() { // Empty switch statements only admit "default" and "case" keywords. c.addKeywordItems(map[string]bool{}, highScore, CASE, DEFAULT) @@ -570,9 +570,20 @@ func Completion(ctx context.Context, snapshot Snapshot, fh FileHandle, protoPos if err := c.selector(ctx, sel); err != nil { return nil, nil, err } - return c.items, c.getSurrounding(), nil - } - if err := c.lexical(ctx); err != nil { + } else if obj, ok := pkg.GetTypesInfo().Defs[n]; ok { + // reject defining identifiers + + if v, ok := obj.(*types.Var); ok && v.IsField() && v.Embedded() { + // An anonymous field is also a reference to a type. + } else { + objStr := "" + if obj != nil { + qual := types.RelativeTo(pkg.GetTypes()) + objStr = types.ObjectString(obj, qual) + } + return nil, nil, ErrIsDefinition{objStr: objStr} + } + } else if err := c.lexical(ctx); err != nil { return nil, nil, err } // The function name hasn't been typed yet, but the parens are there: @@ -599,6 +610,12 @@ func Completion(ctx context.Context, snapshot Snapshot, fh FileHandle, protoPos } } + // Statement candidates offer an entire statement in certain + // contexts, as opposed to a single object. Add statement candidates + // last because they depend on other candidates having already been + // collected. + c.addStatementCandidates() + return c.items, c.getSurrounding(), nil } diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index 2535432ace..a36bebd098 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -166,6 +166,7 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e Score: cand.score, Depth: len(c.deepState.chain), snippet: snip, + obj: obj, } // If the user doesn't want documentation for completion items. if !c.opts.documentation { diff --git a/internal/lsp/source/completion_statements.go b/internal/lsp/source/completion_statements.go index 780616850a..8f72c379ce 100644 --- a/internal/lsp/source/completion_statements.go +++ b/internal/lsp/source/completion_statements.go @@ -18,6 +18,147 @@ import ( // appropriate for the current context. func (c *completer) addStatementCandidates() { c.addErrCheckAndReturn() + c.addAssignAppend() +} + +// addAssignAppend offers a completion candidate of the form: +// +// someSlice = append(someSlice, ) +// +// It will offer the "append" completion in two situations: +// +// 1. Position is in RHS of assign, prefix matches "append", and +// corresponding LHS object is a slice. For example, +// "foo = ap<>" completes to "foo = append(foo, )". +// +// Or +// +// 2. Prefix is an ident or selector in an *ast.ExprStmt (i.e. +// beginning of statement), and our best matching candidate is a +// slice. For example: "foo.ba" completes to "foo.bar = append(foo.bar, )". +func (c *completer) addAssignAppend() { + if len(c.path) < 3 { + return + } + + ident, _ := c.path[0].(*ast.Ident) + if ident == nil { + return + } + + var ( + // sliceText is the full name of our slice object, e.g. "s.abc" in + // "s.abc = app<>". + sliceText string + // needsLHS is true if we need to prepend the LHS slice name and + // "=" to our candidate. + needsLHS = false + fset = c.snapshot.View().Session().Cache().FileSet() + ) + + switch n := c.path[1].(type) { + case *ast.AssignStmt: + // We are already in an assignment. Make sure our prefix matches "append". + if c.matcher.Score("append") <= 0 { + return + } + + exprIdx := exprAtPos(c.pos, n.Rhs) + if exprIdx == len(n.Rhs) || exprIdx > len(n.Lhs)-1 { + return + } + + lhsType := c.pkg.GetTypesInfo().TypeOf(n.Lhs[exprIdx]) + if lhsType == nil { + return + } + + // Make sure our corresponding LHS object is a slice. + if _, isSlice := lhsType.Underlying().(*types.Slice); !isSlice { + return + } + + // The name or our slice is whatever's in the LHS expression. + sliceText = formatNode(fset, n.Lhs[exprIdx]) + case *ast.SelectorExpr: + // Make sure we are a selector at the beginning of a statement. + if _, parentIsExprtStmt := c.path[2].(*ast.ExprStmt); !parentIsExprtStmt { + return + } + + // So far we only know the first part of our slice name. For + // example in "s.a<>" we only know our slice begins with "s." + // since the user could still be typing. + sliceText = formatNode(fset, n.X) + "." + needsLHS = true + case *ast.ExprStmt: + needsLHS = true + default: + return + } + + var ( + label string + snip snippet.Builder + score = highScore + ) + + if needsLHS { + // Offer the long form assign + append candidate if our best + // candidate is a slice. + bestItem := c.topCandidate() + if bestItem == nil || bestItem.obj == nil || bestItem.obj.Type() == nil { + return + } + + if _, isSlice := bestItem.obj.Type().Underlying().(*types.Slice); !isSlice { + return + } + + // Don't rank the full form assign + append candidate above the + // slice itself. + score = bestItem.Score - 0.01 + + // Fill in rest of sliceText now that we have the object name. + sliceText += bestItem.Label + + // Fill in the candidate's LHS bits. + label = fmt.Sprintf("%s = ", bestItem.Label) + snip.WriteText(label) + } + + snip.WriteText(fmt.Sprintf("append(%s, ", sliceText)) + snip.WritePlaceholder(nil) + snip.WriteText(")") + + c.items = append(c.items, CompletionItem{ + Label: label + fmt.Sprintf("append(%s, )", sliceText), + Kind: protocol.FunctionCompletion, + Score: score, + snippet: &snip, + }) +} + +// topCandidate returns the strictly highest scoring candidate +// collected so far. If the top two candidates have the same score, +// nil is returned. +func (c *completer) topCandidate() *CompletionItem { + var bestItem, secondBestItem *CompletionItem + for i := range c.items { + if bestItem == nil || c.items[i].Score > bestItem.Score { + bestItem = &c.items[i] + } else if secondBestItem == nil || c.items[i].Score > secondBestItem.Score { + secondBestItem = &c.items[i] + } + } + + // If secondBestItem has the same score, bestItem isn't + // the strict best. + if secondBestItem != nil && secondBestItem.Score == bestItem.Score { + return nil + } + + return bestItem } // addErrCheckAndReturn offers a completion candidate of the form: diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 655bbf994b..fd19ad0e78 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -119,7 +119,8 @@ func DefaultOptions() Options { }, }, DebuggingOptions: DebuggingOptions{ - CompletionBudget: 100 * time.Millisecond, + CompletionBudget: 100 * time.Millisecond, + LiteralCompletions: true, }, ExperimentalOptions: ExperimentalOptions{ TempModfile: true, @@ -272,6 +273,11 @@ type DebuggingOptions struct { // dynamically reduce the search scope to ensure we return timely // results. Zero means unlimited. CompletionBudget time.Duration + + // LiteralCompletions controls whether literal candidates such as + // "&someStruct{}" are offered. Tests disable this flag to simplify + // their expected values. + LiteralCompletions bool } type Matcher int diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 4a0e9dde07..b0e50d5a57 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -117,11 +117,9 @@ func (r *runner) Completion(t *testing.T, src span.Span, test tests.Completion, opts.Matcher = source.CaseInsensitive opts.DeepCompletion = false opts.UnimportedCompletion = false - opts.InsertTextFormat = protocol.PlainTextTextFormat - // Only enable literal completions if in the completion literals tests. - // TODO(rstambler): Separate out literal completion tests. - if strings.Contains(string(src.URI()), "literal") { - opts.InsertTextFormat = protocol.SnippetTextFormat + opts.InsertTextFormat = protocol.SnippetTextFormat + if !strings.Contains(string(src.URI()), "literal") { + opts.LiteralCompletions = false } }) got = tests.FilterBuiltins(src, got) diff --git a/internal/lsp/testdata/lsp/primarymod/statements/append.go b/internal/lsp/testdata/lsp/primarymod/statements/append.go new file mode 100644 index 0000000000..0eea85a282 --- /dev/null +++ b/internal/lsp/testdata/lsp/primarymod/statements/append.go @@ -0,0 +1,42 @@ +package statements + +func _() { + type mySlice []int + + var ( + abc []int //@item(stmtABC, "abc", "[]int", "var") + abcdef mySlice //@item(stmtABCDEF, "abcdef", "mySlice", "var") + ) + + /* abcdef = append(abcdef, ) */ //@item(stmtABCDEFAssignAppend, "abcdef = append(abcdef, )", "", "func") + + // don't offer "abc = append(abc, )" because "abc" isn't necessarily + // better than "abcdef". + abc //@complete(" //", stmtABC, stmtABCDEF) + + abcdef //@complete(" //", stmtABCDEF, stmtABCDEFAssignAppend) + + /* append(abc, ) */ //@item(stmtABCAppend, "append(abc, )", "", "func") + + abc = app //@snippet(" //", stmtABCAppend, "append(abc, ${1:})", "append(abc, ${1:})") +} + +func _() { + var s struct{ xyz []int } + + /* xyz = append(s.xyz, ) */ //@item(stmtXYZAppend, "xyz = append(s.xyz, )", "", "func") + + s.x //@snippet(" //", stmtXYZAppend, "xyz = append(s.xyz, ${1:})", "xyz = append(s.xyz, ${1:})") + + /* s.xyz = append(s.xyz, ) */ //@item(stmtDeepXYZAppend, "s.xyz = append(s.xyz, )", "", "func") + + sx //@snippet(" //", stmtDeepXYZAppend, "s.xyz = append(s.xyz, ${1:})", "s.xyz = append(s.xyz, ${1:})") +} + +func _() { + var foo [][]int + + /* append(foo[0], ) */ //@item(stmtFooAppend, "append(foo[0], )", "", "func") + + foo[0] = app //@complete(" //"),snippet(" //", stmtFooAppend, "append(foo[0], ${1:})", "append(foo[0], ${1:})") +} diff --git a/internal/lsp/testdata/lsp/summary.txt.golden b/internal/lsp/testdata/lsp/summary.txt.golden index 56d30df3ac..47c1e7b5b9 100644 --- a/internal/lsp/testdata/lsp/summary.txt.golden +++ b/internal/lsp/testdata/lsp/summary.txt.golden @@ -1,7 +1,7 @@ -- summary -- CodeLensCount = 4 -CompletionsCount = 241 -CompletionSnippetCount = 76 +CompletionsCount = 244 +CompletionSnippetCount = 80 UnimportedCompletionsCount = 6 DeepCompletionsCount = 5 FuzzyCompletionsCount = 8