From cbdab0337f29854813c47e95cf14a3d6b6e6cf2d Mon Sep 17 00:00:00 2001 From: Muir Manders Date: Fri, 18 Mar 2022 21:07:51 -0700 Subject: [PATCH] lsp/completion: fix bogus type param candidate In this example: func foo[A []int | []float64]() {} foo[<>] When completing at <> you were getting the creative candidate "[]int | []float64". This came from some code that makes the expected type available as a candidate if it wouldn't otherwise be found by a lexical search. I tweaked the code in this case to instead look at each structural type in the type constraint rather than the constraint as a whole. In the above example you now get the candidates "[]int" and "[]float64". Change-Id: Ib8e422df3009cb4253ec66005b4a53851564c4e1 Reviewed-on: https://go-review.googlesource.com/c/tools/+/394015 Run-TryBot: Muir Manders gopls-CI: kokoro TryBot-Result: Gopher Robot Reviewed-by: Robert Findley Trust: Hyang-Ah Hana Kim --- internal/lsp/source/completion/completion.go | 110 ++++++++++++------ .../lsp/testdata/summary_go1.18.txt.golden | 2 +- .../lsp/testdata/typeparams/type_params.go | 12 ++ 3 files changed, 86 insertions(+), 38 deletions(-) diff --git a/internal/lsp/source/completion/completion.go b/internal/lsp/source/completion/completion.go index 8d4df6bab9..4334d6d433 100644 --- a/internal/lsp/source/completion/completion.go +++ b/internal/lsp/source/completion/completion.go @@ -1388,33 +1388,18 @@ func (c *completer) lexical(ctx context.Context) error { } } - if t := c.inference.objType; t != nil { - 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 - // considered via a lexical search, so we need to directly inject - // them. - if _, named := t.(*types.Named); !named { - // If our expected type is "[]int", this will add a literal - // candidate of "[]int{}". - c.literal(ctx, t, nil) - - if _, isBasic := t.(*types.Basic); !isBasic { - // If we expect a non-basic type name (e.g. "[]int"), hack up - // a named type whose name is literally "[]int". This allows - // us to reuse our object based completion machinery. - fakeNamedType := candidate{ - obj: types.NewTypeName(token.NoPos, nil, types.TypeString(t, c.qf), t), - score: stdScore, - } - // Make sure the type name matches before considering - // candidate. This cuts down on useless candidates. - if c.matchingTypeName(&fakeNamedType) { - c.deepState.enqueue(fakeNamedType) - } + if c.inference.typeName.isTypeParam { + // If we are completing a type param, offer each structural type. + // This ensures we suggest "[]int" and "[]float64" for a constraint + // with type union "[]int | []float64". + if t, _ := c.inference.objType.(*types.Interface); t != nil { + terms, _ := typeparams.InterfaceTermSet(t) + for _, term := range terms { + c.injectType(ctx, term.Type()) } } + } else { + c.injectType(ctx, c.inference.objType) } // Add keyword completion items appropriate in the current context. @@ -1423,6 +1408,43 @@ 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). +func (c *completer) injectType(ctx context.Context, t types.Type) { + if t == nil { + return + } + + 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 + // considered via a lexical search, so we need to directly inject + // them. + if _, named := t.(*types.Named); !named { + // If our expected type is "[]int", this will add a literal + // candidate of "[]int{}". + c.literal(ctx, t, nil) + + if _, isBasic := t.(*types.Basic); !isBasic { + // If we expect a non-basic type name (e.g. "[]int"), hack up + // a named type whose name is literally "[]int". This allows + // us to reuse our object based completion machinery. + fakeNamedType := candidate{ + obj: types.NewTypeName(token.NoPos, nil, types.TypeString(t, c.qf), t), + score: stdScore, + } + // Make sure the type name matches before considering + // candidate. This cuts down on useless candidates. + if c.matchingTypeName(&fakeNamedType) { + c.deepState.enqueue(fakeNamedType) + } + } + } +} + func (c *completer) unimportedPackages(ctx context.Context, seen map[string]struct{}) error { var prefix string if c.surrounding != nil { @@ -1906,6 +1928,9 @@ type typeNameInference struct { // compLitType is true if we are completing a composite literal type // name, e.g "foo<>{}". compLitType bool + + // isTypeParam is true if we are completing a type instantiation parameter + isTypeParam bool } // expectedCandidate returns information about the expected candidate @@ -2094,11 +2119,12 @@ Nodes: inf.objType = t.Key() case *types.Slice, *types.Array: inf.objType = types.Typ[types.UntypedInt] - case *types.Signature: - if tp := typeparams.ForSignature(t); tp.Len() > 0 { - inf.objType = tp.At(0).Constraint() - inf.typeName.wantTypeName = true - } + } + + if ct := expectedConstraint(tv.Type, 0); ct != nil { + inf.objType = ct + inf.typeName.wantTypeName = true + inf.typeName.isTypeParam = true } } } @@ -2106,13 +2132,10 @@ Nodes: case *typeparams.IndexListExpr: if node.Lbrack < c.pos && c.pos <= node.Rbrack { if tv, ok := c.pkg.GetTypesInfo().Types[node.X]; ok { - switch t := tv.Type.Underlying().(type) { - case *types.Signature: - paramIdx := exprAtPos(c.pos, node.Indices) - if tp := typeparams.ForSignature(t); paramIdx < tp.Len() { - inf.objType = tp.At(paramIdx).Constraint() - inf.typeName.wantTypeName = true - } + if ct := expectedConstraint(tv.Type, exprAtPos(c.pos, node.Indices)); ct != nil { + inf.objType = ct + inf.typeName.wantTypeName = true + inf.typeName.isTypeParam = true } } } @@ -2157,6 +2180,19 @@ Nodes: return inf } +func expectedConstraint(t types.Type, idx int) types.Type { + var tp *typeparams.TypeParamList + if named, _ := t.(*types.Named); named != nil { + tp = typeparams.ForNamed(named) + } else if sig, _ := t.Underlying().(*types.Signature); sig != nil { + tp = typeparams.ForSignature(sig) + } + if tp == nil || idx >= tp.Len() { + return nil + } + return tp.At(idx).Constraint() +} + // objChain decomposes e into a chain of objects if possible. For // example, "foo.bar().baz" will yield []types.Object{foo, bar, baz}. // If any part can't be turned into an object, return nil. diff --git a/internal/lsp/testdata/summary_go1.18.txt.golden b/internal/lsp/testdata/summary_go1.18.txt.golden index 71a53d9af5..3224973f25 100644 --- a/internal/lsp/testdata/summary_go1.18.txt.golden +++ b/internal/lsp/testdata/summary_go1.18.txt.golden @@ -6,7 +6,7 @@ CompletionSnippetCount = 109 UnimportedCompletionsCount = 5 DeepCompletionsCount = 5 FuzzyCompletionsCount = 8 -RankedCompletionsCount = 166 +RankedCompletionsCount = 169 CaseSensitiveCompletionsCount = 4 DiagnosticsCount = 37 FoldingRangesCount = 2 diff --git a/internal/lsp/testdata/typeparams/type_params.go b/internal/lsp/testdata/typeparams/type_params.go index 4636dfa8e7..e9acb82ed0 100644 --- a/internal/lsp/testdata/typeparams/type_params.go +++ b/internal/lsp/testdata/typeparams/type_params.go @@ -11,3 +11,15 @@ func _() { two[]() //@rank("]", int, float64) two[int, f]() //@rank("]", float64, float32) } + +func slices[a []int | []float64]() {} //@item(tpInts, "[]int", "[]int", "type"),item(tpFloats, "[]float64", "[]float64", "type") + +func _() { + slices[]() //@rank("]", tpInts),rank("]", tpFloats) +} + +type s[a int | string] struct{} + +func _() { + s[]{} //@rank("]", int, float64) +}