diff --git a/gopls/internal/lsp/source/completion/completion.go b/gopls/internal/lsp/source/completion/completion.go index 805c6d6615b..a20e4633164 100644 --- a/gopls/internal/lsp/source/completion/completion.go +++ b/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 diff --git a/gopls/internal/lsp/source/completion/format.go b/gopls/internal/lsp/source/completion/format.go index 0d47161c4d0..4d3fa23f576 100644 --- a/gopls/internal/lsp/source/completion/format.go +++ b/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 +} diff --git a/gopls/internal/lsp/source/completion/snippet.go b/gopls/internal/lsp/source/completion/snippet.go index c37736254aa..a6786cfa874 100644 --- a/gopls/internal/lsp/source/completion/snippet.go +++ b/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) diff --git a/gopls/internal/test/marker/testdata/completion/func_snippets.txt b/gopls/internal/test/marker/testdata/completion/func_snippets.txt index efbc393f30f..01316342b7f 100644 --- a/gopls/internal/test/marker/testdata/completion/func_snippets.txt +++ b/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})") } diff --git a/gopls/internal/test/marker/testdata/completion/issue51783.txt b/gopls/internal/test/marker/testdata/completion/issue51783.txt new file mode 100644 index 00000000000..074259ca713 --- /dev/null +++ b/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:})") +} diff --git a/gopls/internal/test/marker/testdata/completion/type_params.txt b/gopls/internal/test/marker/testdata/completion/type_params.txt index 185d77f9911..8e2f5d7e401 100644 --- a/gopls/internal/test/marker/testdata/completion/type_params.txt +++ b/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")