From 431033348dd07b0f609a1650948ef44a3c6cce54 Mon Sep 17 00:00:00 2001 From: Muir Manders Date: Wed, 19 Jun 2019 22:24:05 +0000 Subject: [PATCH] internal/lsp: fix function value completions Previously we would always expand *types.Func completion candidates to function calls, even if the expected type matched the function itself, not its return value. Now we check the function itself before we check its return value. This fixes cases like this: func foo() int { return 0 } var f func() int f = // now completes to "foo" instead of "foo()" Also, *types.Var function values were never getting expanded to calls. I fixed the completion formatting to know that both *types.Func and *types.Var objects might need to be invoked in the completion item. This fixes cases like this: foo := func() int { return 0 } var i int i = // now completes to "foo()" instead of "foo" Change-Id: I8d0e9e2774f92866a3dd881092c13019fb3f3fd5 GitHub-Last-Rev: 7442bc84b5bbb86296289bbc745ec56a5f89d901 GitHub-Pull-Request: golang/tools#122 Reviewed-on: https://go-review.googlesource.com/c/tools/+/182879 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/source/completion.go | 73 ++++++++++++++----- internal/lsp/source/completion_format.go | 45 ++++++++---- internal/lsp/source/util.go | 5 ++ internal/lsp/testdata/funcvalue/func_value.go | 27 +++++++ .../lsp/testdata/unresolved/unresolved.go.in | 2 +- internal/lsp/tests/tests.go | 2 +- 6 files changed, 122 insertions(+), 32 deletions(-) create mode 100644 internal/lsp/testdata/funcvalue/func_value.go diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 6b2a9d1e..6c7359ec 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -193,7 +193,7 @@ func (c *completer) setSurrounding(ident *ast.Ident) { // found adds a candidate completion. // // Only the first candidate of a given name is considered. -func (c *completer) found(obj types.Object, weight float64) { +func (c *completer) found(obj types.Object, score float64) { if obj.Pkg() != nil && obj.Pkg() != c.types && !obj.Exported() { return // inaccessible } @@ -201,13 +201,34 @@ func (c *completer) found(obj types.Object, weight float64) { return } c.seen[obj] = true - if c.matchingType(obj) { - weight *= highScore + + cand := candidate{ + obj: obj, + score: score, } + + if c.matchingType(&cand) { + cand.score *= highScore + } + if c.wantTypeName() && !isTypeName(obj) { - weight *= lowScore + cand.score *= lowScore } - c.items = append(c.items, c.item(obj, weight)) + + c.items = append(c.items, c.item(cand)) +} + +// candidate represents a completion candidate. +type candidate struct { + // obj is the types.Object to complete to. + obj types.Object + + // score is used to rank candidates. + score float64 + + // expandFuncCall is true if obj should be invoked in the completion. + // For example, expandFuncCall=true yields "foo()", expandFuncCall=false yields "foo". + expandFuncCall bool } // Completion returns a list of possible candidates for completion, given a @@ -915,23 +936,41 @@ Nodes: // matchingType reports whether an object is a good completion candidate // in the context of the expected type. -func (c *completer) matchingType(obj types.Object) bool { - actual := obj.Type() +func (c *completer) matchingType(cand *candidate) bool { + objType := cand.obj.Type() - // Use a function's return type as its type. - if sig, ok := actual.(*types.Signature); ok { - if sig.Results().Len() == 1 { - actual = sig.Results().At(0).Type() + // Default to invoking *types.Func candidates. This is so function + // completions in an empty statement (or other cases with no expected type) + // are invoked by default. + cand.expandFuncCall = isFunc(cand.obj) + + typeMatches := func(actual types.Type) bool { + // Take into account any type modifiers on the expected type. + actual = c.expectedType.applyTypeModifiers(actual) + + if c.expectedType.objType != nil { + // 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(types.Default(actual), types.Default(c.expectedType.objType)) } + + return false } - // Take into account any type modifiers on the expected type. - actual = c.expectedType.applyTypeModifiers(actual) + if typeMatches(objType) { + // If obj's type matches, we don't want to expand to an invocation of obj. + cand.expandFuncCall = false + return true + } - if c.expectedType.objType != nil { - // 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(types.Default(actual), types.Default(c.expectedType.objType)) + // 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 obj's return value matches the expected type, we need to invoke obj + // in the completion. + cand.expandFuncCall = true + return true + } } return false diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index c399afdc..bdb8cd1b 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -16,11 +16,13 @@ import ( "golang.org/x/tools/internal/lsp/snippet" ) -// formatCompletion creates a completion item for a given types.Object. -func (c *completer) item(obj types.Object, score float64) CompletionItem { +// formatCompletion creates a completion item for a given candidate. +func (c *completer) item(cand candidate) CompletionItem { + obj := cand.obj + // Handle builtin types separately. if obj.Parent() == types.Universe { - return c.formatBuiltin(obj, score) + return c.formatBuiltin(cand) } var ( @@ -32,6 +34,15 @@ func (c *completer) item(obj types.Object, score float64) CompletionItem { placeholderSnippet *snippet.Builder ) + // expandFuncCall mutates the completion label, detail, and snippets + // to that of an invocation of sig. + expandFuncCall := func(sig *types.Signature) { + params := formatParams(sig.Params(), sig.Variadic(), c.qf) + results, writeParens := formatResults(sig.Results(), c.qf) + label, detail = formatFunction(obj.Name(), params, results, writeParens) + plainSnippet, placeholderSnippet = c.functionCallSnippets(obj.Name(), params) + } + switch obj := obj.(type) { case *types.TypeName: detail, kind = formatType(obj.Type(), c.qf) @@ -49,23 +60,29 @@ func (c *completer) item(obj types.Object, score float64) CompletionItem { } else { kind = VariableCompletionItem } + + if sig, ok := obj.Type().Underlying().(*types.Signature); ok && cand.expandFuncCall { + expandFuncCall(sig) + } case *types.Func: - sig, ok := obj.Type().(*types.Signature) + sig, ok := obj.Type().Underlying().(*types.Signature) if !ok { break } - params := formatParams(sig.Params(), sig.Variadic(), c.qf) - results, writeParens := formatResults(sig.Results(), c.qf) - label, detail = formatFunction(obj.Name(), params, results, writeParens) - plainSnippet, placeholderSnippet = c.functionCallSnippets(obj.Name(), params) + kind = FunctionCompletionItem - if sig.Recv() != nil { + if sig != nil && sig.Recv() != nil { kind = MethodCompletionItem } + + if cand.expandFuncCall { + expandFuncCall(sig) + } case *types.PkgName: kind = PackageCompletionItem - detail = fmt.Sprintf("\"%s\"", obj.Imported().Path()) + detail = fmt.Sprintf("%q", obj.Imported().Path()) } + detail = strings.TrimPrefix(detail, "untyped ") return CompletionItem{ @@ -73,7 +90,7 @@ func (c *completer) item(obj types.Object, score float64) CompletionItem { InsertText: insert, Detail: detail, Kind: kind, - Score: score, + Score: cand.score, plainSnippet: plainSnippet, placeholderSnippet: placeholderSnippet, } @@ -93,11 +110,13 @@ func (c *completer) isParameter(v *types.Var) bool { return false } -func (c *completer) formatBuiltin(obj types.Object, score float64) CompletionItem { +func (c *completer) formatBuiltin(cand candidate) CompletionItem { + obj := cand.obj + item := CompletionItem{ Label: obj.Name(), InsertText: obj.Name(), - Score: score, + Score: cand.score, } switch obj.(type) { case *types.Const: diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index 403d1c82..a5aad79a 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -132,6 +132,11 @@ func isTypeName(obj types.Object) bool { return ok } +func isFunc(obj types.Object) bool { + _, ok := obj.(*types.Func) + return ok +} + func formatParams(tup *types.Tuple, variadic bool, qf types.Qualifier) []string { params := make([]string, 0, tup.Len()) for i := 0; i < tup.Len(); i++ { diff --git a/internal/lsp/testdata/funcvalue/func_value.go b/internal/lsp/testdata/funcvalue/func_value.go new file mode 100644 index 00000000..a36a4a26 --- /dev/null +++ b/internal/lsp/testdata/funcvalue/func_value.go @@ -0,0 +1,27 @@ +package funcvalue + +func fooFunc() int { //@item(fvFooFunc, "fooFunc", "func() int", "func") + return 0 +} + +var _ = fooFunc() //@item(fvFooFuncCall, "fooFunc()", "int", "func") + +var fooVar = func() int { //@item(fvFooVar, "fooVar", "func() int", "var") + return 0 +} + +var _ = fooVar() //@item(fvFooVarCall, "fooVar()", "int", "var") + +type myFunc func() int + +var fooType myFunc = fooVar //@item(fvFooType, "fooType", "myFunc", "var") + +var _ = fooType() //@item(fvFooTypeCall, "fooType()", "int", "var") + +func _() { + var f func() int + f = foo //@complete(" //", fvFooFunc, fvFooType, fvFooVar) + + var i int + i = foo //@complete(" //", fvFooFuncCall, fvFooTypeCall, fvFooVarCall) +} diff --git a/internal/lsp/testdata/unresolved/unresolved.go.in b/internal/lsp/testdata/unresolved/unresolved.go.in index 731d5825..ceb7fe2b 100644 --- a/internal/lsp/testdata/unresolved/unresolved.go.in +++ b/internal/lsp/testdata/unresolved/unresolved.go.in @@ -1,6 +1,6 @@ package unresolved -func foo(interface{}) { //@item(unresolvedFoo, "foo(interface{})", "", "func") +func foo(interface{}) { //@item(unresolvedFoo, "foo", "func(interface{})", "func") // don't crash on fake "resolved" type foo(func(i, j f //@complete(" //", unresolvedFoo) } diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index ceda9f7b..ba89f4f6 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -25,7 +25,7 @@ import ( // We hardcode the expected number of test cases to ensure that all tests // are being executed. If a test is added, this number must be changed. const ( - ExpectedCompletionsCount = 125 + ExpectedCompletionsCount = 127 ExpectedCompletionSnippetCount = 14 ExpectedDiagnosticsCount = 17 ExpectedFormatCount = 5