gopls/internal/lsp/source/completion: infer parameter types

This change causes completion to omit type parameters
in the snippet template when the instantiations can be
inferred from the arguments to the call.

This is only implemented when types are available,
not for unimported completions. (In principle it could be
implemented using a best-effort syntactic traversal, similar
to inlining's "freeishNames", but it seems fiddly.)

Fixes golang/go#51783

Change-Id: I4c793628e7f2cbbb28a7f5ca7b5e305ab683491e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/554855
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

gopls/internal/lsp/source/completion/completion.go

@@ -377,6 +377,7 @@ func (c *completer) getSurrounding() *Selection {
 type candidate struct {
 	// obj is the types.Object to complete to.
 	// TODO(adonovan): eliminate dependence on go/types throughout this struct.
+	// See comment in (*completer).selector for explanation.
 	obj types.Object
 
 	// score is used to rank candidates.
@@ -1347,6 +1348,11 @@ func (c *completer) selector(ctx context.Context, sel *ast.SelectorExpr) error {
 					return params
 				}
 
+				// Ideally we would eliminate the suffix of type
+				// parameters that are redundant with inference
+				// from the argument types (#51783), but it's
+				// quite fiddly to do using syntax alone.
+				// (See inferableTypeParams in format.go.)
 				tparams := paramList(fn.Type.TypeParams)
 				params := paramList(fn.Type.Params)
 				var sn snippet.Builder

gopls/internal/lsp/source/completion/format.go

@@ -135,7 +135,27 @@ Suffixes:
 				if err != nil {
 					return CompletionItem{}, err
 				}
-				c.functionCallSnippet("", s.TypeParams(), s.Params(), &snip)
+
+				tparams := s.TypeParams()
+				if len(tparams) > 0 {
+					// Eliminate the suffix of type parameters that are
+					// likely redundant because they can probably be
+					// inferred from the argument types (#51783).
+					//
+					// We don't bother doing the reverse inference from
+					// result types as result-only type parameters are
+					// quite unusual.
+					free := inferableTypeParams(sig)
+					for i := sig.TypeParams().Len() - 1; i >= 0; i-- {
+						tparam := sig.TypeParams().At(i)
+						if !free[tparam] {
+							break
+						}
+						tparams = tparams[:i] // eliminate
+					}
+				}
+
+				c.functionCallSnippet("", tparams, s.Params(), &snip)
 				if sig.Results().Len() == 1 {
 					funcType = sig.Results().At(0).Type()
 				}
@@ -310,6 +330,7 @@ func (c *completer) formatBuiltin(ctx context.Context, cand candidate) (Completi
 		}
 		item.Detail = "func" + sig.Format()
 		item.snippet = &snippet.Builder{}
+		// The signature inferred for a built-in is instantiated, so TypeParams=∅.
 		c.functionCallSnippet(obj.Name(), sig.TypeParams(), sig.Params(), item.snippet)
 	case *types.TypeName:
 		if types.IsInterface(obj.Type()) {
@@ -341,3 +362,78 @@ func (c *completer) wantTypeParams() bool {
 	}
 	return false
 }
+
+// inferableTypeParams returns the set of type parameters
+// of sig that are constrained by (inferred from) the argument types.
+func inferableTypeParams(sig *types.Signature) map[*types.TypeParam]bool {
+	free := make(map[*types.TypeParam]bool)
+
+	// visit adds to free all the free type parameters of t.
+	var visit func(t types.Type)
+	visit = func(t types.Type) {
+		switch t := t.(type) {
+		case *types.Array:
+			visit(t.Elem())
+		case *types.Chan:
+			visit(t.Elem())
+		case *types.Map:
+			visit(t.Key())
+			visit(t.Elem())
+		case *types.Pointer:
+			visit(t.Elem())
+		case *types.Slice:
+			visit(t.Elem())
+		case *types.Interface:
+			for i := 0; i < t.NumExplicitMethods(); i++ {
+				visit(t.ExplicitMethod(i).Type())
+			}
+			for i := 0; i < t.NumEmbeddeds(); i++ {
+				visit(t.EmbeddedType(i))
+			}
+		case *types.Union:
+			for i := 0; i < t.Len(); i++ {
+				visit(t.Term(i).Type())
+			}
+		case *types.Signature:
+			if tp := t.TypeParams(); tp != nil {
+				// Generic signatures only appear as the type of generic
+				// function declarations, so this isn't really reachable.
+				for i := 0; i < tp.Len(); i++ {
+					visit(tp.At(i).Constraint())
+				}
+			}
+			visit(t.Params())
+			visit(t.Results())
+		case *types.Tuple:
+			for i := 0; i < t.Len(); i++ {
+				visit(t.At(i).Type())
+			}
+		case *types.Struct:
+			for i := 0; i < t.NumFields(); i++ {
+				visit(t.Field(i).Type())
+			}
+		case *types.TypeParam:
+			free[t] = true
+		case *types.Basic, *types.Named:
+			// nop
+		default:
+			panic(t)
+		}
+	}
+
+	visit(sig.Params())
+
+	// Perform induction through constraints.
+restart:
+	for i := 0; i < sig.TypeParams().Len(); i++ {
+		tp := sig.TypeParams().At(i)
+		if free[tp] {
+			n := len(free)
+			visit(tp.Constraint())
+			if len(free) > n {
+				goto restart // iterate until fixed point
+			}
+		}
+	}
+	return free
+}

gopls/internal/lsp/source/completion/snippet.go

@@ -50,6 +50,11 @@ func (c *completer) structFieldSnippet(cand candidate, detail string, snip *snip
 }
 
 // functionCallSnippet calculates the snippet for function calls.
+//
+// Callers should omit the suffix of type parameters that are
+// constrained by the argument types, to avoid offering completions
+// that contain instantiations that are redundant because of type
+// inference, such as f[int](1) for func f[T any](x T).
 func (c *completer) functionCallSnippet(name string, tparams, params []string, snip *snippet.Builder) {
 	if !c.opts.completeFunctionCalls {
 		snip.WriteText(name)

gopls/internal/test/marker/testdata/completion/func_snippets.txt

@@ -28,5 +28,5 @@ func Identity[P ~int](p P) P { //@item(Identity, "Identity", "", "")
 
 func _() {
 	_ = NewSyncM //@snippet(" //", NewSyncMap, "NewSyncMap[${1:K comparable}, ${2:V any}]()")
-	_ = Identi //@snippet(" //", Identity, "Identity[${1:P ~int}](${2:p P})")
+	_ = Identi //@snippet(" //", Identity, "Identity(${1:p P})")
 }

gopls/internal/test/marker/testdata/completion/issue51783.txt

@@ -0,0 +1,47 @@
+Regression test for "completion gives unneeded generic type
+instantiation snippet", #51783.
+
+Type parameters that can be inferred from the arguments
+are not part of the offered completion snippet.
+
+-- flags --
+-ignore_extra_diags
+
+-- a.go --
+package a
+
+// identity has a single simple type parameter.
+// The completion omits the instantiation.
+func identity[T any](x T) T
+
+// clone has a second type parameter that is nonetheless constrained by the parameter.
+// The completion omits the instantiation.
+func clone[S ~[]E, E any](s S) S
+
+// unconstrained has a type parameter constrained only by the result.
+// The completion suggests instantiation.
+func unconstrained[X, Y any](x X) Y
+
+// partial has three type parameters,
+// only the last two of which may be omitted as they
+// are constrained by the arguments.
+func partial[R any, S ~[]E, E any](s S) R
+
+//@item(identity, "identity", "details", "kind")
+//@item(clone, "clone", "details", "kind")
+//@item(unconstrained, "unconstrained", "details", "kind")
+//@item(partial, "partial", "details", "kind")
+
+func _() {
+	_ = identity //@snippet("identity", identity, "identity(${1:})")
+
+	_ = clone //@snippet("clone", clone, "clone(${1:})")
+
+	_ = unconstrained //@snippet("unconstrained", unconstrained, "unconstrained[${1:}](${2:})")
+
+	_ = partial //@snippet("partial", partial, "partial[${1:}](${2:})")
+
+	// Result-type inference permits us to omit Y in this (rare) case,
+	// but completion doesn't support that.
+	var _ int = unconstrained //@snippet("unconstrained", unconstrained, "unconstrained[${1:}](${2:})")
+}

gopls/internal/test/marker/testdata/completion/type_params.txt

@@ -52,8 +52,7 @@ func returnTP[A int | float64](a A) A { //@item(returnTP, "returnTP", "something
 }
 
 func _() {
-	// disabled - see issue #54822
-	var _ int = returnTP // snippet(" //", returnTP, "returnTP[${1:}](${2:})")
+	var _ int = returnTP //@snippet(" //", returnTP, "returnTP(${1:})")
 
 	var aa int //@item(tpInt, "aa", "int", "var")
 	var ab float64 //@item(tpFloat, "ab", "float64", "var")