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>
diff --git a/internal/lsp/snippet/snippet_builder.go b/internal/lsp/snippet/snippet_builder.go
index f7fc5b4..fa63e8d 100644
--- a/internal/lsp/snippet/snippet_builder.go
+++ b/internal/lsp/snippet/snippet_builder.go
@@ -96,6 +96,13 @@
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 32d5370..50116c9 100644
--- a/internal/lsp/source/completion/completion.go
+++ b/internal/lsp/source/completion/completion.go
@@ -221,6 +221,12 @@
// 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 @@
}
}
+ // 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 @@
methodSetCache: make(map[methodSetKey]*types.MethodSet),
mapper: pgf.Mapper,
startTime: startTime,
+ scopes: scopes,
}
var cancel context.CancelFunc
@@ -1267,9 +1278,6 @@
// 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 @@
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 @@
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 @@
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 @@
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 5025f1f..03c4401 100644
--- a/internal/lsp/source/completion/literal.go
+++ b/internal/lsp/source/completion/literal.go
@@ -15,6 +15,7 @@
"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 @@
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 @@
} 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 @@
// 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 @@
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 @@
}
// 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 @@
// 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 @@
}
// 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 @@
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 0d60e86..e33bcec 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 f048027..48b428e 100644
--- a/internal/lsp/testdata/typeparams/type_params.go
+++ b/internal/lsp/testdata/typeparams/type_params.go
@@ -24,6 +24,19 @@
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
}
