From 0a46fa39b0518991fd295afa50ddf2094cd2fe71 Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Tue, 31 Mar 2020 13:19:04 -0400 Subject: [PATCH] internal/lsp/source: propose loaded unimported package names first When proposing unimported package members, we start with packages loaded in-memory first, for three reasons: we know that they're fast, they're fully typed, and they're probably pretty relevant. I hadn't bothered doing that for package names, because the first two reasons aren't very relevant. But the third still is -- loaded packages are a pretty good approximation for in module scope, etc. With this change we do package names the same as members, so relevance should be about as good. Not perfect, but nobody's complained much yet. Fair bit of copy-and-paste, but I don't want to extend the getAllCandidates abstraction outside of the imports package yet. Updates #38104. Change-Id: Ia479181607dff898baee3cd6aa84d1ab61715d19 Reviewed-on: https://go-review.googlesource.com/c/tools/+/226559 Run-TryBot: Heschi Kreinick TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/source/completion.go | 130 ++++++++++++++++++++---------- 1 file changed, 86 insertions(+), 44 deletions(-) diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 7637b3b8d8..e13ee6ed69 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -795,7 +795,8 @@ func (c *completer) unimportedMembers(id *ast.Ident) error { }) } - for path, pkg := range known { + for path, relevance := range relevances { + pkg := known[path] if pkg.GetTypes().Name() != id.Name { continue } @@ -806,7 +807,7 @@ func (c *completer) unimportedMembers(id *ast.Ident) error { if imports.ImportPathToAssumedName(path) != pkg.GetTypes().Name() { imp.name = pkg.GetTypes().Name() } - c.packageMembers(pkg.GetTypes(), stdScore+.01*float64(relevances[path]), imp) + c.packageMembers(pkg.GetTypes(), stdScore+.01*float64(relevance), imp) if len(c.items) >= unimportedMemberTarget { return nil } @@ -1017,48 +1018,7 @@ func (c *completer) lexical() error { } if c.opts.unimported { - ctx, cancel := c.deepCompletionContext() - defer cancel() - // Suggest packages that have not been imported yet. - prefix := "" - if c.surrounding != nil { - prefix = c.surrounding.Prefix() - } - 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) - - // Do not add the unimported packages to seen, since we can have - // multiple packages of the same name as completion suggestions, since - // only one will be chosen. - obj := types.NewPkgName(0, nil, pkg.IdentName, types.NewPackage(pkg.StmtInfo.ImportPath, pkg.IdentName)) - 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) - }); err != nil { + if err := c.unimportedPackages(seen); err != nil { return err } } @@ -1086,6 +1046,88 @@ func (c *completer) lexical() error { return nil } +func (c *completer) unimportedPackages(seen map[string]struct{}) error { + prefix := "" + if c.surrounding != nil { + prefix = c.surrounding.Prefix() + } + initialItemCount := len(c.items) + + known, err := c.snapshot.CachedImportPaths(c.ctx) + if err != nil { + return err + } + var paths []string + for path := range known { + if !strings.HasPrefix(path, prefix) { + continue + } + paths = append(paths, path) + } + + var relevances map[string]int + if len(paths) != 0 { + c.snapshot.View().RunProcessEnvFunc(c.ctx, func(opts *imports.Options) error { + relevances = imports.ScoreImportPaths(c.ctx, opts.Env, paths) + return nil + }) + } + + for path, relevance := range relevances { + pkg := known[path] + imp := &importInfo{ + importPath: path, + pkg: pkg, + } + if imports.ImportPathToAssumedName(path) != pkg.GetTypes().Name() { + imp.name = pkg.GetTypes().Name() + } + score := 0.01 * float64(relevance) + c.found(candidate{ + obj: types.NewPkgName(0, nil, pkg.GetTypes().Name(), pkg.GetTypes()), + score: score, + imp: imp, + }) + if len(c.items)-initialItemCount >= maxUnimportedPackageNames { + return nil + } + } + + ctx, cancel := c.deepCompletionContext() + defer cancel() + var mu sync.Mutex + 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) + + // Do not add the unimported packages to seen, since we can have + // multiple packages of the same name as completion suggestions, since + // only one will be chosen. + obj := types.NewPkgName(0, nil, pkg.IdentName, types.NewPackage(pkg.StmtInfo.ImportPath, pkg.IdentName)) + c.found(candidate{ + obj: obj, + score: score, + imp: &importInfo{ + importPath: pkg.StmtInfo.ImportPath, + name: pkg.StmtInfo.Name, + }, + }) + } + return c.snapshot.View().RunProcessEnvFunc(ctx, func(opts *imports.Options) error { + return imports.GetAllCandidates(ctx, add, prefix, c.filename, c.pkg.GetTypes().Name(), opts) + }) +} + // alreadyImports reports whether f has an import with the specified path. func alreadyImports(f *ast.File, path string) bool { for _, s := range f.Imports {