internal/lsp: correctly handle type aliases when formatting

This change improves our approach to handling type aliases. Previously,
we were not fully qualifying the names in the AST, making the code
inserted in completions incorrect at times. Now, we clone the relevant
AST expr and qualify it. We also add handling for the return values of a
function, instead of just the parameters.

Fixes golang/go#38230
Fixes golang/go#37283

Change-Id: Ib79f4636891c9b610ae848e9fa4dbae7c63db509
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229319
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>

14 files changed, 373 insertions, 74 deletions

diff --git a/internal/lsp/completion_test.go b/internal/lsp/completion_test.go
index 93ecb27b0..04dc35e84 100644
--- a/internal/lsp/completion_test.go
+++ b/internal/lsp/completion_test.go
@@ -26,7 +26,7 @@ func (r *runner) Completion(t *testing.T, src span.Span, test tests.Completion,
 	got = tests.FilterBuiltins(src, got)
 	want := expected(t, test, items)
 	if diff := tests.DiffCompletionItems(want, got); diff != "" {
-		t.Errorf("%s: %s", src, diff)
+		t.Errorf("%s", diff)
 	}
 }
 
@@ -43,7 +43,7 @@ func (r *runner) CompletionSnippet(t *testing.T, src span.Span, expected tests.C
 		want = expected.PlaceholderSnippet
 	}
 	if diff := tests.DiffSnippets(want, got); diff != "" {
-		t.Errorf("%s: %v", src, diff)
+		t.Errorf("%s", diff)
 	}
 }
 
@@ -52,7 +52,7 @@ func (r *runner) UnimportedCompletion(t *testing.T, src span.Span, test tests.Co
 	got = tests.FilterBuiltins(src, got)
 	want := expected(t, test, items)
 	if diff := tests.CheckCompletionOrder(want, got, false); diff != "" {
-		t.Errorf("%s: %s", src, diff)
+		t.Errorf("%s", diff)
 	}
 }
 
@@ -65,7 +65,7 @@ func (r *runner) DeepCompletion(t *testing.T, src span.Span, test tests.Completi
 	got = tests.FilterBuiltins(src, got)
 	want := expected(t, test, items)
 	if msg := tests.DiffCompletionItems(want, got); msg != "" {
-		t.Errorf("%s: %s", src, msg)
+		t.Errorf("%s", msg)
 	}
 }
 
@@ -78,7 +78,7 @@ func (r *runner) FuzzyCompletion(t *testing.T, src span.Span, test tests.Complet
 	got = tests.FilterBuiltins(src, got)
 	want := expected(t, test, items)
 	if msg := tests.DiffCompletionItems(want, got); msg != "" {
-		t.Errorf("%s: %s", src, msg)
+		t.Errorf("%s", msg)
 	}
 }
 
@@ -90,7 +90,7 @@ func (r *runner) CaseSensitiveCompletion(t *testing.T, src span.Span, test tests
 	got = tests.FilterBuiltins(src, got)
 	want := expected(t, test, items)
 	if msg := tests.DiffCompletionItems(want, got); msg != "" {
-		t.Errorf("%s: %s", src, msg)
+		t.Errorf("%s", msg)
 	}
 }
 
@@ -102,7 +102,7 @@ func (r *runner) RankCompletion(t *testing.T, src span.Span, test tests.Completi
 	})
 	want := expected(t, test, items)
 	if msg := tests.CheckCompletionOrder(want, got, true); msg != "" {
-		t.Errorf("%s: %s", src, msg)
+		t.Errorf("%s", msg)
 	}
 }
 
diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go
index 9b2c0c760..c8a275608 100644
--- a/internal/lsp/source/completion_format.go
+++ b/internal/lsp/source/completion_format.go
@@ -43,10 +43,10 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e
 
 	// expandFuncCall mutates the completion label, detail, and snippet
 	// to that of an invocation of sig.
