lsp/completion: reorganize how we track candidate type mods

"type mod" refers to agglutinative expressions such as dereference
"*", invocation "()", and slicing "[:]". When considering an object as
a completion candidate, we check whether applying a type mod would
make it a better candidate.

Previously we tracked the type mods we wanted to apply to a candidate
by setting bool fields. Now instead we keep a slice of the type mods.
This has two main advantages:
- The mods are now ordered which will allow us to format candidates
  properly when the same mods can appear in different order (e.g.
  "<-*foo" or *<-foo").
- We can now record any mod multiple times allowing for "<-<-foo" or
  "foo()()".

I changed the formatting code to always create a snippet object since
that made things simpler. I had to tweak a few snippet helper methods
to accept a snippet argument rather than creating a new snippet.

This commit's only functional change is that we no longer show any
type mods in candidate labels. For example, the user will now see
"foo" in the completion popup instead of "*foo". Showing the operators
adds noise to the candidate list, and we didn't display them
consistently.

Updates golang/go#46045.

Change-Id: I3ea7baa1ee2fee80a1f8cfe88cbae1093ae269ba
Reviewed-on: https://go-review.googlesource.com/c/tools/+/323449
Run-TryBot: Muir Manders <muir@mnd.rs>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
diff --git a/gopls/internal/regtest/completion/completion_test.go b/gopls/internal/regtest/completion/completion_test.go
index afdd494..7dca27c 100644
--- a/gopls/internal/regtest/completion/completion_test.go
+++ b/gopls/internal/regtest/completion/completion_test.go
@@ -443,9 +443,9 @@
 		}{
 			{`var _ a = aaaa()`, []string{"aaaa1", "aaaa2"}},
 			{`var _ b = bbbb()`, []string{"bbbb1", "bbbb2"}},
-			{`var _ c = xxxx()`, []string{"***xxxxd", "**xxxxe", "xxxxc"}},
-			{`var _ d = xxxx()`, []string{"***xxxxe", "*xxxxc", "xxxxd"}},
-			{`var _ e = xxxx()`, []string{"**xxxxc", "*xxxxd", "xxxxe"}},
+			{`var _ c = xxxx()`, []string{"xxxxc", "xxxxd", "xxxxe"}},
+			{`var _ d = xxxx()`, []string{"xxxxc", "xxxxd", "xxxxe"}},
+			{`var _ e = xxxx()`, []string{"xxxxc", "xxxxd", "xxxxe"}},
 		}
 		for _, tt := range tests {
 			completions := env.Completion("main.go", env.RegexpSearch("main.go", tt.re))
diff --git a/internal/lsp/source/completion/completion.go b/internal/lsp/source/completion/completion.go
index 6d4bef9..6912ff6 100644
--- a/internal/lsp/source/completion/completion.go
+++ b/internal/lsp/source/completion/completion.go
@@ -370,31 +370,14 @@
 	// expanded calls for function invocations.
 	names []string
 
-	// expandFuncCall is true if obj should be invoked in the completion.
-	// For example, expandFuncCall=true yields "foo()", expandFuncCall=false yields "foo".
-	expandFuncCall bool
-
-	// takeAddress is true if the completion should take a pointer to obj.
-	// For example, takeAddress=true yields "&foo", takeAddress=false yields "foo".
-	takeAddress bool
+	// mods contains modifications that should be applied to the
+	// candidate when inserted. For example, "foo" may be insterted as
+	// "*foo" or "foo()".
+	mods []typeModKind
 
 	// addressable is true if a pointer can be taken to the candidate.
 	addressable bool
 
-	// makePointer is true if the candidate type name T should be made into *T.
-	makePointer bool
-
-	// dereference is a count of how many times to dereference the candidate obj.
-	// For example, dereference=2 turns "foo" into "**foo" when formatting.
-	dereference int
-
-	// takeSlice is true if obj is an array that should be converted to a slice.
-	takeSlice bool
-
-	// variadic is true if this candidate fills a variadic param and
-	// needs "..." appended.
-	variadic bool
-
 	// convertTo is a type that this candidate should be cast to. For
 	// example, if convertTo is float64, "foo" should be formatted as
 	// "float64(foo)".
@@ -405,6 +388,15 @@
 	imp *importInfo
 }
 
+func (c candidate) hasMod(mod typeModKind) bool {
+	for _, m := range c.mods {
+		if m == mod {
+			return true
+		}
+	}
+	return false
+}
+
 // ErrIsDefinition is an error that informs the user they got no
 // completions because they tried to complete the name of a new object
 // being defined.
