diff --git a/internal/lsp/completion_test.go b/internal/lsp/completion_test.go index 6fdffa1fb1..664e075901 100644 --- a/internal/lsp/completion_test.go +++ b/internal/lsp/completion_test.go @@ -49,7 +49,7 @@ func (r *runner) UnimportedCompletion(t *testing.T, src span.Span, test tests.Co got = tests.FilterBuiltins(got) } want := expected(t, test, items) - if diff := tests.CheckCompletionOrder(want, got); diff != "" { + if diff := tests.CheckCompletionOrder(want, got, false); diff != "" { t.Errorf("%s: %s", src, diff) } } @@ -101,7 +101,7 @@ func (r *runner) RankCompletion(t *testing.T, src span.Span, test tests.Completi Deep: true, }) want := expected(t, test, items) - if msg := tests.CheckCompletionOrder(want, got); msg != "" { + if msg := tests.CheckCompletionOrder(want, got, true); msg != "" { t.Errorf("%s: %s", src, msg) } } diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 82230c9a0f..de26587c67 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -1462,8 +1462,8 @@ func (c *completer) matchingCandidate(cand *candidate) bool { return false } - objType := cand.obj.Type() - if objType == nil { + candType := cand.obj.Type() + if candType == nil { return true } @@ -1472,17 +1472,20 @@ func (c *completer) matchingCandidate(cand *candidate) bool { // are invoked by default. cand.expandFuncCall = isFunc(cand.obj) - typeMatches := func(candType types.Type) bool { + typeMatches := func(expType, candType types.Type) bool { + if expType == nil { + return false + } + // Take into account any type modifiers on the expected type. candType = c.expectedType.applyTypeModifiers(candType) - if c.expectedType.objType != nil { - wantType := types.Default(c.expectedType.objType) - - // Handle untyped values specially since AssignableTo gives false negatives - // for them (see https://golang.org/issue/32146). - if candBasic, ok := candType.(*types.Basic); ok && candBasic.Info()&types.IsUntyped > 0 { - if wantBasic, ok := wantType.Underlying().(*types.Basic); ok { + // Handle untyped values specially since AssignableTo gives false negatives + // for them (see https://golang.org/issue/32146). + if candBasic, ok := candType.Underlying().(*types.Basic); ok { + if wantBasic, ok := expType.Underlying().(*types.Basic); ok { + // Make sure at least one of them is untyped. + if isUntyped(candType) || isUntyped(expType) { // Check that their constant kind (bool|int|float|complex|string) matches. // This doesn't take into account the constant value, so there will be some // false positives due to integer sign and overflow. @@ -1490,32 +1493,29 @@ func (c *completer) matchingCandidate(cand *candidate) bool { // Lower candidate score if the types are not identical. // This avoids ranking untyped integer constants above // candidates with an exact type match. - if !types.Identical(candType, c.expectedType.objType) { + if !types.Identical(candType, expType) { cand.score /= 2 } return true } } - return false } - - // AssignableTo covers the case where the types are equal, but also handles - // cases like assigning a concrete type to an interface type. - return types.AssignableTo(candType, wantType) } - return false + // AssignableTo covers the case where the types are equal, but also handles + // cases like assigning a concrete type to an interface type. + return types.AssignableTo(candType, expType) } - if typeMatches(objType) { + if typeMatches(c.expectedType.objType, candType) { // If obj's type matches, we don't want to expand to an invocation of obj. cand.expandFuncCall = false return true } // Try using a function's return type as its type. - if sig, ok := objType.Underlying().(*types.Signature); ok && sig.Results().Len() == 1 { - if typeMatches(sig.Results().At(0).Type()) { + if sig, ok := candType.Underlying().(*types.Signature); ok && sig.Results().Len() == 1 { + if typeMatches(c.expectedType.objType, sig.Results().At(0).Type()) { // If obj's return value matches the expected type, we need to invoke obj // in the completion. cand.expandFuncCall = true @@ -1523,15 +1523,16 @@ func (c *completer) matchingCandidate(cand *candidate) bool { } } - // 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 + // When completing the variadic parameter, if the expected type is + // []T then check candType against T. + if c.expectedType.variadic { + if slice, ok := c.expectedType.objType.(*types.Slice); ok && typeMatches(slice.Elem(), candType) { + return true + } } if c.expectedType.convertibleTo != nil { - return types.ConvertibleTo(objType, c.expectedType.convertibleTo) + return types.ConvertibleTo(candType, c.expectedType.convertibleTo) } return false diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 3ed53a50d0..2e2b0fd81c 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -138,7 +138,7 @@ func (r *runner) UnimportedCompletion(t *testing.T, src span.Span, test tests.Co if !strings.Contains(string(src.URI()), "builtins") { got = tests.FilterBuiltins(got) } - if diff := tests.CheckCompletionOrder(want, got); diff != "" { + if diff := tests.CheckCompletionOrder(want, got, false); diff != "" { t.Errorf("%s: %s", src, diff) } } @@ -229,7 +229,7 @@ func (r *runner) RankCompletion(t *testing.T, src span.Span, test tests.Completi } got = append(got, item) } - if msg := tests.CheckCompletionOrder(want, got); msg != "" { + if msg := tests.CheckCompletionOrder(want, got, true); msg != "" { t.Errorf("%s: %s", src, msg) } } diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index 4e7138592b..68df3fb7fb 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -330,6 +330,13 @@ func isEmptyInterface(T types.Type) bool { return intf != nil && intf.NumMethods() == 0 } +func isUntyped(T types.Type) bool { + if basic, ok := T.(*types.Basic); ok { + return basic.Info()&types.IsUntyped > 0 + } + return false +} + // isSelector returns the enclosing *ast.SelectorExpr when pos is in the // selector. func enclosingSelector(path []ast.Node, pos token.Pos) *ast.SelectorExpr { diff --git a/internal/lsp/testdata/deep/deep.go b/internal/lsp/testdata/deep/deep.go index df70c14725..1bedc9416c 100644 --- a/internal/lsp/testdata/deep/deep.go +++ b/internal/lsp/testdata/deep/deep.go @@ -29,7 +29,7 @@ func _() { context.Background() //@item(ctxBackground, "context.Background", "func() context.Context", "func", "Background returns a non-nil, empty Context.") context.TODO() //@item(ctxTODO, "context.TODO", "func() context.Context", "func", "TODO returns a non-nil, empty Context.") - wantsContext(c) //@rank(")", ctxBackground, ctxTODO) + wantsContext(c) //@rank(")", ctxBackground),rank(")", ctxTODO) } func _() { diff --git a/internal/lsp/testdata/rank/convert_rank.go.in b/internal/lsp/testdata/rank/convert_rank.go.in index ac17416260..77e2d27d0a 100644 --- a/internal/lsp/testdata/rank/convert_rank.go.in +++ b/internal/lsp/testdata/rank/convert_rank.go.in @@ -30,7 +30,7 @@ func _() { mi = conv //@rank(" //", convertG, convertD, convertE) mi + conv //@rank(" //", convertG, convertD, convertE) - 1 + conv //@rank(" //", convertD, convertE, convertG) + 1 + conv //@rank(" //", convertD, convertC),rank(" //", convertE, convertC),rank(" //", convertG, convertC) type myString string var ms myString diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden index 19e9ad33cd..682605b004 100644 --- a/internal/lsp/testdata/summary.txt.golden +++ b/internal/lsp/testdata/summary.txt.golden @@ -4,7 +4,7 @@ CompletionSnippetCount = 47 UnimportedCompletionsCount = 3 DeepCompletionsCount = 5 FuzzyCompletionsCount = 7 -RankedCompletionsCount = 22 +RankedCompletionsCount = 26 CaseSensitiveCompletionsCount = 4 DiagnosticsCount = 22 FoldingRangesCount = 2 diff --git a/internal/lsp/testdata/variadic/variadic.go.in b/internal/lsp/testdata/variadic/variadic.go.in index 76e3a56655..8a7fa241fe 100644 --- a/internal/lsp/testdata/variadic/variadic.go.in +++ b/internal/lsp/testdata/variadic/variadic.go.in @@ -13,10 +13,10 @@ func _() { 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) + 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, s, "") //@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/testdata/variadic/variadic_intf.go b/internal/lsp/testdata/variadic/variadic_intf.go new file mode 100644 index 0000000000..6e23fc9960 --- /dev/null +++ b/internal/lsp/testdata/variadic/variadic_intf.go @@ -0,0 +1,21 @@ +package variadic + +type baz interface { + baz() +} + +func wantsBaz(...baz) {} + +type bazImpl int + +func (bazImpl) baz() {} + +func _() { + var ( + impls []bazImpl //@item(vImplSlice, "impls", "[]bazImpl", "var") + impl bazImpl //@item(vImpl, "impl", "bazImpl", "var") + bazes []baz //@item(vIntfSlice, "bazes", "[]baz", "var") + ) + + wantsBaz() //@rank(")", vImpl, vImplSlice),rank(")", vIntfSlice, vImplSlice) +} diff --git a/internal/lsp/tests/completion.go b/internal/lsp/tests/completion.go index 8936e506b0..ba5ec70f24 100644 --- a/internal/lsp/tests/completion.go +++ b/internal/lsp/tests/completion.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "sort" + "strconv" "strings" "golang.org/x/tools/internal/lsp/protocol" @@ -28,6 +29,8 @@ func ToProtocolCompletionItem(item source.CompletionItem) protocol.CompletionIte TextEdit: &protocol.TextEdit{ NewText: item.Snippet(), }, + // Negate score so best score has lowest sort text like real API. + SortText: fmt.Sprint(-item.Score), } if pItem.InsertText == "" { pItem.InsertText = pItem.Label @@ -64,11 +67,13 @@ func isBuiltin(label, detail string, kind protocol.CompletionItemKind) bool { return false } -func CheckCompletionOrder(want, got []protocol.CompletionItem) string { +func CheckCompletionOrder(want, got []protocol.CompletionItem, strictScores bool) string { var ( matchedIdxs []int lastGotIdx int + lastGotSort float64 inOrder = true + errorMsg = "completions out of order" ) for _, w := range want { var found bool @@ -76,10 +81,19 @@ func CheckCompletionOrder(want, got []protocol.CompletionItem) string { if w.Label == g.Label && w.Detail == g.Detail && w.Kind == g.Kind { matchedIdxs = append(matchedIdxs, i) found = true + if i < lastGotIdx { inOrder = false } lastGotIdx = i + + sort, _ := strconv.ParseFloat(g.SortText, 64) + if strictScores && len(matchedIdxs) > 1 && sort <= lastGotSort { + inOrder = false + errorMsg = "candidate scores not strictly decreasing" + } + lastGotSort = sort + break } } @@ -95,7 +109,7 @@ func CheckCompletionOrder(want, got []protocol.CompletionItem) string { } if !inOrder { - return summarizeCompletionItems(-1, want, matched, "completions out of order") + return summarizeCompletionItems(-1, want, matched, errorMsg) } return ""