From 330b9f13843665b0e77c9a64b2873b8d04a6df67 Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Thu, 5 Dec 2019 18:08:19 -0500 Subject: [PATCH] internal/lsp/source: cap number of unimported completions Building unimported completions requires re-parsing and formatting at least some of the file for each one, which adds up. Limit it to 20; I expect people will just type more rather than scroll through a giant list. Updates golang/go#36001. Change-Id: Ib41232b91c327d4b824e6176e30306abf356f5b4 Reviewed-on: https://go-review.googlesource.com/c/tools/+/210198 Run-TryBot: Heschi Kreinick TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/source/completion.go | 12 ++++++++++++ internal/lsp/testdata/summary.txt.golden | 2 +- internal/lsp/testdata/unimported/unimported.go.in | 10 +++++----- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 52d0bbefa5..72d3805e07 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -547,6 +547,9 @@ func (c *completer) wantTypeName() bool { return c.expectedType.typeName.wantTypeName } +// See https://golang.org/issue/36001. Unimported completions are expensive. +const maxUnimported = 20 + // selector finds completions for the specified selector expression. func (c *completer) selector(sel *ast.SelectorExpr) error { // Is sel a qualified identifier? @@ -570,7 +573,11 @@ func (c *completer) selector(sel *ast.SelectorExpr) error { return err } known := c.snapshot.KnownImportPaths() + startingItems := len(c.items) for _, pkgExport := range pkgExports { + if len(c.items)-startingItems >= maxUnimported { + break + } // 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(), &importInfo{ @@ -726,7 +733,12 @@ func (c *completer) lexical() error { score := stdScore // Rank unimported packages significantly lower than other results. score *= 0.07 + + startingItems := len(c.items) for _, pkg := range pkgs { + if len(c.items)-startingItems >= maxUnimported { + break + } if _, ok := seen[pkg.IdentName]; !ok { // Do not add the unimported packages to seen, since we can have // multiple packages of the same name as completion suggestions, since diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden index 3c952c0c1d..ec01935171 100644 --- a/internal/lsp/testdata/summary.txt.golden +++ b/internal/lsp/testdata/summary.txt.golden @@ -1,7 +1,7 @@ -- summary -- CompletionsCount = 219 CompletionSnippetCount = 51 -UnimportedCompletionsCount = 3 +UnimportedCompletionsCount = 4 DeepCompletionsCount = 5 FuzzyCompletionsCount = 7 RankedCompletionsCount = 26 diff --git a/internal/lsp/testdata/unimported/unimported.go.in b/internal/lsp/testdata/unimported/unimported.go.in index 3b78f7274a..d4ce8b3d25 100644 --- a/internal/lsp/testdata/unimported/unimported.go.in +++ b/internal/lsp/testdata/unimported/unimported.go.in @@ -1,18 +1,18 @@ package unimported func _() { - //@unimported("", bytes, context, cryptoslashrand, time, unsafe, externalpackage) + //@unimported("", hashslashadler32, goslashast, encodingslashbase64, bytes) + 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) } // 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") -/* context */ //@item(context, "context", "\"context\"", "package") -/* 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") /* ring.Ring */ //@item(ringring, "Ring", "(from \"container/ring\")", "var")