lsp/completion: fix literal completions with type params

In cases like:

    type foo[T any] struct{}
    func bar[T any](foo[T]) {}

    bar[int](<A>)
    bar(<B>)

At <A> we will now offer "foo[int]{}". At <B> we will now offer a
snippet "foo[<T>]{}" which lets the user fill in the type arg.

Note that we have no knowledge of type inference, so you can be left
with superfluous type args after completion.

Change-Id: Ia7d63284f3317d9367864fdae3e3f9ae68fdff1a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/400615
Run-TryBot: Muir Manders <muir@mnd.rs>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>

internal/lsp/snippet/snippet_builder.go

@@ -96,6 +96,13 @@ func (b *Builder) String() string {
 	return b.sb.String()
 }
 
+// Clone returns a copy of b.
+func (b *Builder) Clone() *Builder {
+	var clone Builder
+	clone.sb.WriteString(b.String())
+	return &clone
+}
+
 // nextTabStop returns the next tab stop index for a new placeholder.
 func (b *Builder) nextTabStop() int {
 	// Tab stops start from 1, so increment before returning.

internal/lsp/source/completion/completion.go

@@ -221,6 +221,12 @@ type completer struct {
 	// startTime is when we started processing this completion request. It does
 	// not include any time the request spent in the queue.
 	startTime time.Time
+
+	// scopes contains all scopes defined by nodes in our path,
+	// including nil values for nodes that don't defined a scope. It
+	// also includes our package scope and the universal scope at the
+	// end.
+	scopes []*types.Scope
 }
 
 // funcInfo holds info about a function object.
@@ -497,6 +503,10 @@ func Completion(ctx context.Context, snapshot source.Snapshot, fh source.FileHan
 		}
 	}
 
