internal/lsp/source: fix a couple issues completing append() args
In this example:
p := &[]int{}
append([]int{}, *<>)
At <> we completed to "**p" instead of "*p...". There were two fixes:
1. builtinArgType() wasn't propagating the "modifiers", so we were
forgetting about the preceding "*" pointer indirection and
inserting it again with the completion. Fix by propagating
modifiers.
2. The candidate formatting responsible for adding "..." had over
simplified logic to determine if we are completing the variadic
param. Now instead the candidate evaluation code marks the
candidate as "variadic" so the formatting doesn't have to think at
all.
Change-Id: Ib71ee8ecfafb915df331f1d2e55b76f76a530243
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248018
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/source/completion.go b/internal/lsp/source/completion.go
index bcf6c28..2955af6 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -422,6 +422,10 @@
// For example, dereference=2 turns "foo" into "**foo" when formatting.
dereference int
+ // variadic is true if this candidate fills a variadic param and
+ // needs "..." appended.
+ variadic bool
+
// imp is the import that needs to be added to this package in order
// for this candidate to be valid. nil if no import needed.
imp *importInfo
@@ -2278,11 +2282,15 @@
// candidate given the candidate inference. cand's score may be
// mutated to downrank the candidate in certain situations.
func (ci *candidateInference) candTypeMatches(cand *candidate) bool {
- expTypes := make([]types.Type, 0, 2)
+ var (
+ expTypes = make([]types.Type, 0, 2)
+ variadicType types.Type
+ )
if ci.objType != nil {
expTypes = append(expTypes, ci.objType)
if ci.variadic {
- expTypes = append(expTypes, types.NewSlice(ci.objType))
+ variadicType = types.NewSlice(ci.objType)
+ expTypes = append(expTypes, variadicType)
}
}
@@ -2327,6 +2335,10 @@
continue
}
+ if expType == variadicType {
+ cand.variadic = true
+ }
+
// Lower candidate score for untyped conversions. This avoids
// ranking untyped constants above candidates with an exact type
// match. Don't lower score of builtin constants, e.g. "true".
diff --git a/internal/lsp/source/completion_builtin.go b/internal/lsp/source/completion_builtin.go
index cfb004a..2ddb553 100644
--- a/internal/lsp/source/completion_builtin.go
+++ b/internal/lsp/source/completion_builtin.go
@@ -60,8 +60,11 @@
var (
exprIdx = exprAtPos(c.pos, call.Args)
- // Maintain any type name inference from our parent's context.
- inf = candidateInference{typeName: parentInf.typeName}
+ // Propagate certain properties from our parent's inference.
+ inf = candidateInference{
+ typeName: parentInf.typeName,
+ modifiers: parentInf.modifiers,
+ }
)
switch obj.Name() {
diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go
index ade85be..6dfa260 100644
--- a/internal/lsp/source/completion_format.go
+++ b/internal/lsp/source/completion_format.go
@@ -49,11 +49,6 @@
}
snip = c.functionCallSnippet(label, s.params)
detail = "func" + s.format()
-
- // Add variadic "..." if we are using a function result to fill in a variadic parameter.
- if sig.Results().Len() == 1 && c.inference.matchesVariadic(sig.Results().At(0).Type()) {
- snip.WriteText("...")
- }
return nil
}
@@ -83,12 +78,6 @@
return CompletionItem{}, err
}
}
-
- // Add variadic "..." if we are using a variable to fill in a variadic parameter.
- if c.inference.matchesVariadic(obj.Type()) {
- snip = &snippet.Builder{}
- snip.WriteText(insert + "...")
- }
case *types.Func:
sig, ok := obj.Type().Underlying().(*types.Signature)
if !ok {
@@ -155,6 +144,14 @@
label = prefixOp + label
}
+ // Add variadic "..." if we are filling in a variadic param.
+ if cand.variadic {
+ insert += "..."
+ if snip != nil {
+ snip.WriteText("...")
+ }
+ }
+
detail = strings.TrimPrefix(detail, "untyped ")
item := CompletionItem{
Label: label,
diff --git a/internal/lsp/testdata/lsp/primarymod/append/append.go b/internal/lsp/testdata/lsp/primarymod/append/append.go
index f8e64c5..228e856 100644
--- a/internal/lsp/testdata/lsp/primarymod/append/append.go
+++ b/internal/lsp/testdata/lsp/primarymod/append/append.go
@@ -26,4 +26,10 @@
var b struct{ b []baz }
b.b //@item(appendNestedBaz, "b.b", "[]baz", "field")
b.b = append(b.b, b) //@rank(")", appendBazzy, appendBazLiteral, appendNestedBaz)
+
+ var aStringsPtr *[]string //@item(appendStringsPtr, "aStringsPtr", "*[]string", "var")
+ "*aStringsPtr" //@item(appendStringsDeref, "*aStringsPtr", "*[]string", "var")
+ foo(append(nil, a)) //@snippet("))", appendStringsDeref, "*aStringsPtr...", "*aStringsPtr...")
+
+ foo(append(nil, *a)) //@snippet("))", appendStringsPtr, "aStringsPtr...", "aStringsPtr...")
}
diff --git a/internal/lsp/testdata/lsp/summary.txt.golden b/internal/lsp/testdata/lsp/summary.txt.golden
index a76174d..654d04c 100644
--- a/internal/lsp/testdata/lsp/summary.txt.golden
+++ b/internal/lsp/testdata/lsp/summary.txt.golden
@@ -2,7 +2,7 @@
CallHierarchyCount = 1
CodeLensCount = 5
CompletionsCount = 239
-CompletionSnippetCount = 83
+CompletionSnippetCount = 85
UnimportedCompletionsCount = 6
DeepCompletionsCount = 5
FuzzyCompletionsCount = 8