-	expandFuncCall := func(sig *types.Signature) {
+	expandFuncCall := func(sig *types.Signature) error {
 		s, err := newSignature(ctx, c.snapshot, c.pkg, "", sig, nil, c.qf)
 		if err != nil {
-			return
+			return err
 		}
 		snip = c.functionCallSnippet(label, s.params)
 		detail = "func" + s.format()
@@ -55,6 +55,7 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e
 		if sig.Results().Len() == 1 && c.inference.matchesVariadic(sig.Results().At(0).Type()) {
 			snip.WriteText("...")
 		}
+		return nil
 	}
 
 	switch obj := obj.(type) {
@@ -66,11 +67,7 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e
 		if _, ok := obj.Type().(*types.Struct); ok {
 			detail = "struct{...}" // for anonymous structs
 		} else if obj.IsField() {
-			var err error
-			detail, err = formatFieldType(ctx, c.snapshot, c.pkg, obj)
-			if err != nil {
-				detail = types.TypeString(obj.Type(), c.qf)
-			}
+			detail = formatVarType(ctx, c.snapshot, c.pkg, obj, c.qf)
 		}
 		if obj.IsField() {
 			kind = protocol.FieldCompletion
@@ -83,7 +80,9 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e
 		}
 
 		if sig, ok := obj.Type().Underlying().(*types.Signature); ok && cand.expandFuncCall {
-			expandFuncCall(sig)
+			if err := expandFuncCall(sig); err != nil {
+				return CompletionItem{}, err
+			}
 		}
 
 		// Add variadic "..." if we are using a variable to fill in a variadic parameter.
@@ -102,7 +101,9 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e
 		}
 
 		if cand.expandFuncCall {
-			expandFuncCall(sig)
+			if err := expandFuncCall(sig); err != nil {
+				return CompletionItem{}, err
+			}
 		}
 	case *types.PkgName:
 		kind = protocol.ModuleCompletion
diff --git a/internal/lsp/source/types_format.go b/internal/lsp/source/types_format.go
index 51c6b9f30..51b975702 100644
--- a/internal/lsp/source/types_format.go
+++ b/internal/lsp/source/types_format.go
@@ -9,11 +9,12 @@ import (
 	"context"
 	"fmt"
 	"go/ast"
-	"go/doc"
 	"go/printer"
 	"go/types"
 	"strings"
+	"unicode"
 
+	"golang.org/x/tools/go/ast/astutil"
 	"golang.org/x/tools/internal/event"
 	"golang.org/x/tools/internal/lsp/debug/tag"
 	"golang.org/x/tools/internal/lsp/protocol"
@@ -105,18 +106,55 @@ func newBuiltinSignature(ctx context.Context, view View, name string) (*signatur
 	}, nil
 }
 
+var replacer = strings.NewReplacer(
+	`ComplexType`, `complex128`,
+	`FloatType`, `float64`,
+	`IntegerType`, `int`,
+)
+
+func formatFieldList(ctx context.Context, view View, list *ast.FieldList, variadic bool) ([]string, bool) {
+	if list == nil {
+		return nil, false
+	}
+	var writeResultParens bool
+	var result []string
+	for i := 0; i < len(list.List); i++ {
+		if i >= 1 {
+			writeResultParens = true
+		}
+		p := list.List[i]
+		cfg := printer.Config{Mode: printer.UseSpaces | printer.TabIndent, Tabwidth: 4}
+		b := &bytes.Buffer{}
+		if err := cfg.Fprint(b, view.Session().Cache().FileSet(), p.Type); err != nil {
+			event.Error(ctx, "unable to print type", nil, tag.Type.Of(p.Type))
+			continue
+		}
+		typ := replacer.Replace(b.String())
+		if len(p.Names) == 0 {
+			result = append(result, typ)
+		}
+		for _, name := range p.Names {
+			if name.Name != "" {
+				if i == 0 {
+					writeResultParens = true
+				}
+				result = append(result, fmt.Sprintf("%s %s", name.Name, typ))
+			} else {
+				result = append(result, typ)
+			}
+		}
+	}
+	if variadic {
+		result[len(result)-1] = strings.Replace(result[len(result)-1], "[]", "...", 1)
+	}
+	return result, writeResultParens
+}
+
 func newSignature(ctx context.Context, s Snapshot, pkg Package, name string, sig *types.Signature, comment *ast.CommentGroup, qf types.Qualifier) (*signature, error) {
 	params := make([]string, 0, sig.Params().Len())
 	for i := 0; i < sig.Params().Len(); i++ {
 		el := sig.Params().At(i)
-		typ, err := formatFieldType(ctx, s, pkg, el)
-		if err != nil {
-			typ = types.TypeString(el.Type(), qf)
-		}
-		// Handle a variadic parameter (can only be the final parameter).
-		if sig.Variadic() && i == sig.Params().Len()-1 {
-			typ = strings.Replace(typ, "[]", "...", 1)
-		}
+		typ := formatVarType(ctx, s, pkg, el, qf)
 		p := typ
 		if el.Name() != "" {
 			p = el.Name() + " " + typ
@@ -130,8 +168,7 @@ func newSignature(ctx context.Context, s Snapshot, pkg Package, name string, sig
 			needResultParens = true
 		}
 		el := sig.Results().At(i)
-		typ := types.TypeString(el.Type(), qf)
-
+		typ := formatVarType(ctx, s, pkg, el, qf)
 		if el.Name() == "" {
 			results = append(results, typ)
 		} else {
@@ -143,7 +180,7 @@ func newSignature(ctx context.Context, s Snapshot, pkg Package, name string, sig
 	}
 	var c string
 	if comment != nil {
-		c = doc.Synopsis(comment.Text())
+		c = comment.Text()
 	}
 	return &signature{
 		doc:              c,
@@ -154,65 +191,228 @@ func newSignature(ctx context.Context, s Snapshot, pkg Package, name string, sig
 	}, nil
 }
 
-func formatFieldType(ctx context.Context, s Snapshot, srcpkg Package, obj *types.Var) (string, error) {
+// formatVarType formats a *types.Var, accounting for type aliases.
+// To do this, it looks in the AST of the file in which the object is declared.
+// On any errors, it always fallbacks back to types.TypeString.
+func formatVarType(ctx context.Context, s Snapshot, srcpkg Package, obj *types.Var, qf types.Qualifier) string {
 	file, pkg, err := findPosInPackage(s.View(), srcpkg, obj.Pos())
 	if err != nil {
-		return "", err
+		return types.TypeString(obj.Type(), qf)
+	}
+	// Named and unnamed variables must be handled differently.
+	// Unnamed variables appear in the result values of a function signature.
+	var expr ast.Expr
+	if obj.Name() != "" {
+		expr, err = namedVarType(ctx, s, pkg, file, obj)
+	} else {
+		expr, err = unnamedVarType(file, obj)
+	}
+	if err != nil {
+		return types.TypeString(obj.Type(), qf)
 	}
+	// The type names in the AST may not be correctly qualified.
+	// Determine the package name to use based on the package that originated
+	// the query and the package in which the type is declared.
+	// We then qualify the value by cloning the AST node and editing it.
+	pkgName := importedPkgName(s, srcpkg, pkg, file)
+	cloned := cloneExpr(expr)
+	qualified := qualifyExpr(cloned, pkgName)
+	fmted := formatNode(s.View().Session().Cache().FileSet(), qualified)
+	return fmted
+}
+
+// unnamedVarType finds the type for an unnamed variable.
+func unnamedVarType(file *ast.File, obj *types.Var) (ast.Expr, error) {
+	path, _ := astutil.PathEnclosingInterval(file, obj.Pos(), obj.Pos())
+	var expr ast.Expr
+	for _, p := range path {
+		e, ok := p.(ast.Expr)
+		if !ok {
+			break
+		}
+		expr = e
+	}
+	typ, ok := expr.(ast.Expr)
+	if !ok {
+		return nil, fmt.Errorf("unexpected type for node (%T)", path[0])
+	}
+	return typ, nil
+}
+
+// namedVarType returns the type for a named variable.
+func namedVarType(ctx context.Context, s Snapshot, pkg Package, file *ast.File, obj *types.Var) (ast.Expr, error) {
 	ident, err := findIdentifier(ctx, s, pkg, file, obj.Pos())
 	if err != nil {
-		return "", err
+		return nil, err
+	}
+	if ident.Declaration.obj != obj {
+		return nil, fmt.Errorf("expected the ident's declaration %v to be equal to obj %v", ident.Declaration.obj, obj)
 	}
 	if i := ident.ident; i == nil || i.Obj == nil || i.Obj.Decl == nil {
-		return "", fmt.Errorf("no object for ident %v", i.Name)
+		return nil, fmt.Errorf("no declaration for object %s", obj.Name())
 	}
 	f, ok := ident.ident.Obj.Decl.(*ast.Field)
 	if !ok {
-		return "", fmt.Errorf("ident %s is not a field type", ident.Name)
+		return nil, fmt.Errorf("declaration of object %v is %T, not *ast.Field", obj.Name(), ident.ident.Obj.Decl)
 	}
-	return formatNode(s.View().Session().Cache().FileSet(), f.Type), nil
+	typ, ok := f.Type.(ast.Expr)
+	if !ok {
+		return nil, fmt.Errorf("unexpected type for node (%T)", f.Type)
+	}
+	return typ, nil
 }
 
-var replacer = strings.NewReplacer(
-	`ComplexType`, `complex128`,
-	`FloatType`, `float64`,
-	`IntegerType`, `int`,
-)
+// importedPkgName returns the package name used for pkg in srcpkg.
+func importedPkgName(s Snapshot, srcpkg, pkg Package, file *ast.File) string {
+	if srcpkg == pkg {
+		return ""
+	}
+	// If the file already imports the package under another name, use that.
+	for _, group := range astutil.Imports(s.View().Session().Cache().FileSet(), file) {
+		for _, cand := range group {
+			if strings.Trim(cand.Path.Value, `"`) == pkg.PkgPath() {
+				if cand.Name != nil && cand.Name.Name != "" {
+					return cand.Name.Name
+				}
+			}
+		}
+	}
+	return pkg.GetTypes().Name()
+}
 
-func formatFieldList(ctx context.Context, view View, list *ast.FieldList, variadic bool) ([]string, bool) {
-	if list == nil {
-		return nil, false
+// qualifyExpr applies the "pkgName." prefix to any *ast.Ident in the expr.
+func qualifyExpr(expr ast.Expr, pkgName string) ast.Expr {
+	if pkgName == "" {
+		return expr
 	}
-	var writeResultParens bool
-	var result []string
-	for i := 0; i < len(list.List); i++ {
-		if i >= 1 {
-			writeResultParens = true
+	ast.Inspect(expr, func(n ast.Node) bool {
+		switch n := n.(type) {
+		case *ast.ArrayType, *ast.ChanType, *ast.Ellipsis,
+			*ast.FuncType, *ast.MapType, *ast.ParenExpr,
+			*ast.StarExpr, *ast.StructType:
+			// These are the only types that are cloned by cloneExpr below,
+			// so these are the only types that we can traverse and potentially
+			// modify. This is not an ideal approach, but it works for now.
+			return true
+		case *ast.SelectorExpr:
+			// Don't add qualifiers to selectors.
+			return false
+		case *ast.Ident:
+			// Only add the qualifier if the identifier is exported.
+			if unicode.IsUpper(rune(n.Name[0])) {
+				n.Name = pkgName + "." + n.Name
+			}
 		}
-		p := list.List[i]
-		cfg := printer.Config{Mode: printer.UseSpaces | printer.TabIndent, Tabwidth: 4}
-		b := &bytes.Buffer{}
-		if err := cfg.Fprint(b, view.Session().Cache().FileSet(), p.Type); err != nil {
-			event.Error(ctx, "unable to print type", nil, tag.Type.Of(p.Type))
-			continue
+		return false
+	})
+	return expr
+}
+
+// cloneExpr only clones expressions that appear in the parameters or return
+// values of a function declaration. The original expression may be returned
+// to the caller in 2 cases:
+//    (1) The expression has no pointer fields.
+//    (2) The expression cannot appear in an *ast.FuncType, making it
+//        unnecessary to clone.
+// NOTE: This function is tailored to the use case of qualifyExpr, and should
+// be used with caution.
+func cloneExpr(expr ast.Expr) ast.Expr {
+	switch expr := expr.(type) {
+	case *ast.ArrayType:
+		return &ast.ArrayType{
+			Lbrack: expr.Lbrack,
+			Elt:    cloneExpr(expr.Elt),
+			Len:    expr.Len,
 		}
-		typ := replacer.Replace(b.String())
-		if len(p.Names) == 0 {
-			result = append(result, typ)
+	case *ast.ChanType:
+		return &ast.ChanType{
+			Arrow: expr.Arrow,
+			Begin: expr.Begin,
+			Dir:   expr.Dir,
+			Value: cloneExpr(expr.Value),
 		}
-		for _, name := range p.Names {
-			if name.Name != "" {
-				if i == 0 {
-					writeResultParens = true
-				}
-				result = append(result, fmt.Sprintf("%s %s", name.Name, typ))
-			} else {
-				result = append(result, typ)
-			}
+	case *ast.Ellipsis:
+		return &ast.Ellipsis{
+			Ellipsis: expr.Ellipsis,
+			Elt:      cloneExpr(expr.Elt),
+		}
+	case *ast.FuncType:
+		return &ast.FuncType{
+			Func:    expr.Func,
+			Params:  cloneFieldList(expr.Params),
+			Results: cloneFieldList(expr.Results),
+		}
+	case *ast.Ident:
+		return cloneIdent(expr)
+	case *ast.MapType:
+		return &ast.MapType{
+			Map:   expr.Map,
+			Key:   cloneExpr(expr.Key),
+			Value: cloneExpr(expr.Value),
+		}
+	case *ast.ParenExpr:
+		return &ast.ParenExpr{
+			Lparen: expr.Lparen,
+			Rparen: expr.Rparen,
+			X:      cloneExpr(expr.X),
+		}
+	case *ast.SelectorExpr:
+		return &ast.SelectorExpr{
+			Sel: cloneIdent(expr.Sel),
+			X:   cloneExpr(expr.X),
 		}
+	case *ast.StarExpr:
+		return &ast.StarExpr{
+			Star: expr.Star,
+			X:    cloneExpr(expr.X),
+		}
+	case *ast.StructType:
+		return &ast.StructType{
+			Struct:     expr.Struct,
+			Fields:     cloneFieldList(expr.Fields),
+			Incomplete: expr.Incomplete,
+		}
+	default:
+		// Not in function literals or don't need to be cloned.
+		return expr
 	}
-	if variadic {
-		result[len(result)-1] = strings.Replace(result[len(result)-1], "[]", "...", 1)
+}
+
+func cloneFieldList(fl *ast.FieldList) *ast.FieldList {
+	if fl == nil {
+		return nil
+	}
+	if fl.List == nil {
+		return &ast.FieldList{
+			Closing: fl.Closing,
+			Opening: fl.Opening,
+		}
+	}
+	list := make([]*ast.Field, 0, len(fl.List))
+	for _, f := range fl.List {
+		var names []*ast.Ident
+		for _, n := range f.Names {
+			names = append(names, cloneIdent(n))
+		}
+		list = append(list, &ast.Field{
+			Comment: f.Comment,
+			Doc:     f.Doc,
+			Names:   names,
+			Tag:     f.Tag,
+			Type:    cloneExpr(f.Type),
+		})
+	}
+	return &ast.FieldList{
+		Closing: fl.Closing,
+		Opening: fl.Opening,
+		List:    list,
+	}
+}
+
+func cloneIdent(ident *ast.Ident) *ast.Ident {
+	return &ast.Ident{
+		NamePos: ident.NamePos,
+		Name:    ident.Name,
+		Obj:     ident.Obj,
 	}
-	return result, writeResultParens
 }
diff --git a/internal/lsp/testdata/lsp/primarymod/good/good1.go b/internal/lsp/testdata/lsp/primarymod/good/good1.go
index b6180ebab..bdccaed63 100644
--- a/internal/lsp/testdata/lsp/primarymod/good/good1.go
+++ b/internal/lsp/testdata/lsp/primarymod/good/good1.go
@@ -14,7 +14,7 @@ func random() int { //@item(good_random, "random", "func() int", "func")
 func random2(y int) int { //@item(good_random2, "random2", "func(y int) int", "func"),item(good_y_param, "y", "int", "var")
 	//@complete("", good_y_param, types_import, good_random, good_random2, good_stuff)
 	var b types.Bob = &types.X{}   //@prepare("ypes","types", "types")
-	if _, ok := b.(*types.X); ok { //@complete("X", X_struct, Y_struct, Bob_interface)
+	if _, ok := b.(*types.X); ok { //@complete("X", X_struct, Y_struct, Bob_interface, CoolAlias)
 	}
 
 	return y
diff --git a/internal/lsp/testdata/lsp/primarymod/signature/signature.go b/internal/lsp/testdata/lsp/primarymod/signature/signature.go
index 716df9cf1..05f8da2fe 100644
--- a/internal/lsp/testdata/lsp/primarymod/signature/signature.go
+++ b/internal/lsp/testdata/lsp/primarymod/signature/signature.go
@@ -20,8 +20,18 @@ func (*myStruct) foo(e *json.Decoder) (*big.Int, error) {
 	return nil, nil
 }
 
+type MyType struct{}
+
 type MyFunc func(foo int) string
 
+type Alias = int
+type OtherAlias = int
+type StringAlias = string
+
+func AliasSlice(a []*Alias) (b Alias)                                 { return 0 }
+func AliasMap(a map[*Alias]StringAlias) (b, c map[*Alias]StringAlias) { return nil, nil }
+func OtherAliasMap(a, b map[Alias]OtherAlias) map[Alias]OtherAlias    { return nil }
+
 func Qux() {
 	Foo("foo", 123) //@signature("(", "Foo(a string, b int) (c bool)", 0)
 	Foo("foo", 123) //@signature("123", "Foo(a string, b int) (c bool)", 1)
@@ -66,6 +76,9 @@ func Qux() {
 		//@signature("//", "", 0)
 	})
 
+	AliasSlice()    //@signature(")", "AliasSlice(a []*Alias) (b Alias)", 0)
+	AliasMap()      //@signature(")", "AliasMap(a map[*Alias]StringAlias) (b map[*Alias]StringAlias, c map[*Alias]StringAlias)", 0)
+	OtherAliasMap() //@signature(")", "OtherAliasMap(a map[Alias]OtherAlias, b map[Alias]OtherAlias) map[Alias]OtherAlias", 0)
 }
 
 func Hello(func()) {}
diff --git a/internal/lsp/testdata/lsp/primarymod/signature/signature.go.golden b/internal/lsp/testdata/lsp/primarymod/signature/signature.go.golden
index 387abf6dc..486ca7f68 100644
--- a/internal/lsp/testdata/lsp/primarymod/signature/signature.go.golden
+++ b/internal/lsp/testdata/lsp/primarymod/signature/signature.go.golden
@@ -1,14 +1,35 @@
+-- AliasMap(a map[*Alias]StringAlias) (b map[*Alias]StringAlias, c map[*Alias]StringAlias)-signature --
+AliasMap(a map[*Alias]StringAlias) (b map[*Alias]StringAlias, c map[*Alias]StringAlias)
+
+-- AliasSlice(a []*Alias) (b Alias)-signature --
+AliasSlice(a []*Alias) (b Alias)
+
 -- Bar(float64, ...byte)-signature --
 Bar(float64, ...byte)
 
 -- Foo(a string, b int) (c bool)-signature --
 Foo(a string, b int) (c bool)
 
+-- GetAlias() Alias-signature --
+GetAlias() Alias
+
+-- GetAliasPtr() *Alias-signature --
+GetAliasPtr() *Alias
+
 -- Next(n int) []byte-signature --
 Next(n int) []byte
 
 Next returns a slice containing the next n bytes from the buffer, advancing the buffer as if the bytes had been returned by Read.
 
+-- OtherAliasMap(a map[Alias]OtherAlias, b map[Alias]OtherAlias) map[Alias]OtherAlias-signature --
+OtherAliasMap(a map[Alias]OtherAlias, b map[Alias]OtherAlias) map[Alias]OtherAlias
+
+-- SetAliasSlice(a []*Alias)-signature --
+SetAliasSlice(a []*Alias)
+
+-- SetOtherAliasMap(a map[*Alias]OtherAlias)-signature --
+SetOtherAliasMap(a map[*Alias]OtherAlias)
+
 -- fn(hi string, there string) func(i int) rune-signature --
 fn(hi string, there string) func(i int) rune
 
diff --git a/internal/lsp/testdata/lsp/primarymod/signature/signature_test.go b/internal/lsp/testdata/lsp/primarymod/signature/signature_test.go
new file mode 100644
index 000000000..384f08982
--- /dev/null
+++ b/internal/lsp/testdata/lsp/primarymod/signature/signature_test.go
@@ -0,0 +1,13 @@
+package signature_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/internal/lsp/signature"
+)
+
+func TestSignature(t *testing.T) {
+	signature.AliasSlice()    //@signature(")", "AliasSlice(a []*signature.Alias) (b signature.Alias)", 0)
+	signature.AliasMap()      //@signature(")", "AliasMap(a map[*signature.Alias]signature.StringAlias) (b map[*signature.Alias]signature.StringAlias, c map[*signature.Alias]signature.StringAlias)", 0)
+	signature.OtherAliasMap() //@signature(")", "OtherAliasMap(a map[signature.Alias]signature.OtherAlias, b map[signature.Alias]signature.OtherAlias) map[signature.Alias]signature.OtherAlias", 0)
+}
diff --git a/internal/lsp/testdata/lsp/primarymod/signature/signature_test.go.golden b/internal/lsp/testdata/lsp/primarymod/signature/signature_test.go.golden
new file mode 100644
index 000000000..af6f08aa6
--- /dev/null
+++ b/internal/lsp/testdata/lsp/primarymod/signature/signature_test.go.golden
@@ -0,0 +1,21 @@
+-- AliasMap(a map[*signature.Alias]signature.StringAlias) (b map[*signature.Alias]signature.StringAlias, c map[*signature.Alias]signature.StringAlias)-signature --
+AliasMap(a map[*signature.Alias]signature.StringAlias) (b map[*signature.Alias]signature.StringAlias, c map[*signature.Alias]signature.StringAlias)
+
+-- AliasSlice(a []*signature.Alias) (b signature.Alias)-signature --
+AliasSlice(a []*signature.Alias) (b signature.Alias)
+
+-- GetAlias() signature.Alias-signature --
+GetAlias() signature.Alias
+
+-- GetAliasPtr() *signature.Alias-signature --
+GetAliasPtr() *signature.Alias
+
+-- OtherAliasMap(a map[signature.Alias]signature.OtherAlias, b map[signature.Alias]signature.OtherAlias) map[signature.Alias]signature.OtherAlias-signature --
+OtherAliasMap(a map[signature.Alias]signature.OtherAlias, b map[signature.Alias]signature.OtherAlias) map[signature.Alias]signature.OtherAlias
+
+-- SetAliasSlice(a []*signature.Alias)-signature --
+SetAliasSlice(a []*signature.Alias)
+
+-- SetOtherAliasMap(a map[*signature.Alias]signature.OtherAlias)-signature --
+SetOtherAliasMap(a map[*signature.Alias]signature.OtherAlias)
+
diff --git a/internal/lsp/testdata/lsp/primarymod/snippets/literal.go b/internal/lsp/testdata/lsp/primarymod/snippets/literal.go
new file mode 100644
index 000000000..00ad51d40
--- /dev/null
+++ b/internal/lsp/testdata/lsp/primarymod/snippets/literal.go
@@ -0,0 +1,22 @@
+package snippets
+
+import (
+	"golang.org/x/tools/internal/lsp/signature"
+	"golang.org/x/tools/internal/lsp/types"
+)
+
+type structy struct {
+	x signature.MyType
+}
+
+func X(_ map[signatures.Alias]types.CoolAlias) (map[signatures.Alias]types.CoolAlias) {
+	return nil
+}
+
+func _() {
+	X() //@signature(")", "X(_ map[signatures.Alias]types.CoolAlias) map[signatures.Alias]types.CoolAlias", 0)
+	_ = signature.MyType{} //@item(literalMyType, "signature.MyType{}", "", "var")
+	s := structy{
+		x: //@snippet(" //", literalMyType, "signature.MyType{\\}", "signature.MyType{\\}")
+	}
+}
\ No newline at end of file
diff --git a/internal/lsp/testdata/lsp/primarymod/snippets/literal.go.golden b/internal/lsp/testdata/lsp/primarymod/snippets/literal.go.golden
new file mode 100644
index 000000000..6dbe1bae8
--- /dev/null
+++ b/internal/lsp/testdata/lsp/primarymod/snippets/literal.go.golden
@@ -0,0 +1,3 @@
+-- X(_ map[signatures.Alias]types.CoolAlias) map[signatures.Alias]types.CoolAlias-signature --
+X(_ map[signatures.Alias]types.CoolAlias) map[signatures.Alias]types.CoolAlias
+
diff --git a/internal/lsp/testdata/lsp/primarymod/types/types.go b/internal/lsp/testdata/lsp/primarymod/types/types.go
index 6a58328e6..c60d4b2e4 100644
--- a/internal/lsp/testdata/lsp/primarymod/types/types.go
+++ b/internal/lsp/testdata/lsp/primarymod/types/types.go
@@ -1,5 +1,7 @@
 package types
 
+type CoolAlias = int //@item(CoolAlias, "CoolAlias", "int", "type")
+
 type X struct { //@item(X_struct, "X", "struct{...}", "struct")
 	x int
 }
diff --git a/internal/lsp/testdata/lsp/summary.txt.golden b/internal/lsp/testdata/lsp/summary.txt.golden
index 5b902ce28..f1575fef8 100644
--- a/internal/lsp/testdata/lsp/summary.txt.golden
+++ b/internal/lsp/testdata/lsp/summary.txt.golden
@@ -1,7 +1,7 @@
 -- summary --
 CodeLensCount = 2
 CompletionsCount = 237
-CompletionSnippetCount = 74
+CompletionSnippetCount = 75
 UnimportedCompletionsCount = 6
 DeepCompletionsCount = 5
 FuzzyCompletionsCount = 8
@@ -22,7 +22,7 @@ SymbolsCount = 3
 WorkspaceSymbolsCount = 2
 FuzzyWorkspaceSymbolsCount = 3
 CaseSensitiveWorkspaceSymbolsCount = 2
-SignaturesCount = 24
+SignaturesCount = 31
 LinksCount = 8
 ImplementationsCount = 14
 
diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go
index 9d6253f66..3af5292b4 100644
--- a/internal/lsp/tests/tests.go
+++ b/internal/lsp/tests/tests.go
@@ -231,7 +231,7 @@ func DefaultOptions() source.Options {
 
 var haveCgo = false
 
-// For Load() to properly create the folder structure required when testing with modules.
+// Load creates the folder structure required when testing with modules.
 // The directory structure of a test needs to look like the example below:
 //
 // - dir
diff --git a/internal/lsp/tests/util.go b/internal/lsp/tests/util.go
index 472b93c94..58b83280f 100644
--- a/internal/lsp/tests/util.go
+++ b/internal/lsp/tests/util.go
@@ -14,6 +14,8 @@ import (
 	"strings"
 	"testing"
 
+	"golang.org/x/tools/internal/lsp/diff"
+	"golang.org/x/tools/internal/lsp/diff/myers"
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/span"
@@ -279,7 +281,7 @@ func summarizeCodeLens(i int, uri span.URI, want, got []protocol.CodeLens, reaso
 
 func DiffSignatures(spn span.Span, want, got *protocol.SignatureHelp) string {
 	decorate := func(f string, args ...interface{}) string {
-		return fmt.Sprintf("Invalid signature at %s: %s", spn, fmt.Sprintf(f, args...))
+		return fmt.Sprintf("invalid signature at %s: %s", spn, fmt.Sprintf(f, args...))
 	}
 
 	if len(got.Signatures) != 1 {
@@ -297,7 +299,8 @@ func DiffSignatures(spn span.Span, want, got *protocol.SignatureHelp) string {
 	gotSig := got.Signatures[int(got.ActiveSignature)]
 
 	if want.Signatures[0].Label != got.Signatures[0].Label {
-		return decorate("wanted label %q, got %q", want.Signatures[0].Label, got.Signatures[0].Label)
+		d := myers.ComputeEdits("", want.Signatures[0].Label, got.Signatures[0].Label)
+		return decorate("mismatched labels:\n%q", diff.ToUnified("want", "got", want.Signatures[0].Label, d))
 	}
 
 	var paramParts []string