internal/lsp/source: offer smart "append()" completions
Assigning a slice to the appendage of itself is common and tedious
enough to warrant a special case completion candidate. We now offer
smarter "append()" candidates:
var foo []int
foo = app<> // offer "append(foo, <>)"
fo<> // offer "foo = append(foo, <>)"
The latter is only offered if the best completion candidate is a
slice. It is inserted as the second-best candidate because it seems
impossible to avoid annoying false positives if it is ranked first.
I added a new debug option to disable literal completions. This was to
clean up some test logic that was disabling snippets for all tests
just to defeat literal completions. My tests were failing mysteriously
due to having snippets disabled, and it was hard to figure out why.
Change-Id: I3e8313e00a1409840cb99d5d71c593435a7aeb71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221777
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/completion_test.go b/internal/lsp/completion_test.go
index 04dc35e..340dd1c 100644
--- a/internal/lsp/completion_test.go
+++ b/internal/lsp/completion_test.go
@@ -15,13 +15,10 @@
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 fe3ba16..d9f11b5 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -78,6 +78,10 @@
// 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 @@
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 @@
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 @@
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 @@
}
}
+ // 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 2535432..a36bebd 100644
--- a/internal/lsp/source/completion_format.go
+++ b/internal/lsp/source/completion_format.go
@@ -166,6 +166,7 @@
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 7806168..8f72c37 100644
--- a/internal/lsp/source/completion_statements.go
+++ b/internal/lsp/source/completion_statements.go
@@ -18,6 +18,147 @@
// 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 655bbf9..fd19ad0 100644
--- a/internal/lsp/source/options.go
+++ b/internal/lsp/source/options.go
@@ -119,7 +119,8 @@
},
},
DebuggingOptions: DebuggingOptions{
- CompletionBudget: 100 * time.Millisecond,
+ CompletionBudget: 100 * time.Millisecond,
+ LiteralCompletions: true,
},
ExperimentalOptions: ExperimentalOptions{
TempModfile: true,
@@ -272,6 +273,11 @@
// 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 4a0e9dd..b0e50d5 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -117,11 +117,9 @@
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 0000000..0eea85a
--- /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 56d30df..47c1e7b 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
