From 5bb9a5ecb1a02d977bcc550fe1d7664b2b72d649 Mon Sep 17 00:00:00 2001 From: Muir Manders Date: Thu, 14 Apr 2022 21:55:03 -0700 Subject: [PATCH] lsp/completion: fix literal completions with type params In cases like: type foo[T any] struct{} func bar[T any](foo[T]) {} bar[int]() bar() At we will now offer "foo[int]{}". At we will now offer a snippet "foo[]{}" 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 gopls-CI: kokoro TryBot-Result: Gopher Robot Reviewed-by: Robert Findley Reviewed-by: Dmitri Shuralyov --- internal/lsp/snippet/snippet_builder.go | 7 ++ internal/lsp/source/completion/completion.go | 44 +++++-- internal/lsp/source/completion/literal.go | 115 +++++++++++++++--- .../lsp/testdata/summary_go1.18.txt.golden | 4 +- .../lsp/testdata/typeparams/type_params.go | 13 ++ 5 files changed, 154 insertions(+), 29 deletions(-) diff --git a/internal/lsp/snippet/snippet_builder.go b/internal/lsp/snippet/snippet_builder.go index f7fc5b4454..fa63e8d832 100644 --- a/internal/lsp/snippet/snippet_builder.go +++ b/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. diff --git a/internal/lsp/source/completion/completion.go b/internal/lsp/source/completion/completion.go index 32d5370563..50116c940f 100644 --- a/internal/lsp/source/completion/completion.go +++ b/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 +} diff --git a/internal/lsp/source/completion/literal.go b/internal/lsp/source/completion/literal.go index 5025f1f749..03c4401211 100644 --- a/internal/lsp/source/completion/literal.go +++ b/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 +} diff --git a/internal/lsp/testdata/summary_go1.18.txt.golden b/internal/lsp/testdata/summary_go1.18.txt.golden index 0d60e8622d..e33bcec9e7 100644 --- a/internal/lsp/testdata/summary_go1.18.txt.golden +++ b/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 diff --git a/internal/lsp/testdata/typeparams/type_params.go b/internal/lsp/testdata/typeparams/type_params.go index f048027956..48b428ee94 100644 --- a/internal/lsp/testdata/typeparams/type_params.go +++ b/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 }