internal/lsp: search candidate type's package for completions
When our expected type is a named type from another package, we now always
search that other package for completion candidates, even if it is not currently
imported.
Consider the example:
-- foo.go --
import "context"
func doSomething(ctx context.Context) {}
-- bar.go--
doSomething(<>)
"bar.go" doesn't import "context" yet, so normally you need to first import
"context" through whatever means before you get completion items from "context".
Now we notice that the expected type's package hasn't been imported yet and give
deep completions from "context".
Another use case is with literal completions. Consider:
-- foo.go --
import "bytes"
func doSomething(buf *bytes.Buffer) {}
-- bar.go--
doSomething(<>)
Now you will get a literal completion for "&bytes.Buffer{}" in "bar.go" even
though it hasn't imported "bytes" yet.
I had to pipe the import info around a bunch of places so the import is added
automatically for deep completions and literal completions.
Change-Id: Ie86af2aa64ee235038957c1eecf042f7ec2b329b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201207
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index 07076cd..a55a52a 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -325,7 +325,7 @@
} else if isTypeName(obj) {
// If obj is a *types.TypeName that didn't otherwise match, check
// if a literal object of this type makes a good candidate.
- c.literal(obj.Type())
+ c.literal(obj.Type(), imp)
}
// Favor shallow matches by lowering weight according to depth.
@@ -350,7 +350,7 @@
}
}
- c.deepSearch(obj)
+ c.deepSearch(obj, imp)
}
// candidate represents a completion candidate.
@@ -570,7 +570,7 @@
// Is sel a qualified identifier?
if id, ok := sel.X.(*ast.Ident); ok {
if pkgname, ok := c.pkg.GetTypesInfo().Uses[id].(*types.PkgName); ok {
- c.packageMembers(pkgname)
+ c.packageMembers(pkgname.Imported(), nil)
return nil
}
}
@@ -581,17 +581,17 @@
return errors.Errorf("cannot resolve %s", sel.X)
}
- return c.methodsAndFields(tv.Type, tv.Addressable())
+ return c.methodsAndFields(tv.Type, tv.Addressable(), nil)
}
-func (c *completer) packageMembers(pkg *types.PkgName) {
- scope := pkg.Imported().Scope()
+func (c *completer) packageMembers(pkg *types.Package, imp *imports.ImportInfo) {
+ scope := pkg.Scope()
for _, name := range scope.Names() {
- c.found(scope.Lookup(name), stdScore, nil)
+ c.found(scope.Lookup(name), stdScore, imp)
}
}
-func (c *completer) methodsAndFields(typ types.Type, addressable bool) error {
+func (c *completer) methodsAndFields(typ types.Type, addressable bool, imp *imports.ImportInfo) error {
mset := c.methodSetCache[methodSetKey{typ, addressable}]
if mset == nil {
if addressable && !types.IsInterface(typ) && !isPointer(typ) {
@@ -605,12 +605,12 @@
}
for i := 0; i < mset.Len(); i++ {
- c.found(mset.At(i).Obj(), stdScore, nil)
+ c.found(mset.At(i).Obj(), stdScore, imp)
}
// Add fields of T.
for _, f := range fieldSelections(typ) {
- c.found(f, stdScore, nil)
+ c.found(f, stdScore, imp)
}
return nil
}
@@ -681,6 +681,28 @@
}
}
+ if c.expectedType.objType != nil {
+ if named, _ := deref(c.expectedType.objType).(*types.Named); named != nil {
+ // If we expected a named type, check the type's package for
+ // completion items. This is useful when the current file hasn't
+ // imported the type's package yet.
+
+ if named.Obj() != nil && named.Obj().Pkg() != nil {
+ pkg := named.Obj().Pkg()
+
+ // Make sure the package name isn't already in use by another
+ // object, and that this file doesn't import the package yet.
+ if _, ok := seen[pkg.Name()]; !ok && pkg != c.pkg.GetTypes() && !alreadyImports(c.file, pkg.Path()) {
+ seen[pkg.Name()] = struct{}{}
+ obj := types.NewPkgName(0, nil, pkg.Name(), pkg)
+ c.found(obj, stdScore, &imports.ImportInfo{
+ ImportPath: pkg.Path(),
+ })
+ }
+ }
+ }
+ }
+
if c.opts.Unimported {
// Suggest packages that have not been imported yet.
pkgs, err := CandidateImports(c.ctx, c.view, c.filename)
@@ -703,12 +725,11 @@
if c.expectedType.objType != nil {
// If we have an expected type and it is _not_ a named type, see
- // if an object literal makes a good candidate. Named types are
- // handled during the normal lexical object search. For example,
- // if our expected type is "[]int", this will add a candidate of
+ // if an object literal makes a good candidate. For example, if
+ // our expected type is "[]int", this will add a candidate of
// "[]int{}".
if _, named := deref(c.expectedType.objType).(*types.Named); !named {
- c.literal(c.expectedType.objType)
+ c.literal(c.expectedType.objType, nil)
}
}
diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go
index e03857e..ec41c48 100644
--- a/internal/lsp/source/completion_format.go
+++ b/internal/lsp/source/completion_format.go
@@ -13,6 +13,7 @@
"go/types"
"strings"
+ "golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/snippet"
"golang.org/x/tools/internal/span"
@@ -90,17 +91,11 @@
// If this candidate needs an additional import statement,
// add the additional text edits needed.
- if cand.imp != nil {
- edit, err := addNamedImport(c.view.Session().Cache().FileSet(), c.file, cand.imp.Name, cand.imp.ImportPath)
- if err != nil {
- return CompletionItem{}, err
- }
- addlEdits, err := ToProtocolEdits(c.mapper, edit)
- if err != nil {
- return CompletionItem{}, err
- }
- protocolEdits = append(protocolEdits, addlEdits...)
+ addlEdits, err := c.importEdits(cand.imp)
+ if err != nil {
+ return CompletionItem{}, err
}
+ protocolEdits = append(protocolEdits, addlEdits...)
detail = strings.TrimPrefix(detail, "untyped ")
item := CompletionItem{
@@ -151,6 +146,20 @@
return item, nil
}
+// importEdits produces the text eddits necessary to add the given import to the current file.
+func (c *completer) importEdits(imp *imports.ImportInfo) ([]protocol.TextEdit, error) {
+ if imp == nil {
+ return nil, nil
+ }
+
+ edit, err := addNamedImport(c.view.Session().Cache().FileSet(), c.file, imp.Name, imp.ImportPath)
+ if err != nil {
+ return nil, err
+ }
+
+ return ToProtocolEdits(c.mapper, edit)
+}
+
func (c *completer) formatBuiltin(cand candidate) CompletionItem {
obj := cand.obj
item := CompletionItem{
diff --git a/internal/lsp/source/completion_literal.go b/internal/lsp/source/completion_literal.go
index 0a55f67..110f9c6 100644
--- a/internal/lsp/source/completion_literal.go
+++ b/internal/lsp/source/completion_literal.go
@@ -11,6 +11,7 @@
"strings"
"unicode"
+ "golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/lsp/diff"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/snippet"
@@ -20,7 +21,7 @@
// literal generates composite literal, function literal, and make()
// completion items.
-func (c *completer) literal(literalType types.Type) {
+func (c *completer) literal(literalType types.Type, imp *imports.ImportInfo) {
if c.expectedType.objType == nil {
return
}
@@ -93,10 +94,14 @@
matchName = types.TypeString(t.Elem(), qf)
}
+ addlEdits, err := c.importEdits(imp)
+ if err != nil {
+ log.Error(c.ctx, "error adding import for literal candidate", err)
+ return
+ }
+
// If prefix matches the type name, client may want a composite literal.
if score := c.matcher.Score(matchName); score >= 0 {
- var protocolEdits []protocol.TextEdit
-
if isPointer {
if sel != nil {
// If we are in a selector we must place the "&" before the selector.
@@ -107,7 +112,7 @@
log.Error(c.ctx, "error making edit for literal pointer completion", err)
return
}
- protocolEdits = append(protocolEdits, edits...)
+ addlEdits = append(addlEdits, edits...)
} else {
// Otherwise we can stick the "&" directly before the type name.
typeName = "&" + typeName
@@ -116,13 +121,13 @@
switch t := literalType.Underlying().(type) {
case *types.Struct, *types.Array, *types.Slice, *types.Map:
- c.compositeLiteral(t, typeName, float64(score), protocolEdits)
+ c.compositeLiteral(t, typeName, float64(score), addlEdits)
case *types.Basic:
// Add a literal completion for basic types that implement our
// expected interface (e.g. named string type http.Dir
// implements http.FileSystem).
if isInterface(c.expectedType.objType) {
- c.basicLiteral(t, typeName, float64(score), protocolEdits)
+ c.basicLiteral(t, typeName, float64(score), addlEdits)
}
}
}
@@ -134,11 +139,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))
+ c.makeCall(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))
+ c.makeCall(typeName, "", float64(score), addlEdits)
}
}
@@ -341,7 +346,7 @@
}
// makeCall adds a completion item for a "make()" call given a specific type.
-func (c *completer) makeCall(typeName string, secondArg string, matchScore float64) {
+func (c *completer) makeCall(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{}
@@ -365,10 +370,11 @@
nonSnippet.WriteByte(')')
c.items = append(c.items, CompletionItem{
- Label: nonSnippet.String(),
- InsertText: nonSnippet.String(),
- Score: matchScore * literalCandidateScore,
- Kind: protocol.FunctionCompletion,
- snippet: snip,
+ Label: nonSnippet.String(),
+ InsertText: nonSnippet.String(),
+ Score: matchScore * literalCandidateScore,
+ Kind: protocol.FunctionCompletion,
+ AdditionalTextEdits: edits,
+ snippet: snip,
})
}
diff --git a/internal/lsp/source/deep_completion.go b/internal/lsp/source/deep_completion.go
index b175933..3891a93 100644
--- a/internal/lsp/source/deep_completion.go
+++ b/internal/lsp/source/deep_completion.go
@@ -8,6 +8,8 @@
"go/types"
"strings"
"time"
+
+ "golang.org/x/tools/internal/imports"
)
// Limit deep completion results because in most cases there are too many
@@ -140,7 +142,7 @@
// deepSearch searches through obj's subordinate objects for more
// completion items.
-func (c *completer) deepSearch(obj types.Object) {
+func (c *completer) deepSearch(obj types.Object, imp *imports.ImportInfo) {
if c.deepState.maxDepth == 0 {
return
}
@@ -170,7 +172,7 @@
// the deep chain.
c.deepState.push(obj, true)
// The result of a function call is not addressable.
- c.methodsAndFields(sig.Results().At(0).Type(), false)
+ c.methodsAndFields(sig.Results().At(0).Type(), false, imp)
c.deepState.pop()
}
}
@@ -180,11 +182,11 @@
switch obj := obj.(type) {
case *types.PkgName:
- c.packageMembers(obj)
+ c.packageMembers(obj.Imported(), imp)
default:
// For now it is okay to assume obj is addressable since we don't search beyond
// function calls.
- c.methodsAndFields(obj.Type(), true)
+ c.methodsAndFields(obj.Type(), true, imp)
}
// Pop the object off our search stack.
diff --git a/internal/lsp/source/imports.go b/internal/lsp/source/imports.go
index 22121d6..53fb435 100644
--- a/internal/lsp/source/imports.go
+++ b/internal/lsp/source/imports.go
@@ -37,7 +37,7 @@
//
// addNamedImport only returns edits that affect the import declarations.
func addNamedImport(fset *token.FileSet, f *ast.File, name, path string) (edits []diff.TextEdit, err error) {
- if alreadyImports(f, name, path) {
+ if alreadyImportsNamed(f, name, path) {
return nil, nil
}
@@ -198,8 +198,19 @@
return strings.Contains(importPath, ".")
}
-// alreadyImports reports whether f has an import with the specified name and path.
-func alreadyImports(f *ast.File, name, path string) bool {
+// alreadyImports reports whether f has an import with the specified path.
+func alreadyImports(f *ast.File, path string) bool {
+ for _, s := range f.Imports {
+ if importPath(s) == path {
+ return true
+ }
+ }
+ return false
+}
+
+// alreadyImportsNamed reports whether f has an import with the specified name
+// and path.
+func alreadyImportsNamed(f *ast.File, name, path string) bool {
for _, s := range f.Imports {
if importName(s) == name && importPath(s) == path {
return true
diff --git a/internal/lsp/testdata/baz/baz.go.in b/internal/lsp/testdata/baz/baz.go.in
index 534854c..4652e96 100644
--- a/internal/lsp/testdata/baz/baz.go.in
+++ b/internal/lsp/testdata/baz/baz.go.in
@@ -8,6 +8,8 @@
f "golang.org/x/tools/internal/lsp/foo"
)
+var FooStruct f.StructFoo
+
func Baz() {
defer bar.Bar() //@complete("B", Bar)
// TODO(rstambler): Test completion here.
diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden
index 1e5c354..ba20808 100644
--- a/internal/lsp/testdata/summary.txt.golden
+++ b/internal/lsp/testdata/summary.txt.golden
@@ -1,6 +1,6 @@
-- summary --
CompletionsCount = 213
-CompletionSnippetCount = 39
+CompletionSnippetCount = 40
UnimportedCompletionsCount = 1
DeepCompletionsCount = 5
FuzzyCompletionsCount = 7
diff --git a/internal/lsp/testdata/unimported/unimported_cand_type.go b/internal/lsp/testdata/unimported/unimported_cand_type.go
new file mode 100644
index 0000000..765ab19
--- /dev/null
+++ b/internal/lsp/testdata/unimported/unimported_cand_type.go
@@ -0,0 +1,11 @@
+package unimported
+
+import "golang.org/x/tools/internal/lsp/baz"
+
+func _() {
+ foo.StructFoo{} //@item(litFooStructFoo, "foo.StructFoo{}", "struct{...}", "struct")
+
+ // We get the literal completion for "foo.StructFoo{}" even though we haven't
+ // imported "foo" yet.
+ baz.FooStruct = f //@snippet(" //", litFooStructFoo, "foo.StructFoo{$0\\}", "foo.StructFoo{$0\\}")
+}
