From 86caa796c7ab0e32b908c5ebd5748faedd06c1da Mon Sep 17 00:00:00 2001 From: Muir Manders Date: Tue, 15 Oct 2019 14:42:30 -0700 Subject: [PATCH] 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 Reviewed-by: Rebecca Stambler --- internal/lsp/source/completion.go | 49 +++++++++++++------ internal/lsp/source/completion_format.go | 29 +++++++---- internal/lsp/source/completion_literal.go | 34 +++++++------ internal/lsp/source/deep_completion.go | 10 ++-- internal/lsp/source/imports.go | 17 +++++-- internal/lsp/testdata/baz/baz.go.in | 2 + internal/lsp/testdata/summary.txt.golden | 2 +- .../unimported/unimported_cand_type.go | 11 +++++ 8 files changed, 108 insertions(+), 46 deletions(-) create mode 100644 internal/lsp/testdata/unimported/unimported_cand_type.go diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 07076cd555..a55a52a1e9 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -325,7 +325,7 @@ func (c *completer) found(obj types.Object, score float64, imp *imports.ImportIn } 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 @@ func (c *completer) found(obj types.Object, score float64, imp *imports.ImportIn } } - c.deepSearch(obj) + c.deepSearch(obj, imp) } // candidate represents a completion candidate. @@ -570,7 +570,7 @@ func (c *completer) selector(sel *ast.SelectorExpr) error { // 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 @@ func (c *completer) selector(sel *ast.SelectorExpr) error { 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 @@ func (c *completer) methodsAndFields(typ types.Type, addressable bool) error { } 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 @@ func (c *completer) lexical() error { } } + 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 @@ func (c *completer) lexical() error { 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 e03857e378..ec41c483f0 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -13,6 +13,7 @@ import ( "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 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { // 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 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { 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 0a55f67f68..110f9c695c 100644 --- a/internal/lsp/source/completion_literal.go +++ b/internal/lsp/source/completion_literal.go @@ -11,6 +11,7 @@ import ( "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 @@ import ( // 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 @@ func (c *completer) literal(literalType types.Type) { 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 @@ func (c *completer) literal(literalType types.Type) { 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 @@ func (c *completer) literal(literalType types.Type) { 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 @@ func (c *completer) literal(literalType types.Type) { 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 @@ 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) { +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 @@ func (c *completer) makeCall(typeName string, secondArg string, matchScore float 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 b175933879..3891a93d17 100644 --- a/internal/lsp/source/deep_completion.go +++ b/internal/lsp/source/deep_completion.go @@ -8,6 +8,8 @@ import ( "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 @@ func (c *completer) shouldPrune() bool { // 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 @@ func (c *completer) deepSearch(obj types.Object) { // 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 @@ func (c *completer) deepSearch(obj types.Object) { 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 22121d6956..53fb4358e1 100644 --- a/internal/lsp/source/imports.go +++ b/internal/lsp/source/imports.go @@ -37,7 +37,7 @@ import ( // // 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 @@ func isThirdParty(importPath string) bool { 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 534854c3da..4652e966ed 100644 --- a/internal/lsp/testdata/baz/baz.go.in +++ b/internal/lsp/testdata/baz/baz.go.in @@ -8,6 +8,8 @@ import ( 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 1e5c354bfe..ba20808328 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 0000000000..765ab19075 --- /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\\}") +}