From 6edc0a871e694a1c8728996c84668863c13d2b4f Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Thu, 16 Jan 2020 14:54:35 -0500 Subject: [PATCH] internal/lsp/source: score in-memory unimported candidates We were assuming that all in-memory packages were equally useful. That's not true for projects with a large dependency tree. Call into the imports code to score them. While I'm here, score the main module above direct deps. Updates golang/go#36591. Change-Id: I07c56dd3ff7338e76f3643e18d35abc1b52d6763 Reviewed-on: https://go-review.googlesource.com/c/tools/+/215023 Run-TryBot: Heschi Kreinick TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/imports/fix.go | 17 ++++++++++++++++ internal/imports/fix_test.go | 8 ++++---- internal/imports/mod.go | 32 +++++++++++++++++++++++-------- internal/imports/mod_test.go | 9 +++++---- internal/lsp/source/completion.go | 18 ++++++++++++++--- 5 files changed, 65 insertions(+), 19 deletions(-) diff --git a/internal/imports/fix.go b/internal/imports/fix.go index 83349a07da..ac8f6b153d 100644 --- a/internal/imports/fix.go +++ b/internal/imports/fix.go @@ -643,6 +643,14 @@ func getCandidatePkgs(ctx context.Context, wrappedCallback *scanCallback, filena return env.GetResolver().scan(ctx, scanFilter) } +func ScoreImportPaths(ctx context.Context, env *ProcessEnv, paths []string) map[string]int { + result := make(map[string]int) + for _, path := range paths { + result[path] = env.GetResolver().scoreImportPath(ctx, path) + } + return result +} + func PrimeCache(ctx context.Context, env *ProcessEnv) error { // Fully scan the disk for directories, but don't actually read any Go files. callback := &scanCallback{ @@ -874,6 +882,8 @@ type Resolver interface { // loadExports returns the set of exported symbols in the package at dir. // loadExports may be called concurrently. loadExports(ctx context.Context, pkg *pkg, includeTest bool) (string, []string, error) + // scoreImportPath returns the relevance for an import path. + scoreImportPath(ctx context.Context, path string) int ClearForNewScan() } @@ -1251,6 +1261,13 @@ func (r *gopathResolver) scan(ctx context.Context, callback *scanCallback) error return nil } +func (r *gopathResolver) scoreImportPath(ctx context.Context, path string) int { + if _, ok := stdlib[path]; ok { + return MaxRelevance + } + return MaxRelevance - 1 +} + func filterRoots(roots []gopathwalk.Root, include func(gopathwalk.Root) bool) []gopathwalk.Root { var result []gopathwalk.Root for _, root := range roots { diff --git a/internal/imports/fix_test.go b/internal/imports/fix_test.go index 37785fd2f5..67ebe53313 100644 --- a/internal/imports/fix_test.go +++ b/internal/imports/fix_test.go @@ -2539,14 +2539,14 @@ func TestGetCandidates(t *testing.T) { testConfig{ modules: []packagestest.Module{ - { - Name: "foo.com", - Files: fm{"foo/foo.go": "package foo\n"}, - }, { Name: "bar.com", Files: fm{"bar/bar.go": "package bar\n"}, }, + { + Name: "foo.com", + Files: fm{"foo/foo.go": "package foo\n"}, + }, }, }.test(t, func(t *goimportTest) { var mu sync.Mutex diff --git a/internal/imports/mod.go b/internal/imports/mod.go index f38e683239..553f216450 100644 --- a/internal/imports/mod.go +++ b/internal/imports/mod.go @@ -480,6 +480,27 @@ func (r *ModuleResolver) scan(ctx context.Context, callback *scanCallback) error return nil } +func (r *ModuleResolver) scoreImportPath(ctx context.Context, path string) int { + if _, ok := stdlib[path]; ok { + return MaxRelevance + } + mod, _ := r.findPackage(path) + return modRelevance(mod) +} + +func modRelevance(mod *ModuleJSON) int { + switch { + case mod == nil: // out of scope + return MaxRelevance - 4 + case mod.Indirect: + return MaxRelevance - 3 + case !mod.Main: + return MaxRelevance - 2 + default: + return MaxRelevance - 1 // main module ties with stdlib + } +} + // canonicalize gets the result of canonicalizing the packages using the results // of initializing the resolver from 'go list -m'. func (r *ModuleResolver) canonicalize(info directoryPackageInfo) (*pkg, error) { @@ -494,14 +515,9 @@ func (r *ModuleResolver) canonicalize(info directoryPackageInfo) (*pkg, error) { } importPath := info.nonCanonicalImportPath - relevance := MaxRelevance - 3 + mod := r.findModuleByDir(info.dir) // Check if the directory is underneath a module that's in scope. - if mod := r.findModuleByDir(info.dir); mod != nil { - if mod.Indirect { - relevance = MaxRelevance - 2 - } else { - relevance = MaxRelevance - 1 - } + if mod != nil { // It is. If dir is the target of a replace directive, // our guessed import path is wrong. Use the real one. if mod.Dir == info.dir { @@ -519,7 +535,7 @@ func (r *ModuleResolver) canonicalize(info directoryPackageInfo) (*pkg, error) { res := &pkg{ importPathShort: importPath, dir: info.dir, - relevance: relevance, + relevance: modRelevance(mod), } // We may have discovered a package that has a different version // in scope already. Canonicalize to that one if possible. diff --git a/internal/imports/mod_test.go b/internal/imports/mod_test.go index fc022ae386..882be4b129 100644 --- a/internal/imports/mod_test.go +++ b/internal/imports/mod_test.go @@ -880,13 +880,14 @@ import _ "rsc.io/quote" // Stdlib {7, "bytes", "bytes"}, {7, "http", "net/http"}, - // Direct module deps - {6, "quote", "rsc.io/quote"}, + // Main module {6, "rpackage", "example.com/rpackage"}, + // Direct module deps + {5, "quote", "rsc.io/quote"}, // Indirect deps - {5, "language", "golang.org/x/text/language"}, + {4, "language", "golang.org/x/text/language"}, // Out of scope modules - {4, "quote", "rsc.io/quote/v2"}, + {3, "quote", "rsc.io/quote/v2"}, } var mu sync.Mutex var got []res diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 62e0f3851e..41b43fb14e 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -656,12 +656,24 @@ 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 { + 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, pkg := range known { if pkg.GetTypes().Name() != id.Name { continue } - // We don't know what this is, so assign it the highest score. - score := 0.01 * imports.MaxRelevance imp := &importInfo{ importPath: path, pkg: pkg, @@ -669,7 +681,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(), score, imp) + c.packageMembers(pkg.GetTypes(), .01*float64(relevances[path]), imp) if len(c.items) >= unimportedTarget { return nil }