+	// Collect all surrounding scopes, innermost first.
+	scopes := source.CollectScopes(pkg.GetTypesInfo(), path, pos)
+	scopes = append(scopes, pkg.GetTypes().Scope(), types.Universe)
+
 	opts := snapshot.View().Options()
 	c := &completer{
 		pkg:      pkg,
@@ -533,6 +543,7 @@ func Completion(ctx context.Context, snapshot source.Snapshot, fh source.FileHan
 		methodSetCache: make(map[methodSetKey]*types.MethodSet),
 		mapper:         pgf.Mapper,
 		startTime:      startTime,
+		scopes:         scopes,
 	}
 
 	var cancel context.CancelFunc
@@ -1267,9 +1278,6 @@ func (c *completer) methodsAndFields(typ types.Type, addressable bool, imp *impo
 
 // lexical finds completions in the lexical environment.
 func (c *completer) lexical(ctx context.Context) error {
-	scopes := source.CollectScopes(c.pkg.GetTypesInfo(), c.path, c.pos)
-	scopes = append(scopes, c.pkg.GetTypes().Scope(), types.Universe)
-
 	var (
 		builtinIota = types.Universe.Lookup("iota")
 		builtinNil  = types.Universe.Lookup("nil")
@@ -1286,7 +1294,7 @@ func (c *completer) lexical(ctx context.Context) error {
 	seen := make(map[string]struct{})
 
 	// Process scopes innermost first.
-	for i, scope := range scopes {
+	for i, scope := range c.scopes {
 		if scope == nil {
 			continue
 		}
@@ -1407,10 +1415,11 @@ func (c *completer) lexical(ctx context.Context) error {
 	return nil
 }
 
-// injectInferredType manufacters candidates based on the given type.
-// For example, if the type is "[]int", this method makes sure you get
-// candidates "[]int{}" and "[]int" (the latter applies when
-// completing a type name).
+// injectType manufacters candidates based on the given type. This is
+// intended for types not discoverable via lexical search, such as
+// composite and/or generic types. For example, if the type is "[]int",
+// this method makes sure you get candidates "[]int{}" and "[]int"
+// (the latter applies when completing a type name).
 func (c *completer) injectType(ctx context.Context, t types.Type) {
 	if t == nil {
 		return
@@ -1418,11 +1427,12 @@ func (c *completer) injectType(ctx context.Context, t types.Type) {
 
 	t = source.Deref(t)
 
-	// If we have an expected type and it is _not_ a named type,
-	// handle it specially. Non-named types like "[]int" will never be
+	// If we have an expected type and it is _not_ a named type, handle
+	// it specially. Non-named types like "[]int" will never be
 	// considered via a lexical search, so we need to directly inject
-	// them.
-	if _, named := t.(*types.Named); !named {
+	// them. Also allow generic types since lexical search does not
+	// infer instantiated versions of them.
+	if named, _ := t.(*types.Named); named == nil || typeparams.ForNamed(named).Len() > 0 {
 		// If our expected type is "[]int", this will add a literal
 		// candidate of "[]int{}".
 		c.literal(ctx, t, nil)
@@ -2990,3 +3000,13 @@ func candKind(candType types.Type) objKind {
 
 	return kind
 }
+
+// innermostScope returns the innermost scope for c.pos.
+func (c *completer) innermostScope() *types.Scope {
+	for _, s := range c.scopes {
+		if s != nil {
+			return s
+		}
+	}
+	return nil
+}

internal/lsp/source/completion/literal.go

@@ -15,6 +15,7 @@ import (
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/snippet"
 	"golang.org/x/tools/internal/lsp/source"
+	"golang.org/x/tools/internal/typeparams"
 )
 
 // literal generates composite literal, function literal, and make()
@@ -82,7 +83,7 @@ func (c *completer) literal(ctx context.Context, literalType types.Type, imp *im
 		qf = func(_ *types.Package) string { return "" }
 	}
 
-	typeName := types.TypeString(literalType, qf)
+	snip, typeName := c.typeNameSnippet(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
@@ -117,18 +118,19 @@ func (c *completer) literal(ctx context.Context, literalType types.Type, imp *im
 			} else {
 				// Otherwise we can stick the "&" directly before the type name.
 				typeName = "&" + typeName
+				snip.PrependText("&")
 			}
 		}
 
 		switch t := literalType.Underlying().(type) {
 		case *types.Struct, *types.Array, *types.Slice, *types.Map:
-			c.compositeLiteral(t, typeName, float64(score), addlEdits)
+			c.compositeLiteral(t, snip.Clone(), typeName, float64(score), addlEdits)
 		case *types.Signature:
 			// Add a literal completion for a signature type that implements
 			// an interface. For example, offer "http.HandlerFunc()" when
 			// expected type is "http.Handler".
 			if source.IsInterface(expType) {
-				c.basicLiteral(t, typeName, float64(score), addlEdits)
+				c.basicLiteral(t, snip.Clone(), typeName, float64(score), addlEdits)
 			}
 		case *types.Basic:
 			// Add a literal completion for basic types that implement our
@@ -136,7 +138,7 @@ func (c *completer) literal(ctx context.Context, literalType types.Type, imp *im
 			// implements http.FileSystem), or are identical to our expected
 			// type (i.e. yielding a type conversion such as "float64()").
 			if source.IsInterface(expType) || types.Identical(expType, literalType) {
-				c.basicLiteral(t, typeName, float64(score), addlEdits)
+				c.basicLiteral(t, snip.Clone(), typeName, float64(score), addlEdits)
 			}
 		}
 	}
@@ -148,11 +150,11 @@ func (c *completer) literal(ctx context.Context, literalType types.Type, imp *im
 		switch literalType.Underlying().(type) {
 		case *types.Slice:
 			// The second argument to "make()" for slices is required, so default to "0".
-			c.makeCall(typeName, "0", float64(score), addlEdits)
+			c.makeCall(snip.Clone(), typeName, "0", float64(score), addlEdits)
 		case *types.Map, *types.Chan:
 			// Maps and channels don't require the second argument, so omit
 			// to keep things simple for now.
-			c.makeCall(typeName, "", float64(score), addlEdits)
+			c.makeCall(snip.Clone(), typeName, "", float64(score), addlEdits)
 		}
 	}
 
@@ -357,9 +359,8 @@ func abbreviateTypeName(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, edits []protocol.TextEdit) {
-	snip := &snippet.Builder{}
-	snip.WriteText(typeName + "{")
+func (c *completer) compositeLiteral(T types.Type, snip *snippet.Builder, typeName string, matchScore float64, edits []protocol.TextEdit) {
+	snip.WriteText("{")
 	// Don't put the tab stop inside the composite literal curlies "{}"
 	// for structs that have no accessible fields.
 	if strct, ok := T.(*types.Struct); !ok || fieldsAccessible(strct, c.pkg.GetTypes()) {
@@ -381,14 +382,13 @@ func (c *completer) compositeLiteral(T types.Type, typeName string, matchScore f
 
 // basicLiteral adds a literal completion item for the given basic
 // type name typeName.
-func (c *completer) basicLiteral(T types.Type, typeName string, matchScore float64, edits []protocol.TextEdit) {
+func (c *completer) basicLiteral(T types.Type, snip *snippet.Builder, typeName string, matchScore float64, edits []protocol.TextEdit) {
 	// Never give type conversions like "untyped int()".
 	if isUntyped(T) {
 		return
 	}
 
-	snip := &snippet.Builder{}
-	snip.WriteText(typeName + "(")
+	snip.WriteText("(")
 	snip.WriteFinalTabstop()
 	snip.WriteText(")")
 
@@ -406,11 +406,10 @@ func (c *completer) basicLiteral(T types.Type, typeName string, matchScore float
 }
 
 // makeCall adds a completion item for a "make()" call given a specific type.
-func (c *completer) makeCall(typeName string, secondArg string, matchScore float64, edits []protocol.TextEdit) {
+func (c *completer) makeCall(snip *snippet.Builder, typeName string, secondArg string, matchScore float64, edits []protocol.TextEdit) {
 	// Keep it simple and don't add any placeholders for optional "make()" arguments.
 
-	snip := &snippet.Builder{}
-	snip.WriteText("make(" + typeName)
+	snip.PrependText("make(")
 	if secondArg != "" {
 		snip.WriteText(", ")
 		snip.WritePlaceholder(func(b *snippet.Builder) {
@@ -438,3 +437,89 @@ func (c *completer) makeCall(typeName string, secondArg string, matchScore float
 		snippet:             snip,
 	})
 }
+
+// Create a snippet for a type name where type params become placeholders.
+func (c *completer) typeNameSnippet(literalType types.Type, qf types.Qualifier) (*snippet.Builder, string) {
+	var (
+		snip     snippet.Builder
+		typeName string
+		named, _ = literalType.(*types.Named)
+	)
+
+	if named != nil && named.Obj() != nil && typeparams.ForNamed(named).Len() > 0 && !c.fullyInstantiated(named) {
+		// We are not "fully instantiated" meaning we have type params that must be specified.
+		if pkg := qf(named.Obj().Pkg()); pkg != "" {
+			typeName = pkg + "."
+		}
+
+		// We do this to get "someType" instead of "someType[T]".
+		typeName += named.Obj().Name()
+		snip.WriteText(typeName + "[")
+
+		if c.opts.placeholders {
+			for i := 0; i < typeparams.ForNamed(named).Len(); i++ {
+				if i > 0 {
+					snip.WriteText(", ")
+				}
+				snip.WritePlaceholder(func(snip *snippet.Builder) {
+					snip.WriteText(types.TypeString(typeparams.ForNamed(named).At(i), qf))
+				})
+			}
+		} else {
+			snip.WritePlaceholder(nil)
+		}
+		snip.WriteText("]")
+		typeName += "[...]"
+	} else {
+		// We don't have unspecified type params so use default type formatting.
+		typeName = types.TypeString(literalType, qf)
+		snip.WriteText(typeName)
+	}
+
+	return &snip, typeName
+}
+
+// fullyInstantiated reports whether all of t's type params have
+// specified type args.
+func (c *completer) fullyInstantiated(t *types.Named) bool {
+	tps := typeparams.ForNamed(t)
+	tas := typeparams.NamedTypeArgs(t)
+
+	if tps.Len() != tas.Len() {
+		return false
+	}
+
+	for i := 0; i < tas.Len(); i++ {
+		switch ta := tas.At(i).(type) {
+		case *typeparams.TypeParam:
+			// A *TypeParam only counts as specified if it is currently in
+			// scope (i.e. we are in a generic definition).
+			if !c.typeParamInScope(ta) {
+				return false
+			}
+		case *types.Named:
+			if !c.fullyInstantiated(ta) {
+				return false
+			}
+		}
+	}
+	return true
+}
+
+// typeParamInScope returns whether tp's object is in scope at c.pos.
+// This tells you whether you are in a generic definition and can
+// assume tp has been specified.
+func (c *completer) typeParamInScope(tp *typeparams.TypeParam) bool {
+	obj := tp.Obj()
+	if obj == nil {
+		return false
+	}
+
+	scope := c.innermostScope()
+	if scope == nil {
+		return false
+	}
+
+	_, foundObj := scope.LookupParent(obj.Name(), c.pos)
+	return obj == foundObj
+}

internal/lsp/testdata/summary_go1.18.txt.golden

@@ -2,11 +2,11 @@
 CallHierarchyCount = 2
 CodeLensCount = 5
 CompletionsCount = 266
-CompletionSnippetCount = 110
+CompletionSnippetCount = 113
 UnimportedCompletionsCount = 5
 DeepCompletionsCount = 5
 FuzzyCompletionsCount = 8
-RankedCompletionsCount = 170
+RankedCompletionsCount = 173
 CaseSensitiveCompletionsCount = 4
 DiagnosticsCount = 37
 FoldingRangesCount = 2

internal/lsp/testdata/typeparams/type_params.go

@@ -24,6 +24,19 @@ func _() {
 	s[]{} //@rank("]", int, float64)
 }
 
+func takesGeneric[a int | string](s[a]) {
+	"s[a]{}" //@item(tpInScopeLit, "s[a]{}", "", "var")
+	takesGeneric() //@rank(")", tpInScopeLit),snippet(")", tpInScopeLit, "s[a]{\\}", "s[a]{\\}")
+}
+
+func _() {
+	s[int]{} //@item(tpInstLit, "s[int]{}", "", "var")
+	takesGeneric[int]() //@rank(")", tpInstLit),snippet(")", tpInstLit, "s[int]{\\}", "s[int]{\\}")
+
+	"s[...]{}" //@item(tpUninstLit, "s[...]{}", "", "var")
+	takesGeneric() //@rank(")", tpUninstLit),snippet(")", tpUninstLit, "s[${1:}]{\\}", "s[${1:a}]{\\}")
+}
+
 func returnTP[A int | float64](a A) A { //@item(returnTP, "returnTP", "something", "func")
 	return a
 }