From 02335f11d59836d522f159307a6cfee10f179add Mon Sep 17 00:00:00 2001 From: Muir Manders Date: Tue, 15 Oct 2019 09:26:11 -0700 Subject: [PATCH] internal/lsp: don't qualify literal candidates in *ast.SelectorExpr Previously we unconditionally qualified literal candidate types with their package. For example: var buf *bytes.Buffer buf = &bytes.Bu<> would complete to: buf = &bytes.bytes.Buffer{} Now we don't qualify the type if the cursor position is in the selector of an *ast.SelectorExpr. We only generate literal candidates for type names, so if we are in a selector then we can assume it is a package qualified type (as opposed to an object field). We also handle the insertion of "&" for literal pointers better. If you are in the selector of an *ast.SelectorExpr, we prepend the "&" to the beginning of the expression rather than the selector. For example, you will end up with "&bytes.Buffer{}" instead of "bytes.&Buffer{}". Updates golang/go#34872. Change-Id: I812aa809cd4e649a429853386789f80033412814 Reviewed-on: https://go-review.googlesource.com/c/tools/+/201200 Run-TryBot: Rebecca Stambler Reviewed-by: Rebecca Stambler --- internal/lsp/source/completion_literal.go | 89 +++++++++++++++---- internal/lsp/source/util.go | 20 +++++ ...ral_snippets.go => literal_snippets.go.in} | 14 +++ internal/lsp/testdata/summary.txt.golden | 2 +- 4 files changed, 105 insertions(+), 20 deletions(-) rename internal/lsp/testdata/snippets/{literal_snippets.go => literal_snippets.go.in} (89%) diff --git a/internal/lsp/source/completion_literal.go b/internal/lsp/source/completion_literal.go index c8dbafb1bc..0a55f67f68 100644 --- a/internal/lsp/source/completion_literal.go +++ b/internal/lsp/source/completion_literal.go @@ -5,12 +5,17 @@ package source import ( + "go/ast" + "go/token" "go/types" "strings" "unicode" + "golang.org/x/tools/internal/lsp/diff" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/snippet" + "golang.org/x/tools/internal/span" + "golang.org/x/tools/internal/telemetry/log" ) // literal generates composite literal, function literal, and make() @@ -64,7 +69,18 @@ func (c *completer) literal(literalType types.Type) { literalType = ptr.Elem() } - typeName := types.TypeString(literalType, c.qf) + var ( + qf = c.qf + sel = enclosingSelector(c.path, c.pos) + ) + + // Don't qualify the type name if we are in a selector expression + // since the package name is already present. + if sel != nil { + qf = func(_ *types.Package) string { return "" } + } + + typeName := types.TypeString(literalType, qf) // A type name of "[]int" doesn't work very will with the matcher // since "[" isn't a valid identifier prefix. Here we strip off the @@ -72,26 +88,41 @@ func (c *completer) literal(literalType types.Type) { matchName := typeName switch t := literalType.(type) { case *types.Slice: - matchName = types.TypeString(t.Elem(), c.qf) + matchName = types.TypeString(t.Elem(), qf) case *types.Array: - matchName = types.TypeString(t.Elem(), c.qf) + matchName = types.TypeString(t.Elem(), qf) } // If prefix matches the type name, client may want a composite literal. if score := c.matcher.Score(matchName); score >= 0 { + var protocolEdits []protocol.TextEdit + if isPointer { - typeName = "&" + typeName + if sel != nil { + // If we are in a selector we must place the "&" before the selector. + // For example, "foo.B<>" must complete to "&foo.Bar{}", not + // "foo.&Bar{}". + edits, err := referenceEdit(c.view.Session().Cache().FileSet(), c.mapper, sel) + if err != nil { + log.Error(c.ctx, "error making edit for literal pointer completion", err) + return + } + protocolEdits = append(protocolEdits, edits...) + } else { + // Otherwise we can stick the "&" directly before the type name. + typeName = "&" + typeName + } } switch t := literalType.Underlying().(type) { case *types.Struct, *types.Array, *types.Slice, *types.Map: - c.compositeLiteral(t, typeName, float64(score)) + c.compositeLiteral(t, typeName, float64(score), protocolEdits) case *types.Basic: // Add a literal completion for basic types that implement our // expected interface (e.g. named string type http.Dir // implements http.FileSystem). if isInterface(c.expectedType.objType) { - c.basicLiteral(t, typeName, float64(score)) + c.basicLiteral(t, typeName, float64(score), protocolEdits) } } } @@ -120,6 +151,24 @@ func (c *completer) literal(literalType types.Type) { } } +// referenceEdit produces text edits that prepend a "&" operator to the +// specified node. +func referenceEdit(fset *token.FileSet, m *protocol.ColumnMapper, node ast.Node) ([]protocol.TextEdit, error) { + rng := span.Range{ + FileSet: fset, + Start: node.Pos(), + End: node.Pos(), + } + spn, err := rng.Span() + if err != nil { + return nil, err + } + return ToProtocolEdits(m, []diff.TextEdit{{ + Span: spn, + NewText: "&", + }}) +} + // literalCandidateScore is the base score for literal candidates. // Literal candidates match the expected type so they should be high // scoring, but we want them ranked below lexical objects of the @@ -248,7 +297,7 @@ func abbreviateCamel(s string) string { } // compositeLiteral adds a composite literal completion item for the given typeName. -func (c *completer) compositeLiteral(T types.Type, typeName string, matchScore float64) { +func (c *completer) compositeLiteral(T types.Type, typeName string, matchScore float64, edits []protocol.TextEdit) { snip := &snippet.Builder{} snip.WriteText(typeName + "{") // Don't put the tab stop inside the composite literal curlies "{}" @@ -261,17 +310,18 @@ func (c *completer) compositeLiteral(T types.Type, typeName string, matchScore f nonSnippet := typeName + "{}" c.items = append(c.items, CompletionItem{ - Label: nonSnippet, - InsertText: nonSnippet, - Score: matchScore * literalCandidateScore, - Kind: protocol.VariableCompletion, - snippet: snip, + Label: nonSnippet, + InsertText: nonSnippet, + Score: matchScore * literalCandidateScore, + Kind: protocol.VariableCompletion, + AdditionalTextEdits: edits, + snippet: snip, }) } // basicLiteral adds a literal completion item for the given basic // type name typeName. -func (c *completer) basicLiteral(T types.Type, typeName string, matchScore float64) { +func (c *completer) basicLiteral(T types.Type, typeName string, matchScore float64, edits []protocol.TextEdit) { snip := &snippet.Builder{} snip.WriteText(typeName + "(") snip.WriteFinalTabstop() @@ -280,12 +330,13 @@ func (c *completer) basicLiteral(T types.Type, typeName string, matchScore float nonSnippet := typeName + "()" c.items = append(c.items, CompletionItem{ - Label: nonSnippet, - InsertText: nonSnippet, - Detail: T.String(), - Score: matchScore * literalCandidateScore, - Kind: protocol.VariableCompletion, - snippet: snip, + Label: nonSnippet, + InsertText: nonSnippet, + Detail: T.String(), + Score: matchScore * literalCandidateScore, + Kind: protocol.VariableCompletion, + AdditionalTextEdits: edits, + snippet: snip, }) } diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index 304de90920..1e6d736320 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -340,6 +340,26 @@ func isEmptyInterface(T types.Type) bool { return intf != nil && intf.NumMethods() == 0 } +// isSelector returns the enclosing *ast.SelectorExpr when pos is in the +// selector. +func enclosingSelector(path []ast.Node, pos token.Pos) *ast.SelectorExpr { + if len(path) == 0 { + return nil + } + + if sel, ok := path[0].(*ast.SelectorExpr); ok { + return sel + } + + if _, ok := path[0].(*ast.Ident); ok && len(path) > 1 { + if sel, ok := path[1].(*ast.SelectorExpr); ok && pos >= sel.Sel.Pos() { + return sel + } + } + + return nil +} + // typeConversion returns the type being converted to if call is a type // conversion expression. func typeConversion(call *ast.CallExpr, info *types.Info) types.Type { diff --git a/internal/lsp/testdata/snippets/literal_snippets.go b/internal/lsp/testdata/snippets/literal_snippets.go.in similarity index 89% rename from internal/lsp/testdata/snippets/literal_snippets.go rename to internal/lsp/testdata/snippets/literal_snippets.go.in index 658baf39cd..8a73c44481 100644 --- a/internal/lsp/testdata/snippets/literal_snippets.go +++ b/internal/lsp/testdata/snippets/literal_snippets.go.in @@ -3,6 +3,8 @@ package snippets import ( "net/http" "sort" + + "golang.org/x/tools/internal/lsp/foo" ) func _() { @@ -129,3 +131,15 @@ func _() { var duplicateParams func(myImpl, int, myImpl) duplicateParams = f //@snippet(" //", litFunc, "", "func(${1:mi} myImpl, ${2:_} int, ${3:_} myImpl) {$0\\}") } + +func _() { + StructFoo{} //@item(litStructFoo, "StructFoo{}", "struct{...}", "struct") + + var sfp *foo.StructFoo + // Don't insert the "&" before "StructFoo{}". + sfp = foo.Str //@snippet(" //", litStructFoo, "StructFoo{$0\\}", "StructFoo{$0\\}") + + var sf foo.StructFoo + sf = foo.Str //@snippet(" //", litStructFoo, "StructFoo{$0\\}", "StructFoo{$0\\}") + sf = foo. //@snippet(" //", litStructFoo, "StructFoo{$0\\}", "StructFoo{$0\\}") +} diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden index bccb2349c5..3b3b10f8fb 100644 --- a/internal/lsp/testdata/summary.txt.golden +++ b/internal/lsp/testdata/summary.txt.golden @@ -1,6 +1,6 @@ -- summary -- CompletionsCount = 178 -CompletionSnippetCount = 36 +CompletionSnippetCount = 39 UnimportedCompletionsCount = 1 DeepCompletionsCount = 5 FuzzyCompletionsCount = 6