internal/lsp: improve variadic completion
Improve candidate ranking when completing the variadic parameter of
function calls.
Using the example:
func foo(strs ...string) {}
- When completing foo(<>), we prefer candidates of type []string or
string (previously we only preferred []string).
- When completing foo("hi", <>), we prefer candidates of type
string (previously we preferred []string).
- When completing foo(<>), we use a snippet to add on the "..."
automatically to candidates of type []string.
I also fixed completion tests to work properly when you have multiple
notes referring to the same position. For example:
foo() //@rank(")", a, b),rank(")", a, c)
Previously the second "rank" was silently overwriting the first
because they both refer to the same ")".
Fixes golang/go#34334.
Change-Id: I4f64be44a4ccbb533fb7682738c759cbca3a93cd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205117
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index 5088762..edcffa5 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -1101,16 +1101,41 @@
if tv, ok := c.pkg.GetTypesInfo().Types[node.Fun]; ok {
if sig, ok := tv.Type.(*types.Signature); ok {
- if sig.Params().Len() == 0 {
+ numParams := sig.Params().Len()
+ if numParams == 0 {
return inf
}
- i := indexExprAtPos(c.pos, node.Args)
- // Make sure not to run past the end of expected parameters.
- if i >= sig.Params().Len() {
- i = sig.Params().Len() - 1
+
+ var (
+ exprIdx = indexExprAtPos(c.pos, node.Args)
+ isLastParam = exprIdx == numParams-1
+ beyondLastParam = exprIdx >= numParams
+ )
+
+ if sig.Variadic() {
+ // If we are beyond the last param or we are the last
+ // param w/ further expressions, we expect a single
+ // variadic item.
+ if beyondLastParam || isLastParam && len(node.Args) > numParams {
+ inf.objType = sig.Params().At(numParams - 1).Type().(*types.Slice).Elem()
+ break Nodes
+ }
+
+ // Otherwise if we are at the last param then we are
+ // completing the variadic positition (i.e. we expect a
+ // slice type []T or an individual item T).
+ if isLastParam {
+ inf.variadic = true
+ }
}
- inf.objType = sig.Params().At(i).Type()
- inf.variadic = sig.Variadic() && i == sig.Params().Len()-1
+
+ // Make sure not to run past the end of expected parameters.
+ if beyondLastParam {
+ inf.objType = sig.Params().At(numParams - 1).Type()
+ } else {
+ inf.objType = sig.Params().At(exprIdx).Type()
+ }
+
break Nodes
}
}
@@ -1225,6 +1250,12 @@
return typ
}
+// matchesVariadic returns true if we are completing a variadic
+// parameter and candType is a compatible slice type.
+func (ti typeInference) matchesVariadic(candType types.Type) bool {
+ return ti.variadic && types.AssignableTo(ti.objType, candType)
+}
+
// findSwitchStmt returns an *ast.CaseClause's corresponding *ast.SwitchStmt or
// *ast.TypeSwitchStmt. path should start from the case clause's first ancestor.
func findSwitchStmt(path []ast.Node, pos token.Pos, c *ast.CaseClause) ast.Stmt {
@@ -1443,6 +1474,13 @@
}
}
+ // When completing the variadic parameter, say objType matches if
+ // []objType matches. This is because you can use []T or T for the
+ // variadic parameter.
+ if c.expectedType.variadic && typeMatches(types.NewSlice(objType)) {
+ return true
+ }
+
if c.expectedType.convertibleTo != nil {
return types.ConvertibleTo(objType, c.expectedType.convertibleTo)
}
diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go
index 05b36b9..c6537f6 100644
--- a/internal/lsp/source/completion_format.go
+++ b/internal/lsp/source/completion_format.go
@@ -49,6 +49,11 @@
snip = c.functionCallSnippet(label, params)
results, writeParens := formatResults(sig.Results(), c.qf)
detail = "func" + formatFunction(params, results, writeParens)
+
+ // Add variadic "..." if we are using a function result to fill in a variadic parameter.
+ if sig.Results().Len() == 1 && c.expectedType.matchesVariadic(sig.Results().At(0).Type()) {
+ snip.WriteText("...")
+ }
}
switch obj := obj.(type) {
@@ -73,6 +78,12 @@
if sig, ok := obj.Type().Underlying().(*types.Signature); ok && cand.expandFuncCall {
expandFuncCall(sig)
}
+
+ // Add variadic "..." if we are using a variable to fill in a variadic parameter.
+ if c.expectedType.matchesVariadic(obj.Type()) {
+ snip = &snippet.Builder{}
+ snip.WriteText(insert + "...")
+ }
case *types.Func:
sig, ok := obj.Type().Underlying().(*types.Signature)
if !ok {
diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden
index 51af997..15fa98b 100644
--- a/internal/lsp/testdata/summary.txt.golden
+++ b/internal/lsp/testdata/summary.txt.golden
@@ -1,10 +1,10 @@
-- summary --
-CompletionsCount = 213
-CompletionSnippetCount = 40
+CompletionsCount = 214
+CompletionSnippetCount = 43
UnimportedCompletionsCount = 3
DeepCompletionsCount = 5
FuzzyCompletionsCount = 7
-RankedCompletionsCount = 8
+RankedCompletionsCount = 16
CaseSensitiveCompletionsCount = 4
DiagnosticsCount = 22
FoldingRangesCount = 2
diff --git a/internal/lsp/testdata/variadic/variadic.go.in b/internal/lsp/testdata/variadic/variadic.go.in
new file mode 100644
index 0000000..76e3a56
--- /dev/null
+++ b/internal/lsp/testdata/variadic/variadic.go.in
@@ -0,0 +1,23 @@
+package variadic
+
+func foo(i int, strs ...string) {}
+
+func bar() []string { //@item(vFunc, "bar", "func() []string", "func")
+ return nil
+}
+
+func _() {
+ var (
+ i int //@item(vInt, "i", "int", "var")
+ s string //@item(vStr, "s", "string", "var")
+ ss []string //@item(vStrSlice, "ss", "[]string", "var")
+ )
+
+ foo() //@rank(")", vInt, vStr),rank(")", vInt, vStrSlice)
+ foo(123, ) //@rank(")", vStr, vInt),rank(")", vStrSlice, vInt)
+ foo(123, "", ) //@rank(")", vStr, vInt),rank(")", vStr, vStrSlice)
+ foo(123, , "") //@rank(" ,", vStr, vInt),rank(")", vStr, vStrSlice)
+
+ // snippet will add the "..." for you
+ foo(123, ) //@snippet(")", vStrSlice, "ss...", "ss..."),snippet(")", vFunc, "bar()...", "bar()..."),snippet(")", vStr, "s", "s")
+}
diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go
index 4b47ff6..95d5a13 100644
--- a/internal/lsp/tests/tests.go
+++ b/internal/lsp/tests/tests.go
@@ -15,6 +15,7 @@
"io/ioutil"
"path/filepath"
"sort"
+ "strconv"
"strings"
"sync"
"testing"
@@ -39,13 +40,13 @@
type Diagnostics map[span.URI][]source.Diagnostic
type CompletionItems map[token.Pos]*source.CompletionItem
-type Completions map[span.Span]Completion
-type CompletionSnippets map[span.Span]CompletionSnippet
-type UnimportedCompletions map[span.Span]Completion
-type DeepCompletions map[span.Span]Completion
-type FuzzyCompletions map[span.Span]Completion
-type CaseSensitiveCompletions map[span.Span]Completion
-type RankCompletions map[span.Span]Completion
+type Completions map[span.Span][]Completion
+type CompletionSnippets map[span.Span][]CompletionSnippet
+type UnimportedCompletions map[span.Span][]Completion
+type DeepCompletions map[span.Span][]Completion
+type FuzzyCompletions map[span.Span][]Completion
+type CaseSensitiveCompletions map[span.Span][]Completion
+type RankCompletions map[span.Span][]Completion
type FoldingRanges []span.Span
type Formats []span.Span
type Imports []span.Span
@@ -343,80 +344,67 @@
t.Helper()
checkData(t, data)
+ eachCompletion := func(t *testing.T, cases map[span.Span][]Completion, test func(*testing.T, span.Span, Completion, CompletionItems)) {
+ t.Helper()
+
+ for src, exp := range cases {
+ for i, e := range exp {
+ t.Run(spanName(src)+"_"+strconv.Itoa(i), func(t *testing.T) {
+ t.Helper()
+ test(t, src, e, data.CompletionItems)
+ })
+ }
+
+ }
+ }
+
t.Run("Completion", func(t *testing.T) {
t.Helper()
- for src, test := range data.Completions {
- t.Run(spanName(src), func(t *testing.T) {
- t.Helper()
- tests.Completion(t, src, test, data.CompletionItems)
- })
- }
+ eachCompletion(t, data.Completions, tests.Completion)
})
t.Run("CompletionSnippets", func(t *testing.T) {
t.Helper()
for _, placeholders := range []bool{true, false} {
- for src, expected := range data.CompletionSnippets {
- name := spanName(src)
- if placeholders {
- name += "_placeholders"
+ for src, expecteds := range data.CompletionSnippets {
+ for i, expected := range expecteds {
+ name := spanName(src) + "_" + strconv.Itoa(i+1)
+ if placeholders {
+ name += "_placeholders"
+ }
+
+ t.Run(name, func(t *testing.T) {
+ t.Helper()
+ tests.CompletionSnippet(t, src, expected, placeholders, data.CompletionItems)
+ })
}
- t.Run(name, func(t *testing.T) {
- t.Helper()
- tests.CompletionSnippet(t, src, expected, placeholders, data.CompletionItems)
- })
}
}
})
t.Run("UnimportedCompletion", func(t *testing.T) {
t.Helper()
- for src, test := range data.UnimportedCompletions {
- t.Run(spanName(src), func(t *testing.T) {
- t.Helper()
- tests.UnimportedCompletion(t, src, test, data.CompletionItems)
- })
- }
+ eachCompletion(t, data.UnimportedCompletions, tests.UnimportedCompletion)
})
t.Run("DeepCompletion", func(t *testing.T) {
t.Helper()
- for src, test := range data.DeepCompletions {
- t.Run(spanName(src), func(t *testing.T) {
- t.Helper()
- tests.DeepCompletion(t, src, test, data.CompletionItems)
- })
- }
+ eachCompletion(t, data.DeepCompletions, tests.DeepCompletion)
})
t.Run("FuzzyCompletion", func(t *testing.T) {
t.Helper()
- for src, test := range data.FuzzyCompletions {
- t.Run(spanName(src), func(t *testing.T) {
- t.Helper()
- tests.FuzzyCompletion(t, src, test, data.CompletionItems)
- })
- }
+ eachCompletion(t, data.FuzzyCompletions, tests.FuzzyCompletion)
})
t.Run("CaseSensitiveCompletion", func(t *testing.T) {
t.Helper()
- for src, test := range data.CaseSensitiveCompletions {
- t.Run(spanName(src), func(t *testing.T) {
- t.Helper()
- tests.CaseSensitiveCompletion(t, src, test, data.CompletionItems)
- })
- }
+ eachCompletion(t, data.CaseSensitiveCompletions, tests.CaseSensitiveCompletion)
})
t.Run("RankCompletions", func(t *testing.T) {
t.Helper()
- for src, test := range data.RankCompletions {
- t.Run(spanName(src), func(t *testing.T) {
- t.Helper()
- tests.RankCompletion(t, src, test, data.CompletionItems)
- })
- }
+ eachCompletion(t, data.RankCompletions, tests.RankCompletion)
})
t.Run("Diagnostics", func(t *testing.T) {
@@ -594,13 +582,25 @@
}
}
- fmt.Fprintf(buf, "CompletionsCount = %v\n", len(data.Completions))
- fmt.Fprintf(buf, "CompletionSnippetCount = %v\n", len(data.CompletionSnippets))
- fmt.Fprintf(buf, "UnimportedCompletionsCount = %v\n", len(data.UnimportedCompletions))
- fmt.Fprintf(buf, "DeepCompletionsCount = %v\n", len(data.DeepCompletions))
- fmt.Fprintf(buf, "FuzzyCompletionsCount = %v\n", len(data.FuzzyCompletions))
- fmt.Fprintf(buf, "RankedCompletionsCount = %v\n", len(data.RankCompletions))
- fmt.Fprintf(buf, "CaseSensitiveCompletionsCount = %v\n", len(data.CaseSensitiveCompletions))
+ snippetCount := 0
+ for _, want := range data.CompletionSnippets {
+ snippetCount += len(want)
+ }
+
+ countCompletions := func(c map[span.Span][]Completion) (count int) {
+ for _, want := range c {
+ count += len(want)
+ }
+ return count
+ }
+
+ fmt.Fprintf(buf, "CompletionsCount = %v\n", countCompletions(data.Completions))
+ fmt.Fprintf(buf, "CompletionSnippetCount = %v\n", snippetCount)
+ fmt.Fprintf(buf, "UnimportedCompletionsCount = %v\n", countCompletions(data.UnimportedCompletions))
+ fmt.Fprintf(buf, "DeepCompletionsCount = %v\n", countCompletions(data.DeepCompletions))
+ fmt.Fprintf(buf, "FuzzyCompletionsCount = %v\n", countCompletions(data.FuzzyCompletions))
+ fmt.Fprintf(buf, "RankedCompletionsCount = %v\n", countCompletions(data.RankCompletions))
+ fmt.Fprintf(buf, "CaseSensitiveCompletionsCount = %v\n", countCompletions(data.CaseSensitiveCompletions))
fmt.Fprintf(buf, "DiagnosticsCount = %v\n", diagnosticsCount)
fmt.Fprintf(buf, "FoldingRangesCount = %v\n", len(data.FoldingRanges))
fmt.Fprintf(buf, "FormatCount = %v\n", len(data.Formats))
@@ -723,10 +723,10 @@
}
func (data *Data) collectCompletions(typ CompletionTestType) func(span.Span, []token.Pos) {
- result := func(m map[span.Span]Completion, src span.Span, expected []token.Pos) {
- m[src] = Completion{
+ result := func(m map[span.Span][]Completion, src span.Span, expected []token.Pos) {
+ m[src] = append(m[src], Completion{
CompletionItems: expected,
- }
+ })
}
switch typ {
case CompletionDeep:
@@ -896,11 +896,11 @@
}
func (data *Data) collectCompletionSnippets(spn span.Span, item token.Pos, plain, placeholder string) {
- data.CompletionSnippets[spn] = CompletionSnippet{
+ data.CompletionSnippets[spn] = append(data.CompletionSnippets[spn], CompletionSnippet{
CompletionItem: item,
PlainSnippet: plain,
PlaceholderSnippet: placeholder,
- }
+ })
}
func (data *Data) collectLinks(spn span.Span, link string, note *expect.Note, fset *token.FileSet) {