@@ -1768,20 +1760,23 @@
 	return nil
 }
 
-// typeModifier represents an operator that changes the expected type.
-type typeModifier struct {
-	mod      typeMod
+// typeMod represents an operator that changes the expected type.
+type typeMod struct {
+	mod      typeModKind
 	arrayLen int64
 }
 
-type typeMod int
+type typeModKind int
 
 const (
-	dereference typeMod = iota // pointer indirection: "*"
-	reference                  // adds level of pointer: "&" for values, "*" for type names
-	chanRead                   // channel read operator ("<-")
-	slice                      // make a slice type ("[]" in "[]int")
-	array                      // make an array type ("[2]" in "[2]int")
+	dereference   typeModKind = iota // pointer indirection: "*"
+	reference                        // adds level of pointer: "&" for values, "*" for type names
+	chanRead                         // channel read operator: "<-"
+	sliceType                        // make a slice type: "[]" in "[]int"
+	arrayType                        // make an array type: "[2]" in "[2]int"
+	invoke                           // make a function call: "()" in "foo()"
+	takeSlice                        // take slice of array: "[:]" in "foo[:]"
+	takeDotDotDot                    // turn slice into variadic args: "..." in "foo..."
 )
 
 type objKind int
@@ -1832,7 +1827,7 @@
 
 	// modifiers are prefixes such as "*", "&" or "<-" that influence how
 	// a candidate type relates to the expected type.
-	modifiers []typeModifier
+	modifiers []typeMod
 
 	// convertibleTo is a type our candidate type must be convertible to.
 	convertibleTo types.Type
@@ -1882,7 +1877,7 @@
 
 	// modifiers are prefixes such as "*", "&" or "<-" that influence how
 	// a candidate type relates to the expected type.
-	modifiers []typeModifier
+	modifiers []typeMod
 
 	// assertableFrom is a type that must be assertable to our candidate type.
 	assertableFrom types.Type
@@ -2108,13 +2103,13 @@
 			}
 			return inf
 		case *ast.StarExpr:
-			inf.modifiers = append(inf.modifiers, typeModifier{mod: dereference})
+			inf.modifiers = append(inf.modifiers, typeMod{mod: dereference})
 		case *ast.UnaryExpr:
 			switch node.Op {
 			case token.AND:
-				inf.modifiers = append(inf.modifiers, typeModifier{mod: reference})
+				inf.modifiers = append(inf.modifiers, typeMod{mod: reference})
 			case token.ARROW:
-				inf.modifiers = append(inf.modifiers, typeModifier{mod: chanRead})
+				inf.modifiers = append(inf.modifiers, typeMod{mod: chanRead})
 			}
 		case *ast.DeferStmt, *ast.GoStmt:
 			inf.objKind |= kindFunc
@@ -2209,9 +2204,9 @@
 		switch mod.mod {
 		case reference:
 			typ = types.NewPointer(typ)
-		case array:
+		case arrayType:
 			typ = types.NewArray(typ, mod.arrayLen)
-		case slice:
+		case sliceType:
 			typ = types.NewSlice(typ)
 		}
 	}
@@ -2325,7 +2320,7 @@
 			}
 			return typeNameInference{}
 		case *ast.StarExpr:
-			inf.modifiers = append(inf.modifiers, typeModifier{mod: reference})
+			inf.modifiers = append(inf.modifiers, typeMod{mod: reference})
 		case *ast.CompositeLit:
 			// We want a type name if position is in the "Type" part of a
 			// composite literal (e.g. "Foo<>{}").
@@ -2338,7 +2333,7 @@
 					// the composite literal and not the type name, but if
 					// affects our type completion nonetheless.
 					if u, ok := c.path[i+1].(*ast.UnaryExpr); ok && u.Op == token.AND {
-						inf.modifiers = append(inf.modifiers, typeModifier{mod: reference})
+						inf.modifiers = append(inf.modifiers, typeMod{mod: reference})
 					}
 				}
 			}
