internal/lsp/source: improve completion in append()
In the following example:
var foo []someStruct
foo = append(foo, <>)
we now downrank "foo" as a candidate at "<>". You very rarely append a
slice to itself, so having "foo" ranked highly was counterproductive.
Fixes golang/go#40535.
Change-Id: Ic01366aeded4ba2b6b64bfddd33415499b35a323
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247519
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
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 340dd1c..c8992f5 100644
--- a/internal/lsp/completion_test.go
+++ b/internal/lsp/completion_test.go
@@ -96,6 +96,7 @@
opts.DeepCompletion = true
opts.Matcher = source.Fuzzy
opts.UnimportedCompletion = false
+ opts.LiteralCompletions = true
})
want := expected(t, test, items)
if msg := tests.CheckCompletionOrder(want, got, true); msg != "" {
diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index 64b63ec..bcf6c28 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -331,15 +331,8 @@
if c.matchingCandidate(&cand) {
cand.score *= highScore
- if c.seenInSwitchCase(&cand) {
- // If this candidate matches an expression already used in a
- // different case clause, downrank. We only downrank a little
- // because the user could write something like:
- //
- // switch foo {
- // case bar:
- // case ba<>: // they may want "bar|baz" or "bar.baz()"
- cand.score *= 0.9
+ if p := c.penalty(&cand); p > 0 {
+ cand.score *= (1 - p)
}
} else if isTypeName(obj) {
// If obj is a *types.TypeName that didn't otherwise match, check
@@ -387,16 +380,17 @@
c.deepSearch(ctx, cand)
}
-// seenInSwitchCase reports whether cand has already been seen in
-// another switch case statement.
-func (c *completer) seenInSwitchCase(cand *candidate) bool {
- for _, chain := range c.inference.seenSwitchCases {
- if c.objChainMatches(cand.obj, chain) {
- return true
+// penalty reports a score penalty for cand in the range (0, 1).
+// For example, a candidate is penalized if it has already been used
+// in another switch case statement.
+func (c *completer) penalty(cand *candidate) float64 {
+ for _, p := range c.inference.penalized {
+ if c.objChainMatches(cand.obj, p.objChain) {
+ return p.penalty
}
}
- return false
+ return 0
}
// candidate represents a completion candidate.
@@ -1556,6 +1550,16 @@
kindFunc
)
+// penalizedObj represents an object that should be disfavored as a
+// completion candidate.
+type penalizedObj struct {
+ // objChain is the full "chain", e.g. "foo.bar().baz" becomes
+ // []types.Object{foo, bar, baz}.
+ objChain []types.Object
+ // penalty is score penalty in the range (0, 1).
+ penalty float64
+}
+
// candidateInference holds information we have inferred about a type that can be
// used at the current position.
type candidateInference struct {
@@ -1602,13 +1606,12 @@
// foo(bar, baz<>) // variadicAssignees=false
variadicAssignees bool
- // seenSwitchCases tracks the expressions already used in the
- // containing switch statement's cases. Each expression is tracked
- // as a slice of objects. For example, "case foo.bar().baz:" is
- // tracked as []types.Object{foo, bar, baz}. Tracking the entire
- // "chain" allows us to differentiate "a.foo" and "b.foo" when "a"
- // and "b" are the same type.
- seenSwitchCases [][]types.Object
+ // penalized holds expressions that should be disfavored as
+ // candidates. For example, it tracks expressions already used in a
+ // switch statement's other cases. Each expression is tracked using
+ // its entire object "chain" allowing differentiation between
+ // "a.foo" and "b.foo" when "a" and "b" are the same type.
+ penalized []penalizedObj
// objChain contains the chain of objects representing the
// surrounding *ast.SelectorExpr. For example, if we are completing
@@ -1798,7 +1801,7 @@
}
if objs := objChain(c.pkg.GetTypesInfo(), caseExpr); len(objs) > 0 {
- inf.seenSwitchCases = append(inf.seenSwitchCases, objs)
+ inf.penalized = append(inf.penalized, penalizedObj{objChain: objs, penalty: 0.1})
}
}
}
diff --git a/internal/lsp/source/completion_builtin.go b/internal/lsp/source/completion_builtin.go
index 781cec3..cfb004a 100644
--- a/internal/lsp/source/completion_builtin.go
+++ b/internal/lsp/source/completion_builtin.go
@@ -72,10 +72,19 @@
inf.objType = parentInf.objType
- if exprIdx > 0 {
- inf.objType = deslice(inf.objType)
- // Check if we are completing the variadic append() param.
- inf.variadic = exprIdx == 1 && len(call.Args) <= 2
+ if exprIdx <= 0 {
+ break
+ }
+
+ inf.objType = deslice(inf.objType)
+
+ // Check if we are completing the variadic append() param.
+ inf.variadic = exprIdx == 1 && len(call.Args) <= 2
+
+ // Penalize the first append() argument as a candidate. You
+ // don't normally append a slice to itself.
+ if sliceChain := objChain(c.pkg.GetTypesInfo(), call.Args[0]); len(sliceChain) > 0 {
+ inf.penalized = append(inf.penalized, penalizedObj{objChain: sliceChain, penalty: 0.9})
}
case "delete":
if exprIdx > 0 && len(call.Args) > 0 {
diff --git a/internal/lsp/testdata/lsp/primarymod/append/append.go b/internal/lsp/testdata/lsp/primarymod/append/append.go
index 68cae67..f8e64c5 100644
--- a/internal/lsp/testdata/lsp/primarymod/append/append.go
+++ b/internal/lsp/testdata/lsp/primarymod/append/append.go
@@ -16,5 +16,14 @@
// Don't add "..." to append() argument.
bar(append()) //@snippet("))", appendStrings, "aStrings", "aStrings")
-}
+
+ type baz struct{}
+ baz{} //@item(appendBazLiteral, "baz{}", "", "var")
+ var bazzes []baz //@item(appendBazzes, "bazzes", "[]baz", "var")
+ var bazzy baz //@item(appendBazzy, "bazzy", "baz", "var")
+ bazzes = append(bazzes, ba) //@rank(")", appendBazzy, appendBazLiteral, appendBazzes)
+
+ var b struct{ b []baz }
+ b.b //@item(appendNestedBaz, "b.b", "[]baz", "field")
+ b.b = append(b.b, b) //@rank(")", appendBazzy, appendBazLiteral, appendNestedBaz)
}
diff --git a/internal/lsp/testdata/lsp/summary.txt.golden b/internal/lsp/testdata/lsp/summary.txt.golden
index 2ea9450..a76174d 100644
--- a/internal/lsp/testdata/lsp/summary.txt.golden
+++ b/internal/lsp/testdata/lsp/summary.txt.golden
@@ -6,7 +6,7 @@
UnimportedCompletionsCount = 6
DeepCompletionsCount = 5
FuzzyCompletionsCount = 8
-RankedCompletionsCount = 137
+RankedCompletionsCount = 139
CaseSensitiveCompletionsCount = 4
DiagnosticsCount = 44
FoldingRangesCount = 2
