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 <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

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,
+		Label:               nonSnippet,
-		InsertText: nonSnippet,
+		InsertText:          nonSnippet,
-		Score:      matchScore * literalCandidateScore,
+		Score:               matchScore * literalCandidateScore,
-		Kind:       protocol.VariableCompletion,
+		Kind:                protocol.VariableCompletion,
-		snippet:    snip,
+		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,
+		Label:               nonSnippet,
-		InsertText: nonSnippet,
+		InsertText:          nonSnippet,
-		Detail:     T.String(),
+		Detail:              T.String(),
-		Score:      matchScore * literalCandidateScore,
+		Score:               matchScore * literalCandidateScore,
-		Kind:       protocol.VariableCompletion,
+		Kind:                protocol.VariableCompletion,
-		snippet:    snip,
+		AdditionalTextEdits: edits,
+		snippet:             snip,
 	})
 }
 

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 {

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\\}")
+}

internal/lsp/testdata/summary.txt.golden

@@ -1,6 +1,6 @@
 -- summary --
 CompletionsCount = 178
-CompletionSnippetCount = 36
+CompletionSnippetCount = 39
 UnimportedCompletionsCount = 1
 DeepCompletionsCount = 5
 FuzzyCompletionsCount = 6