@@ -2349,13 +2344,13 @@
 				inf.wantTypeName = true
 				if n.Len == nil {
 					// No "Len" expression means a slice type.
-					inf.modifiers = append(inf.modifiers, typeModifier{mod: slice})
+					inf.modifiers = append(inf.modifiers, typeMod{mod: sliceType})
 				} else {
 					// Try to get the array type using the constant value of "Len".
 					tv, ok := c.pkg.GetTypesInfo().Types[n.Len]
 					if ok && tv.Value != nil && tv.Value.Kind() == constant.Int {
 						if arrayLen, ok := constant.Int64Val(tv.Value); ok {
-							inf.modifiers = append(inf.modifiers, typeModifier{mod: array, arrayLen: arrayLen})
+							inf.modifiers = append(inf.modifiers, typeMod{mod: arrayType, arrayLen: arrayLen})
 						}
 					}
 				}
@@ -2417,7 +2412,7 @@
 	if sig, ok := objType.Underlying().(*types.Signature); ok {
 		if sig.Results().Len() == 1 && f(sig.Results().At(0).Type(), false) {
 			// Mark the candidate so we know to append "()" when formatting.
-			c.expandFuncCall = true
+			c.mods = append(c.mods, invoke)
 			return true
 		}
 	}
@@ -2454,8 +2449,10 @@
 		}
 
 		if f(ptr.Elem(), false) {
-			// Mark the candidate so we know to prepend "*" when formatting.
-			c.dereference = ptrDepth
+			for i := 0; i < ptrDepth; i++ {
+				// Mark the candidate so we know to prepend "*" when formatting.
+				c.mods = append(c.mods, dereference)
+			}
 			return true
 		}
 
@@ -2465,13 +2462,13 @@
 	// Check if c is addressable and a pointer to c matches our type inference.
 	if c.addressable && f(types.NewPointer(objType), false) {
 		// Mark the candidate so we know to prepend "&" when formatting.
-		c.takeAddress = true
+		c.mods = append(c.mods, reference)
 		return true
 	}
 
 	if array, ok := objType.Underlying().(*types.Array); ok {
 		if f(types.NewSlice(array.Elem()), false) {
-			c.takeSlice = true
+			c.mods = append(c.mods, takeSlice)
 			return true
 		}
 	}
@@ -2510,7 +2507,7 @@
 	if sig, ok := candType.Underlying().(*types.Signature); ok {
 		if c.inference.assigneesMatch(cand, sig) {
 			// Invoke the candidate if its results are multi-assignable.
-			cand.expandFuncCall = true
+			cand.mods = append(cand.mods, invoke)
 			return true
 		}
 	}
@@ -2518,7 +2515,9 @@
 	// 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)
+	if isFunc(cand.obj) {
+		cand.mods = append(cand.mods, invoke)
+	}
 
 	return false
 }
@@ -2572,7 +2571,7 @@
 			}
 
 			if expType == variadicType {
-				cand.variadic = true
+				cand.mods = append(cand.mods, takeDotDotDot)
 			}
 
 			// Lower candidate score for untyped conversions. This avoids
@@ -2611,7 +2610,7 @@
 			// matches.
 			if ci.kindMatches(candType) {
 				if ci.objKind == kindFunc {
-					cand.expandFuncCall = true
+					cand.mods = append(cand.mods, invoke)
 				}
 				return true
 			}
@@ -2814,11 +2813,11 @@
 		if c.inference.typeName.compLitType {
 			// If we are completing a composite literal type as in
 			// "foo<>{}", to make a pointer we must prepend "&".
-			cand.takeAddress = true
+			cand.mods = append(cand.mods, reference)
 		} else {
 			// If we are completing a normal type name such as "foo<>", to
 			// make a pointer we must prepend "*".
-			cand.makePointer = true
+			cand.mods = append(cand.mods, dereference)
 		}
 		return true
 	}
diff --git a/internal/lsp/source/completion/deep_completion.go b/internal/lsp/source/completion/deep_completion.go
index 71a6726..e7c4854 100644
--- a/internal/lsp/source/completion/deep_completion.go
+++ b/internal/lsp/source/completion/deep_completion.go
@@ -249,7 +249,7 @@
 	}
 
 	// Lower score of method calls so we prefer fields and vars over calls.
