From 7baacfbe02f235d27c577edaa1e74f9f7f55c103 Mon Sep 17 00:00:00 2001 From: Muir Manders Date: Thu, 5 Sep 2019 12:04:13 -0700 Subject: [PATCH] internal/lsp: support function literal completions Now we will offer function literal completions when we know the expected type is a function. For example: sort.Slice(someSlice, <>) will offer the completion "func(...) {}" which if selected will insert: func(i, j int) bool {<>} I opted to use an abbreviated label "func(...) {}" because function signatures can be quite long/verbose with all the type names in there. The only interesting challenge is how to handle signatures that don't name the parameters. For example, func HandleFunc(pattern string, handler func(ResponseWriter, *Request)) { does not name the "ResponseWriter" and "Request" parameters. I went with a minimal effort approach where we try abbreviating the type names, so the literal completion item for "handler" would look like: func( ResponseWriter, *Request) {<>} where <> denote placeholders. The user can tab through quickly if they like the abbreviations, otherwise they can rename them. For unnamed types or if the abbreviation would duplicate a previous abbreviation, we fall back to "_" as the parameter name. The user will have to rename the parameter before they can use it. One side effect of this is that we cannot support function literal completions with unnamed parameters unless the user has enabled snippet placeholders. Change-Id: Ic0ec81e85cd8de79bff73314e80e722f10f8c84c Reviewed-on: https://go-review.googlesource.com/c/tools/+/193699 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/lsp_test.go | 18 ++- internal/lsp/source/completion.go | 2 +- internal/lsp/source/completion_literal.go | 133 +++++++++++++++++- internal/lsp/source/source_test.go | 3 - .../lsp/testdata/snippets/literal_snippets.go | 25 ++++ internal/lsp/tests/tests.go | 2 +- 6 files changed, 172 insertions(+), 11 deletions(-) diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 2cc30953cb..a6a1b604af 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -179,17 +179,25 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests break } } - if got == nil { - t.Fatalf("%s:%d: couldn't find completion matching %q", src.URI(), src.Start().Line(), wantItem.Label) - } var expected string if usePlaceholders { expected = want.PlaceholderSnippet } else { expected = want.PlainSnippet } - if expected != got.TextEdit.NewText { - t.Errorf("%s: expected snippet %q, got %q", src, expected, got.TextEdit.NewText) + + if expected == "" { + if got != nil { + t.Fatalf("%s:%d: expected no snippet but got %q", src.URI(), src.Start().Line(), got.TextEdit.NewText) + } + } else { + if got == nil { + t.Fatalf("%s:%d: couldn't find completion matching %q", src.URI(), src.Start().Line(), wantItem.Label) + } + + if expected != got.TextEdit.NewText { + t.Errorf("%s: expected snippet %q, got %q", src, expected, got.TextEdit.NewText) + } } view.SetOptions(original) } diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 665f28dd68..b71318270e 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -662,7 +662,7 @@ func (c *completer) lexical() error { // handled during the normal lexical object search. For example, // if our expected type is "[]int", this will add a candidate of // "[]int{}". - if _, named := c.expectedType.objType.(*types.Named); !named { + if _, named := deref(c.expectedType.objType).(*types.Named); !named { c.literal(c.expectedType.objType) } } diff --git a/internal/lsp/source/completion_literal.go b/internal/lsp/source/completion_literal.go index 9e990dc3f4..cf59079e7d 100644 --- a/internal/lsp/source/completion_literal.go +++ b/internal/lsp/source/completion_literal.go @@ -7,11 +7,13 @@ package source import ( "go/types" "strings" + "unicode" "golang.org/x/tools/internal/lsp/snippet" ) -// literal generates composite literal and make() completion items. +// literal generates composite literal, function literal, and make() +// completion items. func (c *completer) literal(literalType types.Type) { if c.expectedType.objType == nil { return @@ -100,6 +102,14 @@ func (c *completer) literal(literalType types.Type) { c.makeCall(typeName, "", float64(score)) } } + + // If prefix matches "func", client may want a function literal. + if score := c.matcher.Score("func"); !isPointer && score >= 0 { + switch t := literalType.Underlying().(type) { + case *types.Signature: + c.functionLiteral(t, float64(score)) + } + } } // literalCandidateScore is the base score for literal candidates. @@ -108,6 +118,127 @@ func (c *completer) literal(literalType types.Type) { // correct type, so scale down highScore. const literalCandidateScore = highScore / 2 +// functionLiteral adds a function literal completion item for the +// given signature. +func (c *completer) functionLiteral(sig *types.Signature, matchScore float64) { + snip := &snippet.Builder{} + snip.WriteText("func(") + seenParamNames := make(map[string]bool) + for i := 0; i < sig.Params().Len(); i++ { + if i > 0 { + snip.WriteText(", ") + } + + p := sig.Params().At(i) + name := p.Name() + + // If the parameter has no name in the signature, we need to try + // come up with a parameter name. + if name == "" { + // Our parameter names are guesses, so they must be placeholders + // for easy correction. If placeholders are disabled, don't + // offer the completion. + if !c.opts.Placeholders { + return + } + + // Try abbreviating named types. If the type isn't named, or the + // abbreviation duplicates a previous name, give up and use + // "_". The user will have to provide a name for this parameter + // in order to use it. + if named, ok := deref(p.Type()).(*types.Named); ok { + name = abbreviateCamel(named.Obj().Name()) + if seenParamNames[name] { + name = "_" + } else { + seenParamNames[name] = true + } + } else { + name = "_" + } + snip.WritePlaceholder(func(b *snippet.Builder) { + b.WriteText(name) + }) + } else { + snip.WriteText(name) + } + + // If the following param's type is identical to this one, omit + // this param's type string. For example, emit "i, j int" instead + // of "i int, j int". + if i == sig.Params().Len()-1 || !types.Identical(p.Type(), sig.Params().At(i+1).Type()) { + snip.WriteText(" ") + typeStr := types.TypeString(p.Type(), c.qf) + if sig.Variadic() && i == sig.Params().Len()-1 { + typeStr = strings.Replace(typeStr, "[]", "...", 1) + } + snip.WriteText(typeStr) + } + } + snip.WriteText(")") + + results := sig.Results() + if results.Len() > 0 { + snip.WriteText(" ") + } + + resultsNeedParens := results.Len() > 1 || + results.Len() == 1 && results.At(0).Name() != "" + + if resultsNeedParens { + snip.WriteText("(") + } + for i := 0; i < results.Len(); i++ { + if i > 0 { + snip.WriteText(", ") + } + r := results.At(i) + if name := r.Name(); name != "" { + snip.WriteText(name + " ") + } + snip.WriteText(types.TypeString(r.Type(), c.qf)) + } + if resultsNeedParens { + snip.WriteText(")") + } + + snip.WriteText(" {") + snip.WriteFinalTabstop() + snip.WriteText("}") + + c.items = append(c.items, CompletionItem{ + Label: "func(...) {}", + Score: matchScore * literalCandidateScore, + Kind: VariableCompletionItem, + snippet: snip, + }) +} + +// abbreviateCamel abbreviates camel case identifiers into +// abbreviations. For example, "fooBar" is abbreviated "fb". +func abbreviateCamel(s string) string { + var ( + b strings.Builder + useNextUpper bool + ) + for i, r := range s { + if i == 0 { + b.WriteRune(unicode.ToLower(r)) + } + + if unicode.IsUpper(r) { + if useNextUpper { + b.WriteRune(unicode.ToLower(r)) + useNextUpper = false + } + } else { + useNextUpper = true + } + } + + return b.String() +} + // compositeLiteral adds a composite literal completion item for the given typeName. func (c *completer) compositeLiteral(T types.Type, typeName string, matchScore float64) { snip := &snippet.Builder{} diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 9e1398fad0..bdc7722670 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -186,9 +186,6 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests break } } - if got == nil { - t.Fatalf("%s: couldn't find completion matching %q", src.URI(), wantItem.Label) - } expected := want.PlainSnippet if usePlaceholders { expected = want.PlaceholderSnippet diff --git a/internal/lsp/testdata/snippets/literal_snippets.go b/internal/lsp/testdata/snippets/literal_snippets.go index 0f207df407..f2c947281b 100644 --- a/internal/lsp/testdata/snippets/literal_snippets.go +++ b/internal/lsp/testdata/snippets/literal_snippets.go @@ -1,5 +1,10 @@ package snippets +import ( + "net/http" + "sort" +) + func _() { []int{} //@item(litIntSlice, "[]int{}", "", "var") make([]int, 0) //@item(makeIntSlice, "make([]int, 0)", "", "func") @@ -96,3 +101,23 @@ func _() { // Don't give literal slice candidate for variadic arg. foo("", myStruct) //@complete(")", litStructType) } + +func _() { + "func(...) {}" //@item(litFunc, "func(...) {}", "", "var") + + sort.Slice(nil, f) //@snippet(")", litFunc, "func(i, j int) bool {$0\\}", "func(i, j int) bool {$0\\}") + + http.HandleFunc("", f) //@snippet(")", litFunc, "", "func(${1:rw} http.ResponseWriter, ${2:r} *http.Request) {$0\\}") + + var namedReturn func(s string) (b bool) + namedReturn = f //@snippet(" //", litFunc, "func(s string) (b bool) {$0\\}", "func(s string) (b bool) {$0\\}") + + var multiReturn func() (bool, int) + multiReturn = f //@snippet(" //", litFunc, "func() (bool, int) {$0\\}", "func() (bool, int) {$0\\}") + + var multiNamedReturn func() (b bool, i int) + multiNamedReturn = f //@snippet(" //", litFunc, "func() (b bool, i int) {$0\\}", "func() (b bool, i int) {$0\\}") + + var duplicateParams func(myImpl, int, myImpl) + duplicateParams = f //@snippet(" //", litFunc, "", "func(${1:mi} myImpl, ${2:_} int, ${3:_} myImpl) {$0\\}") +} diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index 9dfabc4d47..97e57095c3 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -31,7 +31,7 @@ import ( // are being executed. If a test is added, this number must be changed. const ( ExpectedCompletionsCount = 164 - ExpectedCompletionSnippetCount = 29 + ExpectedCompletionSnippetCount = 35 ExpectedDiagnosticsCount = 21 ExpectedFormatCount = 6 ExpectedImportCount = 2