qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-18 16:44:43 -07:00
Code Releases Activity

internal/lsp/source: limit to 5 unimported package name candidates

Browse Source 
Currently we show up to ~100 unimported package names matching the
completion prefix. This isn't useful since, assuming the user even
wants an unimported package, they will just type more to narrow it
down rather than scroll through 100 options. Having so many candidates
also slows things down due to per-candidate overhead in gopls and in
the LSP client. Now we instead limit to 5 unimported package names.

Unimported package members, on the other hand, make sense to list
many. The user may want to scroll through because they don't remember
the name of what they are looking for. I left the max value at 100.

Change-Id: I00e11fa0420758f8db6c7049f80fa156773a5ee6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218879
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This commit is contained in:
Muir Manders 2020-02-10 11:54:59 -08:00 committed by Heschi Kreinick
parent b0390335f4
commit 0bc66720f3
2 changed files with 28 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
internal/lsp/source/completion.go
View File

@ -663,14 +663,17 @@ func (c *completer) wantTypeName() bool {
}
// See https://golang.org/issue/36001. Unimported completions are expensive.
const unimportedTarget = 100
const (
maxUnimportedPackageNames = 5
unimportedMemberTarget = 100
)
// selector finds completions for the specified selector expression.
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.Imported(), stdScore, nil)
if pkgName, ok := c.pkg.GetTypesInfo().Uses[id].(*types.PkgName); ok {
c.packageMembers(pkgName.Imported(), stdScore, nil)
return nil
}
}
@ -682,7 +685,7 @@ func (c *completer) selector(sel *ast.SelectorExpr) error {
}
// Try unimported packages.
if id, ok := sel.X.(*ast.Ident); ok && c.opts.unimported && len(c.items) < unimportedTarget {
if id, ok := sel.X.(*ast.Ident); ok && c.opts.unimported {
if err := c.unimportedMembers(id); err != nil {
return err
}
@ -696,6 +699,7 @@ func (c *completer) unimportedMembers(id *ast.Ident) error {
if err != nil {
return err
}
var paths []string
for path, pkg := range known {
if pkg.GetTypes().Name() != id.Name {
@ -703,6 +707,7 @@ func (c *completer) unimportedMembers(id *ast.Ident) error {
}
paths = append(paths, path)
}
var relevances map[string]int
if len(paths) != 0 {
c.snapshot.View().RunProcessEnvFunc(c.ctx, func(opts *imports.Options) error {
@ -710,6 +715,7 @@ func (c *completer) unimportedMembers(id *ast.Ident) error {
return nil
})
}
for path, pkg := range known {
if pkg.GetTypes().Name() != id.Name {
continue
@ -722,7 +728,7 @@ func (c *completer) unimportedMembers(id *ast.Ident) error {
imp.name = pkg.GetTypes().Name()
}
c.packageMembers(pkg.GetTypes(), stdScore+.01*float64(relevances[path]), imp)
if len(c.items) >= unimportedTarget {
if len(c.items) >= unimportedMemberTarget {
return nil
}
}
@ -750,7 +756,7 @@ func (c *completer) unimportedMembers(id *ast.Ident) error {
},
})
}
if len(c.items) >= unimportedTarget {
if len(c.items) >= unimportedMemberTarget {
cancel()
}
}
@ -930,7 +936,7 @@ func (c *completer) lexical() error {
}
}
if c.opts.unimported && len(c.items) < unimportedTarget {
if c.opts.unimported {
ctx, cancel := c.deepCompletionContext()
defer cancel()
// Suggest packages that have not been imported yet.
@ -938,13 +944,22 @@ func (c *completer) lexical() error {
if c.surrounding != nil {
prefix = c.surrounding.Prefix()
}
var mu sync.Mutex
var (
mu sync.Mutex
initialItemCount = len(c.items)
)
add := func(pkg imports.ImportFix) {
mu.Lock()
defer mu.Unlock()
if _, ok := seen[pkg.IdentName]; ok {
return
}
if len(c.items)-initialItemCount >= maxUnimportedPackageNames {
cancel()
return
}
// Rank unimported packages significantly lower than other results.
score := 0.01 * float64(pkg.Relevance)
@ -960,18 +975,6 @@ func (c *completer) lexical() error {
name: pkg.StmtInfo.Name,
},
})
if len(c.items) >= unimportedTarget {
cancel()
}
c.found(candidate{
obj: obj,
score: score,
imp: &importInfo{
importPath: pkg.StmtInfo.ImportPath,
name: pkg.StmtInfo.Name,
},
})
}
if err := c.snapshot.View().RunProcessEnvFunc(ctx, func(opts *imports.Options) error {
return imports.GetAllCandidates(ctx, add, prefix, c.filename, c.pkg.GetTypes().Name(), opts)

12
internal/lsp/testdata/lsp/primarymod/unimported/unimported.go.in vendored
View File

@ -1,19 +1,17 @@
package unimported
func _() {
//@unimported("", hashslashadler32, goslashast, encodingslashbase64, bytes)
pkg //@unimported("g", externalpackage)
http //@unimported("p", nethttp, nethttptest)
pkg //@unimported("g", externalpackage)
// 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)
context.Bac //@unimported("Bac", contextBackground, contextBackgroundErr)
context.Bac //@unimported("Bac", contextBackground, contextBackgroundErr)
}
// Create markers for unimported std lib packages. Only for use by this test.
/* adler32 */ //@item(hashslashadler32, "adler32", "\"hash/adler32\"", "package")
/* ast */ //@item(goslashast, "ast", "\"go/ast\"", "package")
/* base64 */ //@item(encodingslashbase64, "base64", "\"encoding/base64\"", "package")
/* bytes */ //@item(bytes, "bytes", "\"bytes\"", "package")
/* http */ //@item(nethttp, "http", "\"net/http\"", "package")
/* httptest */ //@item(nethttptest, "httptest", "\"net/http/httptest\"", "package")
/* pkg */ //@item(externalpackage, "pkg", "\"example.com/extramodule/pkg\"", "package")
/* ring.Ring */ //@item(ringring, "Ring", "(from \"container/ring\")", "var")