-	if cand.expandFuncCall {
+	if cand.hasMod(invoke) {
 		if sig, ok := obj.Type().Underlying().(*types.Signature); ok && sig.Recv() != nil {
 			cand.score *= 0.9
 		}
diff --git a/internal/lsp/source/completion/format.go b/internal/lsp/source/completion/format.go
index 985b79f..08b933f 100644
--- a/internal/lsp/source/completion/format.go
+++ b/internal/lsp/source/completion/format.go
@@ -46,20 +46,14 @@
 		detail        = types.TypeString(obj.Type(), c.qf)
 		insert        = label
 		kind          = protocol.TextCompletion
-		snip          *snippet.Builder
+		snip          snippet.Builder
 		protocolEdits []protocol.TextEdit
 	)
 	if obj.Type() == nil {
 		detail = ""
 	}
 
-	// expandFuncCall mutates the completion label, detail, and snippet
-	// to that of an invocation of sig.
-	expandFuncCall := func(sig *types.Signature) {
-		s := source.NewSignature(ctx, c.snapshot, c.pkg, sig, nil, c.qf)
-		snip = c.functionCallSnippet(label, s.Params())
-		detail = "func" + s.Format()
-	}
+	snip.WriteText(insert)
 
 	switch obj := obj.(type) {
 	case *types.TypeName:
@@ -74,17 +68,13 @@
 		}
 		if obj.IsField() {
 			kind = protocol.FieldCompletion
-			snip = c.structFieldSnippet(cand, label, detail)
+			c.structFieldSnippet(cand, detail, &snip)
 		} else {
 			kind = protocol.VariableCompletion
 		}
 		if obj.Type() == nil {
 			break
 		}
-
-		if sig, ok := obj.Type().Underlying().(*types.Signature); ok && cand.expandFuncCall {
-			expandFuncCall(sig)
-		}
 	case *types.Func:
 		sig, ok := obj.Type().Underlying().(*types.Signature)
 		if !ok {
@@ -94,10 +84,6 @@
 		if sig != nil && sig.Recv() != nil {
 			kind = protocol.MethodCompletion
 		}
-
-		if cand.expandFuncCall {
-			expandFuncCall(sig)
-		}
 	case *types.PkgName:
 		kind = protocol.ModuleCompletion
 		detail = fmt.Sprintf("%q", obj.Imported().Path())
@@ -106,6 +92,49 @@
 		detail = "label"
 	}
 
+	var prefix string
+	for _, mod := range cand.mods {
+		switch mod {
+		case reference:
+			prefix = "&" + prefix
+		case dereference:
+			prefix = "*" + prefix
+		case chanRead:
+			prefix = "<-" + prefix
+		}
+	}
+
+	var (
+		suffix   string
+		funcType = obj.Type()
+	)
+Suffixes:
+	for _, mod := range cand.mods {
+		switch mod {
+		case invoke:
+			if sig, ok := funcType.Underlying().(*types.Signature); ok {
+				s := source.NewSignature(ctx, c.snapshot, c.pkg, sig, nil, c.qf)
+				c.functionCallSnippet("", s.Params(), &snip)
+				if sig.Results().Len() == 1 {
+					funcType = sig.Results().At(0).Type()
+				}
+				detail = "func" + s.Format()
+			}
+
+			if !c.opts.snippets {
+				// Without snippets the candidate will not include "()". Don't
+				// add further suffixes since they will be invalid. For
+				// example, with snippets "foo()..." would become "foo..."
+				// without snippets if we added the dotDotDot.
+				break Suffixes
+			}
+		case takeSlice:
+			suffix += "[:]"
+		case takeDotDotDot:
+			suffix += "..."
+		}
+	}
+
 	// If this candidate needs an additional import statement,
 	// add the additional text edits needed.
 	if cand.imp != nil {
@@ -123,20 +152,6 @@
 		}
 	}
 
-	var prefix, suffix string
-
-	// Prepend "&" or "*" operator as appropriate.
-	if cand.takeAddress {
-		prefix = "&"
-	} else if cand.makePointer {
-		prefix = "*"
-	} else if cand.dereference > 0 {
-		prefix = strings.Repeat("*", cand.dereference)
-	}
-
-	// Include "*" and "&" prefixes in the label.
-	label = prefix + label
-
 	if cand.convertTo != nil {
 		typeName := types.TypeString(cand.convertTo, c.qf)
 
@@ -151,15 +166,6 @@
 		suffix = ")"
 	}
 
