From 88c59381210f001413ddaa0358875e64f29d850d Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Fri, 1 Nov 2019 13:50:21 -0400 Subject: [PATCH] internal/lsp/source: propose exports for unimported packages When a user completes rand.<>, propose rand.Seed (from math/rand) and rand.Prime (from crypto/rand), etc. Because we don't necessarily have type checking information for unimported packages, I had to add shortcut cases to a number of functions around the completion code. Better suggestions welcome. Change-Id: I7822dc75c86b24156963e7bdd959443f4f2748b1 Reviewed-on: https://go-review.googlesource.com/c/tools/+/204819 Run-TryBot: Heschi Kreinick TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler Reviewed-by: Muir Manders --- internal/lsp/source/completion.go | 22 ++++++++-- internal/lsp/source/completion_format.go | 25 ++++++++--- internal/lsp/source/deep_completion.go | 4 ++ internal/lsp/source/format.go | 42 ++++++++++++++----- internal/lsp/testdata/summary.txt.golden | 2 +- .../{unimported.go => unimported.go.in} | 3 ++ internal/lsp/tests/completion.go | 6 ++- 7 files changed, 84 insertions(+), 20 deletions(-) rename internal/lsp/testdata/unimported/{unimported.go => unimported.go.in} (83%) diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index a55a52a1e9a..905043d814a 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -577,11 +577,24 @@ func (c *completer) selector(sel *ast.SelectorExpr) error { // Invariant: sel is a true selector. tv, ok := c.pkg.GetTypesInfo().Types[sel.X] - if !ok { - return errors.Errorf("cannot resolve %s", sel.X) + if ok { + return c.methodsAndFields(tv.Type, tv.Addressable(), nil) } - return c.methodsAndFields(tv.Type, tv.Addressable(), nil) + // Try unimported packages. + if id, ok := sel.X.(*ast.Ident); ok { + pkgExports, err := PackageExports(c.ctx, c.view, id.Name, c.filename) + if err != nil { + return err + } + for _, pkgExport := range pkgExports { + pkg := types.NewPackage(pkgExport.Fix.StmtInfo.ImportPath, pkgExport.Fix.IdentName) + for _, export := range pkgExport.Exports { + c.found(types.NewVar(0, pkg, export, nil), 0.07, &pkgExport.Fix.StmtInfo) + } + } + } + return nil } func (c *completer) packageMembers(pkg *types.Package, imp *imports.ImportInfo) { @@ -1347,6 +1360,9 @@ func (c *completer) matchingCandidate(cand *candidate) bool { } objType := cand.obj.Type() + if objType == nil { + return true + } // Default to invoking *types.Func candidates. This is so function // completions in an empty statement (or other cases with no expected type) diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index ec41c483f06..000e664e2cd 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -39,6 +39,9 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { snip *snippet.Builder protocolEdits []protocol.TextEdit ) + if obj.Type() == nil { + detail = "" + } // expandFuncCall mutates the completion label, detail, and snippet // to that of an invocation of sig. @@ -64,6 +67,9 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { } else { kind = protocol.VariableCompletion } + if obj.Type() == nil { + break + } if sig, ok := obj.Type().Underlying().(*types.Signature); ok && cand.expandFuncCall { expandFuncCall(sig) @@ -91,11 +97,20 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { // If this candidate needs an additional import statement, // add the additional text edits needed. - addlEdits, err := c.importEdits(cand.imp) - if err != nil { - return CompletionItem{}, err + if cand.imp != nil { + addlEdits, err := c.importEdits(cand.imp) + if err != nil { + return CompletionItem{}, err + } + + protocolEdits = append(protocolEdits, addlEdits...) + if kind != protocol.ModuleCompletion { + if detail != "" { + detail += " " + } + detail += fmt.Sprintf("(from %q)", cand.imp.ImportPath) + } } - protocolEdits = append(protocolEdits, addlEdits...) detail = strings.TrimPrefix(detail, "untyped ") item := CompletionItem{ @@ -146,7 +161,7 @@ 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. +// importEdits produces the text edits 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 diff --git a/internal/lsp/source/deep_completion.go b/internal/lsp/source/deep_completion.go index 3891a93d170..eddacac7528 100644 --- a/internal/lsp/source/deep_completion.go +++ b/internal/lsp/source/deep_completion.go @@ -158,6 +158,10 @@ func (c *completer) deepSearch(obj types.Object, imp *imports.ImportInfo) { return } + if obj.Type() == nil { + return + } + // Don't search embedded fields because they were already included in their // parent's fields. if v, ok := obj.(*types.Var); ok && v.Embedded() { diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index 7a2608aefc9..4a8974f3bd7 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -240,11 +240,8 @@ func AllImportsFixes(ctx context.Context, view View, f File) (edits []protocol.T return edits, editsPerFix, nil } -// AllImportsFixes formats f for each possible fix to the imports. -// In addition to returning the result of applying all edits, -// it returns a list of fixes that could be applied to the file, with the -// corresponding TextEdits that would be needed to apply that fix. -func CandidateImports(ctx context.Context, view View, filename string) (pkgs []imports.ImportFix, err error) { +// CandidateImports returns every import that could be added to filename. +func CandidateImports(ctx context.Context, view View, filename string) ([]imports.ImportFix, error) { ctx, done := trace.StartSpan(ctx, "source.CandidateImports") defer done() @@ -257,16 +254,41 @@ func CandidateImports(ctx context.Context, view View, filename string) (pkgs []i TabIndent: true, TabWidth: 8, } + + var imps []imports.ImportFix importFn := func(opts *imports.Options) error { - pkgs, err = imports.GetAllCandidates(filename, opts) + var err error + imps, err = imports.GetAllCandidates(filename, opts) return err } - err = view.RunProcessEnvFunc(ctx, importFn, options) - if err != nil { - return nil, err + err := view.RunProcessEnvFunc(ctx, importFn, options) + return imps, err +} + +// PackageExports returns all the packages named pkg that could be imported by +// filename, and their exports. +func PackageExports(ctx context.Context, view View, pkg, filename string) ([]imports.PackageExport, error) { + ctx, done := trace.StartSpan(ctx, "source.PackageExports") + defer done() + + options := &imports.Options{ + // Defaults. + AllErrors: true, + Comments: true, + Fragment: true, + FormatOnly: false, + TabIndent: true, + TabWidth: 8, } - return pkgs, nil + var pkgs []imports.PackageExport + importFn := func(opts *imports.Options) error { + var err error + pkgs, err = imports.GetPackageExports(pkg, filename, opts) + return err + } + err := view.RunProcessEnvFunc(ctx, importFn, options) + return pkgs, err } // hasParseErrors returns true if the given file has parse errors. diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden index ba208083284..1b64a5da4d5 100644 --- a/internal/lsp/testdata/summary.txt.golden +++ b/internal/lsp/testdata/summary.txt.golden @@ -1,7 +1,7 @@ -- summary -- CompletionsCount = 213 CompletionSnippetCount = 40 -UnimportedCompletionsCount = 1 +UnimportedCompletionsCount = 2 DeepCompletionsCount = 5 FuzzyCompletionsCount = 7 RankedCompletionsCount = 8 diff --git a/internal/lsp/testdata/unimported/unimported.go b/internal/lsp/testdata/unimported/unimported.go.in similarity index 83% rename from internal/lsp/testdata/unimported/unimported.go rename to internal/lsp/testdata/unimported/unimported.go.in index ed0670c808c..c721b314ea5 100644 --- a/internal/lsp/testdata/unimported/unimported.go +++ b/internal/lsp/testdata/unimported/unimported.go.in @@ -2,6 +2,7 @@ package unimported func _() { //@unimported("", bytes, context, cryptoslashrand, time, unsafe, externalpackage) + bytes. //@unimported(" /", bytesbuffer) } // Create markers for unimported std lib packages. Only for use by this test. @@ -11,3 +12,5 @@ func _() { /* time */ //@item(time, "time", "\"time\"", "package") /* unsafe */ //@item(unsafe, "unsafe", "\"unsafe\"", "package") /* pkg */ //@item(externalpackage, "pkg", "\"example.com/extramodule/pkg\"", "package" ) + +/* bytes.Buffer */ //@item(bytesbuffer, "Buffer", "(from \"bytes\")", "var" ) \ No newline at end of file diff --git a/internal/lsp/tests/completion.go b/internal/lsp/tests/completion.go index 867ab09a810..8936e506b07 100644 --- a/internal/lsp/tests/completion.go +++ b/internal/lsp/tests/completion.go @@ -19,7 +19,7 @@ func ToProtocolCompletionItems(items []source.CompletionItem) []protocol.Complet } func ToProtocolCompletionItem(item source.CompletionItem) protocol.CompletionItem { - return protocol.CompletionItem{ + pItem := protocol.CompletionItem{ Label: item.Label, Kind: item.Kind, Detail: item.Detail, @@ -29,6 +29,10 @@ func ToProtocolCompletionItem(item source.CompletionItem) protocol.CompletionIte NewText: item.Snippet(), }, } + if pItem.InsertText == "" { + pItem.InsertText = pItem.Label + } + return pItem } func FilterBuiltins(items []protocol.CompletionItem) []protocol.CompletionItem {