From a99e43fcffb727f68c326ff6558808c401346b76 Mon Sep 17 00:00:00 2001 From: Muir Manders Date: Fri, 15 Nov 2019 22:28:29 -0800 Subject: [PATCH] internal/lsp: fix literal completions in variadic params In cases like: var foo []bytes.Buffer foo = append(foo, <>) you will now get a literal candidate "bytes.Buffer{}". Previously we were skipping all literal candidates at the variadic position, but the intention was to only skip literal slice candidates (i.e. "[]bytes.Buffer{}" in the above example). I also improved the literal struct snippet to not leave the cursor inside the curlies when the struct type has no accessible fields. Previously it was only checking if the struct had no fields at all. This means after completing in the above example you will end up with "bytes.Buffer{}<>" instead of "bytes.Buffer{<>}", where "<>" denotes the cursor. Change-Id: Ic2604a4ea65d84ad855ad6e6d98b8ab76eb08d77 Reviewed-on: https://go-review.googlesource.com/c/tools/+/207537 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/source/completion_literal.go | 35 ++++++++++--------- internal/lsp/source/util.go | 11 ++++++ .../testdata/snippets/literal_snippets.go.in | 14 ++++++-- internal/lsp/testdata/summary.txt.golden | 2 +- 4 files changed, 43 insertions(+), 19 deletions(-) diff --git a/internal/lsp/source/completion_literal.go b/internal/lsp/source/completion_literal.go index bdf1222b5ab..004950dd216 100644 --- a/internal/lsp/source/completion_literal.go +++ b/internal/lsp/source/completion_literal.go @@ -21,14 +21,22 @@ import ( // literal generates composite literal, function literal, and make() // completion items. func (c *completer) literal(literalType types.Type, imp *importInfo) { - // Don't provide literal candidates for variadic function arguments. - // For example, don't provide "[]interface{}{}" in "fmt.Print(<>)". - if c.expectedType.variadic { - return - } - expType := c.expectedType.objType + if c.expectedType.variadic { + // Don't offer literal slice candidates for variadic arguments. + // For example, don't offer "[]interface{}{}" in "fmt.Print(<>)". + if c.expectedType.matchesVariadic(literalType) { + return + } + + // Otherwise, consider our expected type to be the variadic + // element type, not the slice type. + if slice, ok := expType.(*types.Slice); ok { + expType = slice.Elem() + } + } + // Avoid literal candidates if the expected type is an empty // interface. It isn't very useful to suggest a literal candidate of // every possible type. @@ -54,19 +62,14 @@ func (c *completer) literal(literalType types.Type, imp *importInfo) { // Check if an object of type literalType or *literalType would // match our expected type. + var isPointer bool if !c.matchingType(literalType) { - literalType = types.NewPointer(literalType) - - if !c.matchingType(literalType) { + isPointer = true + if !c.matchingType(types.NewPointer(literalType)) { return } } - ptr, isPointer := literalType.(*types.Pointer) - if isPointer { - literalType = ptr.Elem() - } - var ( qf = c.qf sel = enclosingSelector(c.path, c.pos) @@ -303,8 +306,8 @@ func (c *completer) compositeLiteral(T types.Type, typeName string, matchScore f snip := &snippet.Builder{} snip.WriteText(typeName + "{") // Don't put the tab stop inside the composite literal curlies "{}" - // for structs that have no fields. - if strct, ok := T.(*types.Struct); !ok || strct.NumFields() > 0 { + // for structs that have no accessible fields. + if strct, ok := T.(*types.Struct); !ok || fieldsAccessible(strct, c.pkg.GetTypes()) { snip.WriteFinalTabstop() } snip.WriteText("}") diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index 7ad9625fc87..00470f6cd3e 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -380,6 +380,17 @@ func typeConversion(call *ast.CallExpr, info *types.Info) types.Type { return nil } +// fieldsAccessible returns whether s has at least one field accessible by p. +func fieldsAccessible(s *types.Struct, p *types.Package) bool { + for i := 0; i < s.NumFields(); i++ { + f := s.Field(i) + if f.Exported() || f.Pkg() == p { + return true + } + } + return false +} + func formatParams(s Snapshot, pkg Package, sig *types.Signature, qf types.Qualifier) []string { params := make([]string, 0, sig.Params().Len()) for i := 0; i < sig.Params().Len(); i++ { diff --git a/internal/lsp/testdata/snippets/literal_snippets.go.in b/internal/lsp/testdata/snippets/literal_snippets.go.in index 1a8e77563d5..57489fb3e32 100644 --- a/internal/lsp/testdata/snippets/literal_snippets.go.in +++ b/internal/lsp/testdata/snippets/literal_snippets.go.in @@ -1,6 +1,7 @@ package snippets import ( + "bytes" "net/http" "sort" @@ -105,11 +106,20 @@ func _() { } func _() { - type myStruct struct{ i int } //@item(litStructType, "myStruct", "struct{...}", "struct") + type myStruct struct{ i int } //@item(litMyStructType, "myStruct", "struct{...}", "struct") + myStruct{} //@item(litMyStruct, "myStruct{}", "", "var") foo := func(s string, args ...myStruct) {} // Don't give literal slice candidate for variadic arg. - foo("", myStruct) //@complete(")", litStructType) + // Do give literal candidates for variadic element. + foo("", myStruct) //@complete(")", litMyStruct, litMyStructType) +} + +func _() { + Buffer{} //@item(litBuffer, "Buffer{}", "", "var") + + var b *bytes.Buffer + b = bytes.Bu //@snippet(" //", litBuffer, "Buffer{\\}", "Buffer{\\}") } func _() { diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden index ae1a2631797..6ad6c57ce30 100644 --- a/internal/lsp/testdata/summary.txt.golden +++ b/internal/lsp/testdata/summary.txt.golden @@ -1,6 +1,6 @@ -- summary -- CompletionsCount = 215 -CompletionSnippetCount = 50 +CompletionSnippetCount = 51 UnimportedCompletionsCount = 3 DeepCompletionsCount = 5 FuzzyCompletionsCount = 7