-	if cand.takeSlice {
-		suffix += "[:]"
-	}
-
-	// Add variadic "..." only if snippets if enabled or cand is not a function
-	if cand.variadic && (c.opts.snippets || !cand.expandFuncCall) {
-		suffix += "..."
-	}
-
 	if prefix != "" {
 		// If we are in a selector, add an edit to place prefix before selector.
 		if sel := enclosingSelector(c.path, c.pos); sel != nil {
@@ -171,17 +177,13 @@
 		} else {
 			// If there is no selector, just stick the prefix at the start.
 			insert = prefix + insert
-			if snip != nil {
-				snip.PrependText(prefix)
-			}
+			snip.PrependText(prefix)
 		}
 	}
 
 	if suffix != "" {
 		insert += suffix
-		if snip != nil {
-			snip.WriteText(suffix)
-		}
+		snip.WriteText(suffix)
 	}
 
 	detail = strings.TrimPrefix(detail, "untyped ")
@@ -197,7 +199,7 @@
 		Kind:                kind,
 		Score:               cand.score,
 		Depth:               len(cand.path),
-		snippet:             snip,
+		snippet:             &snip,
 		obj:                 obj,
 	}
 	// If the user doesn't want documentation for completion items.
@@ -282,7 +284,8 @@
 			return CompletionItem{}, err
 		}
 		item.Detail = "func" + sig.Format()
