Skip to content

Commit f505e54

Browse files
committed
internal/lsp: use available type info for unimported completions
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Rebecca Stambler <[email protected]>
1 parent 88c5938 commit f505e54

File tree

6 files changed

+48
-5
lines changed

6 files changed

+48
-5
lines changed

internal/lsp/cache/snapshot.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,30 @@ func (s *snapshot) getPackages(uri span.URI, m source.ParseMode) (cphs []source.
9898
return cphs
9999
}
100100

101+
func (s *snapshot) KnownImportPaths() map[string]source.Package {
102+
s.mu.Lock()
103+
defer s.mu.Unlock()
104+
105+
results := map[string]source.Package{}
106+
for _, cph := range s.packages {
107+
cachedPkg, err := cph.cached()
108+
if err != nil {
109+
continue
110+
}
111+
for importPath, newPkg := range cachedPkg.imports {
112+
if oldPkg, ok := results[string(importPath)]; ok {
113+
// Using the same trick as NarrowestPackageHandle, prefer non-variants.
114+
if len(newPkg.files) < len(oldPkg.(*pkg).files) {
115+
results[string(importPath)] = newPkg
116+
}
117+
} else {
118+
results[string(importPath)] = newPkg
119+
}
120+
}
121+
}
122+
return results
123+
}
124+
101125
func (s *snapshot) getPackage(id packageID, m source.ParseMode) *checkPackageHandle {
102126
s.mu.Lock()
103127
defer s.mu.Unlock()

internal/lsp/source/completion.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,15 @@ func (c *completer) selector(sel *ast.SelectorExpr) error {
587587
if err != nil {
588588
return err
589589
}
590+
known := c.snapshot.KnownImportPaths()
590591
for _, pkgExport := range pkgExports {
592+
// If we've seen this import path, use the fully-typed version.
593+
if knownPkg, ok := known[pkgExport.Fix.StmtInfo.ImportPath]; ok {
594+
c.packageMembers(knownPkg.GetTypes(), &pkgExport.Fix.StmtInfo)
595+
continue
596+
}
597+
598+
// Otherwise, continue with untyped proposals.
591599
pkg := types.NewPackage(pkgExport.Fix.StmtInfo.ImportPath, pkgExport.Fix.IdentName)
592600
for _, export := range pkgExport.Exports {
593601
c.found(types.NewVar(0, pkg, export, nil), 0.07, &pkgExport.Fix.StmtInfo)

internal/lsp/source/view.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,10 @@ type Snapshot interface {
275275
// CheckPackageHandles returns the CheckPackageHandles for the packages
276276
// that this file belongs to.
277277
CheckPackageHandles(ctx context.Context, f File) ([]CheckPackageHandle, error)
278+
279+
// KnownImportPaths returns all the packages loaded in this snapshot,
280+
// indexed by their import path.
281+
KnownImportPaths() map[string]Package
278282
}
279283

280284
// File represents a source file of any type.

internal/lsp/testdata/summary.txt.golden

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
-- summary --
22
CompletionsCount = 213
33
CompletionSnippetCount = 40
4-
UnimportedCompletionsCount = 2
4+
UnimportedCompletionsCount = 3
55
DeepCompletionsCount = 5
66
FuzzyCompletionsCount = 7
77
RankedCompletionsCount = 8

internal/lsp/testdata/unimported/unimported.go.in

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ package unimported
22

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

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

16-
/* bytes.Buffer */ //@item(bytesbuffer, "Buffer", "(from \"bytes\")", "var" )
18+
/* ring.Ring */ //@item(ringring, "Ring", "(from \"container/ring\")", "var")
19+
20+
/* signature.Foo */ //@item(signaturefoo, "Foo", "func(a string, b int) (c bool) (from \"golang.org/x/tools/internal/lsp/signature\")", "func")

internal/lsp/testdata/unimported/unimported_cand_type.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package unimported
22

3-
import "golang.org/x/tools/internal/lsp/baz"
3+
import (
4+
"golang.org/x/tools/internal/lsp/baz"
5+
"golang.org/x/tools/internal/lsp/signature" // provide type information for unimported completions in the other file
6+
)
47

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

0 commit comments

Comments
 (0)