Skip to content

Commit

Permalink
internal/lsp: use available type info for unimported completions
Browse files Browse the repository at this point in the history
Packages that aren't imported in the current file will often have been
used elsewhere, which means that gopls will have their type information
available. Expose loaded packages in the Snapshot, and try to use that
information when possible for unimported packages.

Change-Id: Icb672618a9f9ec31b9796f0c5da56ed3d2b38aa7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204824
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
  • Loading branch information
heschi committed Nov 4, 2019
1 parent 88c5938 commit f505e54
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 5 deletions.
24 changes: 24 additions & 0 deletions internal/lsp/cache/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,30 @@ func (s *snapshot) getPackages(uri span.URI, m source.ParseMode) (cphs []source.
return cphs
}

func (s *snapshot) KnownImportPaths() map[string]source.Package {
s.mu.Lock()
defer s.mu.Unlock()

results := map[string]source.Package{}
for _, cph := range s.packages {
cachedPkg, err := cph.cached()
if err != nil {
continue
}
for importPath, newPkg := range cachedPkg.imports {
if oldPkg, ok := results[string(importPath)]; ok {
// Using the same trick as NarrowestPackageHandle, prefer non-variants.
if len(newPkg.files) < len(oldPkg.(*pkg).files) {
results[string(importPath)] = newPkg
}
} else {
results[string(importPath)] = newPkg
}
}
}
return results
}

func (s *snapshot) getPackage(id packageID, m source.ParseMode) *checkPackageHandle {
s.mu.Lock()
defer s.mu.Unlock()
Expand Down
8 changes: 8 additions & 0 deletions internal/lsp/source/completion.go
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,15 @@ func (c *completer) selector(sel *ast.SelectorExpr) error {
if err != nil {
return err
}
known := c.snapshot.KnownImportPaths()
for _, pkgExport := range pkgExports {
// If we've seen this import path, use the fully-typed version.
if knownPkg, ok := known[pkgExport.Fix.StmtInfo.ImportPath]; ok {
c.packageMembers(knownPkg.GetTypes(), &pkgExport.Fix.StmtInfo)
continue
}

// Otherwise, continue with untyped proposals.
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)
Expand Down
4 changes: 4 additions & 0 deletions internal/lsp/source/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,10 @@ type Snapshot interface {
// CheckPackageHandles returns the CheckPackageHandles for the packages
// that this file belongs to.
CheckPackageHandles(ctx context.Context, f File) ([]CheckPackageHandle, error)

// KnownImportPaths returns all the packages loaded in this snapshot,
// indexed by their import path.
KnownImportPaths() map[string]Package
}

// File represents a source file of any type.
Expand Down
2 changes: 1 addition & 1 deletion internal/lsp/testdata/summary.txt.golden
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
-- summary --
CompletionsCount = 213
CompletionSnippetCount = 40
UnimportedCompletionsCount = 2
UnimportedCompletionsCount = 3
DeepCompletionsCount = 5
FuzzyCompletionsCount = 7
RankedCompletionsCount = 8
Expand Down
10 changes: 7 additions & 3 deletions internal/lsp/testdata/unimported/unimported.go.in
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package unimported

func _() {
//@unimported("", bytes, context, cryptoslashrand, time, unsafe, externalpackage)
bytes. //@unimported(" /", bytesbuffer)
// container/ring is extremely unlikely to be imported by anything, so shouldn't have type information.
ring.Ring //@unimported("Ring", ringring)
signature.Foo //@unimported("Foo", signaturefoo)
}

// Create markers for unimported std lib packages. Only for use by this test.
Expand All @@ -11,6 +13,8 @@ func _() {
/* rand */ //@item(cryptoslashrand, "rand", "\"crypto/rand\"", "package")
/* time */ //@item(time, "time", "\"time\"", "package")
/* unsafe */ //@item(unsafe, "unsafe", "\"unsafe\"", "package")
/* pkg */ //@item(externalpackage, "pkg", "\"example.com/extramodule/pkg\"", "package" )
/* pkg */ //@item(externalpackage, "pkg", "\"example.com/extramodule/pkg\"", "package")

/* bytes.Buffer */ //@item(bytesbuffer, "Buffer", "(from \"bytes\")", "var" )
/* ring.Ring */ //@item(ringring, "Ring", "(from \"container/ring\")", "var")

/* signature.Foo */ //@item(signaturefoo, "Foo", "func(a string, b int) (c bool) (from \"golang.org/x/tools/internal/lsp/signature\")", "func")
5 changes: 4 additions & 1 deletion internal/lsp/testdata/unimported/unimported_cand_type.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package unimported

import "golang.org/x/tools/internal/lsp/baz"
import (
"golang.org/x/tools/internal/lsp/baz"
"golang.org/x/tools/internal/lsp/signature" // provide type information for unimported completions in the other file
)

func _() {
foo.StructFoo{} //@item(litFooStructFoo, "foo.StructFoo{}", "struct{...}", "struct")
Expand Down

0 comments on commit f505e54

Please sign in to comment.