-		item.snippet = c.functionCallSnippet(obj.Name(), sig.Params())
+		item.snippet = &snippet.Builder{}
+		c.functionCallSnippet(obj.Name(), sig.Params(), item.snippet)
 	case *types.TypeName:
 		if types.IsInterface(obj.Type()) {
 			item.Kind = protocol.InterfaceCompletion
diff --git a/internal/lsp/source/completion/literal.go b/internal/lsp/source/completion/literal.go
index ddf1de1..0fc7a81 100644
--- a/internal/lsp/source/completion/literal.go
+++ b/internal/lsp/source/completion/literal.go
@@ -103,7 +103,7 @@
 
 	// If prefix matches the type name, client may want a composite literal.
 	if score := c.matcher.Score(matchName); score > 0 {
-		if cand.takeAddress {
+		if cand.hasMod(reference) {
 			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
@@ -144,7 +144,7 @@
 	// If prefix matches "make", client may want a "make()"
 	// invocation. We also include the type name to allow for more
 	// flexible fuzzy matching.
-	if score := c.matcher.Score("make." + matchName); !cand.takeAddress && score > 0 {
+	if score := c.matcher.Score("make." + matchName); !cand.hasMod(reference) && score > 0 {
 		switch literalType.Underlying().(type) {
 		case *types.Slice:
 			// The second argument to "make()" for slices is required, so default to "0".
@@ -157,7 +157,7 @@
 	}
 
 	// If prefix matches "func", client may want a function literal.
-	if score := c.matcher.Score("func"); !cand.takeAddress && score > 0 && !source.IsInterface(expType) {
+	if score := c.matcher.Score("func"); !cand.hasMod(reference) && score > 0 && !source.IsInterface(expType) {
 		switch t := literalType.Underlying().(type) {
 		case *types.Signature:
 			c.functionLiteral(ctx, t, float64(score))
diff --git a/internal/lsp/source/completion/snippet.go b/internal/lsp/source/completion/snippet.go
index 4a4288e..3649314 100644
--- a/internal/lsp/source/completion/snippet.go
+++ b/internal/lsp/source/completion/snippet.go
@@ -11,29 +11,27 @@
 )
 
 // structFieldSnippets calculates the snippet for struct literal field names.
-func (c *completer) structFieldSnippet(cand candidate, label, detail string) *snippet.Builder {
+func (c *completer) structFieldSnippet(cand candidate, detail string, snip *snippet.Builder) {
 	if !c.wantStructFieldCompletions() {
-		return nil
+		return
 	}
 
 	// If we are in a deep completion then we can't be completing a field
 	// name (e.g. "Foo{f<>}" completing to "Foo{f.Bar}" should not generate
 	// a snippet).
 	if len(cand.path) > 0 {
-		return nil
+		return
 	}
 
 	clInfo := c.enclosingCompositeLiteral
 
 	// If we are already in a key-value expression, we don't want a snippet.
 	if clInfo.kv != nil {
-		return nil
+		return
 	}
 
-	snip := &snippet.Builder{}
-
 	// A plain snippet turns "Foo{Ba<>" into "Foo{Bar: <>".
-	snip.WriteText(label + ": ")
+	snip.WriteText(": ")
 	snip.WritePlaceholder(func(b *snippet.Builder) {
 		// A placeholder snippet turns "Foo{Ba<>" into "Foo{Bar: <*int*>".
 		if c.opts.placeholders {
@@ -48,12 +46,10 @@
 	if fset.Position(c.pos).Line != fset.Position(clInfo.cl.Lbrace).Line {
 		snip.WriteText(",")
 	}
-
-	return snip
 }
 
 // functionCallSnippets calculates the snippet for function calls.
-func (c *completer) functionCallSnippet(name string, params []string) *snippet.Builder {
+func (c *completer) functionCallSnippet(name string, params []string, snip *snippet.Builder) {
 	// If there is no suffix then we need to reuse existing call parens
 	// "()" if present. If there is an identifier suffix then we always
 	// need to include "()" since we don't overwrite the suffix.
@@ -66,17 +62,17 @@
 			// inserted when fixing the AST. In this case, we do still need
 			// to insert the calling "()" parens.
 			if n.Fun == c.path[0] && n.Lparen != n.Rparen {
-				return nil
+				return
 			}
 		case *ast.SelectorExpr:
 			if len(c.path) > 2 {
 				if call, ok := c.path[2].(*ast.CallExpr); ok && call.Fun == c.path[1] && call.Lparen != call.Rparen {
-					return nil
+					return
 				}
 			}
 		}
 	}
-	snip := &snippet.Builder{}
+
 	snip.WriteText(name + "(")
 
 	if c.opts.placeholders {
@@ -97,6 +93,4 @@
 	}
 
 	snip.WriteText(")")
-
-	return snip
 }
diff --git a/internal/lsp/testdata/address/address.go b/internal/lsp/testdata/address/address.go
index 59d5d4c..3f1c2fa 100644
--- a/internal/lsp/testdata/address/address.go
+++ b/internal/lsp/testdata/address/address.go
@@ -13,37 +13,32 @@
 		b int    //@item(addrB, "b", "int", "var")
 	)
 
-	&b //@item(addrBRef, "&b", "int", "var")
-
-	wantsPtr()   //@rank(")", addrBRef, addrA),snippet(")", addrBRef, "&b", "&b")
+	wantsPtr()   //@rank(")", addrB, addrA),snippet(")", addrB, "&b", "&b")
 	wantsPtr(&b) //@snippet(")", addrB, "b", "b")
 
-	wantsVariadicPtr() //@rank(")", addrBRef, addrA),snippet(")", addrBRef, "&b", "&b")
+	wantsVariadicPtr() //@rank(")", addrB, addrA),snippet(")", addrB, "&b", "&b")
 
 	var s foo
 	s.c          //@item(addrDeepC, "s.c", "int", "field")
-	&s.c         //@item(addrDeepCRef, "&s.c", "int", "field")
-	wantsPtr()   //@snippet(")", addrDeepCRef, "&s.c", "&s.c")
-	wantsPtr(s)  //@snippet(")", addrDeepCRef, "&s.c", "&s.c")
+	wantsPtr()   //@snippet(")", addrDeepC, "&s.c", "&s.c")
+	wantsPtr(s)  //@snippet(")", addrDeepC, "&s.c", "&s.c")
 	wantsPtr(&s) //@snippet(")", addrDeepC, "s.c", "s.c")
 
 	// don't add "&" in item (it gets added as an additional edit)
 	wantsPtr(&s.c) //@snippet(")", addrFieldC, "c", "c")
 
 	// check dereferencing as well
-	var c *int
-	*c            //@item(addrCPtr, "*c", "*int", "var")
+	var c *int    //@item(addrCPtr, "c", "*int", "var")
 	var _ int = _ //@rank("_ //", addrCPtr, addrA),snippet("_ //", addrCPtr, "*c", "*c")
 
 	wantsVariadic() //@rank(")", addrCPtr, addrA),snippet(")", addrCPtr, "*c", "*c")
 
-	var d **int
-	**d           //@item(addrDPtr, "**d", "**int", "var")
+	var d **int   //@item(addrDPtr, "d", "**int", "var")
 	var _ int = _ //@rank("_ //", addrDPtr, addrA),snippet("_ //", addrDPtr, "**d", "**d")
 
 	type namedPtr *int
-	var np namedPtr
-	*np           //@item(addrNamedPtr, "*np", "namedPtr", "var")
+	var np namedPtr //@item(addrNamedPtr, "np", "namedPtr", "var")
+
 	var _ int = _ //@rank("_ //", addrNamedPtr, addrA)
 
 	// don't get tripped up by recursive pointer type
@@ -62,10 +57,9 @@
 	getFoo().c //@item(addrGetFooC, "getFoo().c", "int", "field")
 
 	// addressable
-	getFoo().ptr().c  //@item(addrGetFooPtrC, "getFoo().ptr().c", "int", "field")
-	&getFoo().ptr().c //@item(addrGetFooPtrCRef, "&getFoo().ptr().c", "int", "field")
+	getFoo().ptr().c //@item(addrGetFooPtrC, "getFoo().ptr().c", "int", "field")
 
-	wantsPtr()   //@rank(addrGetFooPtrCRef, addrGetFooC),snippet(")", addrGetFooPtrCRef, "&getFoo().ptr().c", "&getFoo().ptr().c")
+	wantsPtr()   //@rank(addrGetFooPtrC, addrGetFooC),snippet(")", addrGetFooPtrC, "&getFoo().ptr().c", "&getFoo().ptr().c")
 	wantsPtr(&g) //@rank(addrGetFooPtrC, addrGetFooC),snippet(")", addrGetFooPtrC, "getFoo().ptr().c", "getFoo().ptr().c")
 }
 
@@ -76,8 +70,8 @@
 func _() {
 	getNested := func() nested { return nested{} }
 
-	getNested().f.c        //@item(addrNestedC, "getNested().f.c", "int", "field")
-	&getNested().f.ptr().c //@item(addrNestedPtrC, "&getNested().f.ptr().c", "int", "field")
+	getNested().f.c       //@item(addrNestedC, "getNested().f.c", "int", "field")
+	getNested().f.ptr().c //@item(addrNestedPtrC, "getNested().f.ptr().c", "int", "field")
 
 	// addrNestedC is not addressable, so rank lower
 	wantsPtr(getNestedfc) //@fuzzy(")", addrNestedPtrC, addrNestedC)
diff --git a/internal/lsp/testdata/append/append.go b/internal/lsp/testdata/append/append.go
index 1998f6d..2880e59 100644
--- a/internal/lsp/testdata/append/append.go
+++ b/internal/lsp/testdata/append/append.go
@@ -32,8 +32,7 @@
 	b.b = append(b.b, b) //@rank(")", appendBazzy, appendBazLiteral, appendNestedBaz)
 
 	var aStringsPtr *[]string  //@item(appendStringsPtr, "aStringsPtr", "*[]string", "var")
-	"*aStringsPtr"             //@item(appendStringsDeref, "*aStringsPtr", "*[]string", "var")
-	foo(append([]string{}, a)) //@snippet("))", appendStringsDeref, "*aStringsPtr...", "*aStringsPtr...")
+	foo(append([]string{}, a)) //@snippet("))", appendStringsPtr, "*aStringsPtr...", "*aStringsPtr...")
 
 	foo(append([]string{}, *a)) //@snippet("))", appendStringsPtr, "aStringsPtr...", "aStringsPtr...")
 }
diff --git a/internal/lsp/testdata/complit/complit.go.in b/internal/lsp/testdata/complit/complit.go.in
index c888c01..e819810 100644
--- a/internal/lsp/testdata/complit/complit.go.in
+++ b/internal/lsp/testdata/complit/complit.go.in
@@ -73,14 +73,12 @@
 func _() {
 	type foo struct{} //@item(complitFoo, "foo", "struct{...}", "struct")
 
-	"&foo" //@item(complitAndFoo, "&foo", "struct{...}", "struct")
-
-	var _ *foo = &fo{} //@rank("{", complitFoo)
-	var _ *foo = fo{} //@rank("{", complitAndFoo)
+	var _ *foo = &fo{} //@snippet("{", complitFoo, "foo", "foo")
+	var _ *foo = fo{} //@snippet("{", complitFoo, "&foo", "&foo")
 
 	struct { a, b *foo }{
 		a: &fo{}, //@rank("{", complitFoo)
-		b: fo{}, //@rank("{", complitAndFoo)
+		b: fo{}, //@snippet("{", complitFoo, "&foo", "&foo")
 	}
 }
 
diff --git a/internal/lsp/testdata/deep/deep.go b/internal/lsp/testdata/deep/deep.go
index b713c60..6ed5ff8 100644
--- a/internal/lsp/testdata/deep/deep.go
+++ b/internal/lsp/testdata/deep/deep.go
@@ -34,8 +34,8 @@
 		*deepCircle
 	}
 	var circle deepCircle   //@item(deepCircle, "circle", "deepCircle", "var")
-	*circle.deepCircle      //@item(deepCircleField, "*circle.deepCircle", "*deepCircle", "field")
-	var _ deepCircle = circ //@deep(" //", deepCircle, deepCircleField)
+	circle.deepCircle       //@item(deepCircleField, "circle.deepCircle", "*deepCircle", "field")
+	var _ deepCircle = circ //@deep(" //", deepCircle, deepCircleField),snippet(" //", deepCircleField, "*circle.deepCircle", "*circle.deepCircle")
 }
 
 func _() {
diff --git a/internal/lsp/testdata/maps/maps.go.in b/internal/lsp/testdata/maps/maps.go.in
index b4a4cdd..eeb5576 100644
--- a/internal/lsp/testdata/maps/maps.go.in
+++ b/internal/lsp/testdata/maps/maps.go.in
@@ -11,8 +11,8 @@
 	// comparable
 	type aStruct struct{} //@item(mapStructType, "aStruct", "struct{...}", "struct")
 
-	map[]a{} //@complete("]", mapSliceTypePtr, mapStructType)
+	map[]a{} //@complete("]", mapSliceType, mapStructType),snippet("]", mapSliceType, "*aSlice", "*aSlice")
 
-	map[a]a{} //@complete("]", mapSliceTypePtr, mapStructType)
+	map[a]a{} //@complete("]", mapSliceType, mapStructType)
 	map[a]a{} //@complete("{", mapSliceType, mapStructType)
 }
diff --git a/internal/lsp/testdata/rank/convert_rank.go.in b/internal/lsp/testdata/rank/convert_rank.go.in
index 372d9c3..c430048 100644
--- a/internal/lsp/testdata/rank/convert_rank.go.in
+++ b/internal/lsp/testdata/rank/convert_rank.go.in
@@ -46,8 +46,7 @@
 
 	var _ time.Duration = conv //@rank(" //", convertD, convertE),snippet(" //", convertE, "time.Duration(convE)", "time.Duration(convE)")
 
-	var convP myInt
-	&convP            //@item(convertP, "&convP", "myInt", "var")
+	var convP myInt   //@item(convertP, "convP", "myInt", "var")
 	var _ *int = conv //@snippet(" //", convertP, "(*int)(&convP)", "(*int)(&convP)")
 
 	var ff float64 //@item(convertFloat, "ff", "float64", "var")
diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden
index 24b1f54..170d0dd 100644
--- a/internal/lsp/testdata/summary.txt.golden
+++ b/internal/lsp/testdata/summary.txt.golden
@@ -2,11 +2,11 @@
 CallHierarchyCount = 2
 CodeLensCount = 5
 CompletionsCount = 265
-CompletionSnippetCount = 95
+CompletionSnippetCount = 100
 UnimportedCompletionsCount = 5
 DeepCompletionsCount = 5
 FuzzyCompletionsCount = 8
-RankedCompletionsCount = 166
+RankedCompletionsCount = 163
 CaseSensitiveCompletionsCount = 4
 DiagnosticsCount = 37
 FoldingRangesCount = 2
diff --git a/internal/lsp/testdata/typeassert/type_assert.go b/internal/lsp/testdata/typeassert/type_assert.go
index 0dfd3a1..e24b68a 100644
--- a/internal/lsp/testdata/typeassert/type_assert.go
+++ b/internal/lsp/testdata/typeassert/type_assert.go
@@ -13,14 +13,12 @@
 type abcNotImpl struct{} //@item(abcNotImpl, "abcNotImpl", "struct{...}", "struct")
 
 func _() {
-	*abcPtrImpl //@item(abcPtrImplPtr, "*abcPtrImpl", "struct{...}", "struct")
-
 	var a abc
 	switch a.(type) {
-	case ab: //@complete(":", abcPtrImplPtr, abcImpl, abcIntf, abcNotImpl)
+	case ab: //@complete(":", abcImpl, abcPtrImpl, abcIntf, abcNotImpl)
 	case *ab: //@complete(":", abcImpl, abcPtrImpl, abcIntf, abcNotImpl)
 	}
 
-	a.(ab)  //@complete(")", abcPtrImplPtr, abcImpl, abcIntf, abcNotImpl)
+	a.(ab)  //@complete(")", abcImpl, abcPtrImpl, abcIntf, abcNotImpl)
 	a.(*ab) //@complete(")", abcImpl, abcPtrImpl, abcIntf, abcNotImpl)
 }