qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-09-30 22:58:34 -06:00
Code Releases Activity

internal/imports,lsp: use callbacks for completion functions

Browse Source 
We only need to return a relatively small number of completions to the
user. There's no point continuing once we have those, so switch the
completion functions to be callback-based, and cancel once we've got
what we want.

Change-Id: Ied199fb1f41346819c7237dfed8251fa3ac73ad7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212634
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This commit is contained in:
Heschi Kreinick 2019-12-27 15:46:49 -05:00
parent 3f7dfa39cf
commit c2a8f45ada
8 changed files with 175 additions and 162 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
internal/imports/fix.go
View File

@ -82,7 +82,7 @@ type ImportFix struct {
IdentName string
// FixType is the type of fix this is (AddImport, DeleteImport, SetImportName).
FixType ImportFixType
relevance int // see pkg
Relevance int // see pkg
}
// An ImportInfo represents a single import statement.
@ -585,6 +585,10 @@ func getFixes(fset *token.FileSet, f *ast.File, filename string, env *ProcessEnv
return fixes, nil
}
// Highest relevance, used for the standard library. Chosen arbitrarily to
// match pre-existing gopls code.
const MaxRelevance = 7
// getCandidatePkgs returns the list of pkgs that are accessible from filename,
// filtered to those that match pkgnameFilter.
func getCandidatePkgs(ctx context.Context, wrappedCallback *scanCallback, filename string, env *ProcessEnv) error {
@ -596,7 +600,7 @@ func getCandidatePkgs(ctx context.Context, wrappedCallback *scanCallback, filena
dir: filepath.Join(env.GOROOT, "src", importPath),
importPathShort: importPath,
packageName: path.Base(importPath),
relevance: 0,
relevance: MaxRelevance,
}
if wrappedCallback.packageNameLoaded(p) {
wrappedCallback.exportsLoaded(p, exports)
@ -630,17 +634,6 @@ func getCandidatePkgs(ctx context.Context, wrappedCallback *scanCallback, filena
return env.GetResolver().scan(ctx, scanFilter, exclude)
}
// Compare first by relevance, then by package name, with import path as a tiebreaker.
func compareFix(fi, fj *ImportFix) bool {
if fi.relevance != fj.relevance {
return fi.relevance < fj.relevance
}
if fi.IdentName != fj.IdentName {
return fi.IdentName < fj.IdentName
}
return fi.StmtInfo.ImportPath < fj.StmtInfo.ImportPath
}
func candidateImportName(pkg *pkg) string {
if ImportPathToAssumedName(pkg.importPathShort) != pkg.packageName {
return pkg.packageName
@ -649,39 +642,32 @@ func candidateImportName(pkg *pkg) string {
}
// getAllCandidates gets all of the candidates to be imported, regardless of if they are needed.
func getAllCandidates(ctx context.Context, prefix string, filename string, env *ProcessEnv) ([]ImportFix, error) {
var mu sync.Mutex
var results []ImportFix
filter := &scanCallback{
func getAllCandidates(ctx context.Context, wrapped func(ImportFix), prefix string, filename string, env *ProcessEnv) error {
callback := &scanCallback{
dirFound: func(pkg *pkg) bool {
// TODO(heschi): apply dir match heuristics like pkgIsCandidate
return true
},
packageNameLoaded: func(pkg *pkg) bool {
if strings.HasPrefix(pkg.packageName, prefix) {
mu.Lock()
defer mu.Unlock()
results = append(results, ImportFix{
wrapped(ImportFix{
StmtInfo: ImportInfo{
ImportPath: pkg.importPathShort,
Name: candidateImportName(pkg),
},
IdentName: pkg.packageName,
FixType: AddImport,
relevance: pkg.relevance,
Relevance: pkg.relevance,
})
}
return false
},
}
err := getCandidatePkgs(ctx, filter, filename, env)
err := getCandidatePkgs(ctx, callback, filename, env)
if err != nil {
return nil, err
return err
}
sort.Slice(results, func(i, j int) bool {
return compareFix(&results[i], &results[j])
})
return results, nil
return nil
}
// A PackageExport is a package and its exports.
@ -690,9 +676,7 @@ type PackageExport struct {
Exports []string
}
func getPackageExports(ctx context.Context, completePackage, filename string, env *ProcessEnv) ([]PackageExport, error) {
var mu sync.Mutex
var results []PackageExport
func getPackageExports(ctx context.Context, wrapped func(PackageExport), completePackage, filename string, env *ProcessEnv) error {
callback := &scanCallback{
dirFound: func(pkg *pkg) bool {
// TODO(heschi): apply dir match heuristics like pkgIsCandidate
@ -702,10 +686,8 @@ func getPackageExports(ctx context.Context, completePackage, filename string, en
return pkg.packageName == completePackage
},
exportsLoaded: func(pkg *pkg, exports []string) {
mu.Lock()
defer mu.Unlock()
sort.Strings(exports)
results = append(results, PackageExport{
wrapped(PackageExport{
Fix: &ImportFix{
StmtInfo: ImportInfo{
ImportPath: pkg.importPathShort,
@ -713,7 +695,7 @@ func getPackageExports(ctx context.Context, completePackage, filename string, en
},
IdentName: pkg.packageName,
FixType: AddImport,
relevance: pkg.relevance,
Relevance: pkg.relevance,
},
Exports: exports,
})
@ -721,12 +703,9 @@ func getPackageExports(ctx context.Context, completePackage, filename string, en
}
err := getCandidatePkgs(ctx, callback, filename, env)
if err != nil {
return nil, err
return err
}
sort.Slice(results, func(i, j int) bool {
return compareFix(results[i].Fix, results[j].Fix)
})
return results, nil
return nil
}
// ProcessEnv contains environment variables and settings that affect the use of
@ -1175,10 +1154,10 @@ func (r *gopathResolver) scan(ctx context.Context, callback *scanCallback, exclu
p := &pkg{
importPathShort: info.nonCanonicalImportPath,
dir: dir,
relevance: 1,
relevance: MaxRelevance - 1,
}
if info.rootType == gopathwalk.RootGOROOT {
p.relevance = 0
p.relevance = MaxRelevance
}
if callback.dirFound(p) {

69
internal/imports/fix_test.go
View File

@ -13,6 +13,7 @@ import (
"path/filepath"
"reflect"
"runtime"
"sort"
"strings"
"sync"
"testing"
@ -2492,15 +2493,15 @@ var _ = bytes.Buffer{}
// with correct priorities.
func TestGetCandidates(t *testing.T) {
type res struct {
relevance int
name, path string
}
want := []res{
{"bytes", "bytes"},
{"http", "net/http"},
{"rand", "crypto/rand"},
{"rand", "math/rand"},
{"bar", "bar.com/bar"},
{"foo", "foo.com/foo"},
{0, "bytes", "bytes"},
{0, "http", "net/http"},
{0, "rand", "crypto/rand"},
{0, "bar", "bar.com/bar"},
{0, "foo", "foo.com/foo"},
}
testConfig{
@ -2515,18 +2516,31 @@ func TestGetCandidates(t *testing.T) {
},
},
}.test(t, func(t *goimportTest) {
candidates, err := getAllCandidates(context.Background(), "", "x.go", t.env)
if err != nil {
t.Fatalf("GetAllCandidates() = %v", err)
}
var mu sync.Mutex
var got []res
for _, c := range candidates {
add := func(c ImportFix) {
mu.Lock()
defer mu.Unlock()
for _, w := range want {
if c.StmtInfo.ImportPath == w.path {
got = append(got, res{c.IdentName, c.StmtInfo.ImportPath})
got = append(got, res{c.Relevance, c.IdentName, c.StmtInfo.ImportPath})
}
}
}
if err := getAllCandidates(context.Background(), add, "", "x.go", t.env); err != nil {
t.Fatalf("GetAllCandidates() = %v", err)
}
// Sort, then clear out relevance so it doesn't mess up the DeepEqual.
sort.Slice(got, func(i, j int) bool {
ri, rj := got[i], got[j]
if ri.relevance != rj.relevance {
return ri.relevance > rj.relevance // Highest first.
}
return ri.name < rj.name
})
for i := range got {
got[i].relevance = 0
}
if !reflect.DeepEqual(want, got) {
t.Errorf("wanted stdlib results in order %v, got %v", want, got)
}
@ -2535,12 +2549,12 @@ func TestGetCandidates(t *testing.T) {
func TestGetPackageCompletions(t *testing.T) {
type res struct {
relevance int
name, path, symbol string
}
want := []res{
{"rand", "crypto/rand", "Prime"},
{"rand", "math/rand", "Seed"},
{"rand", "bar.com/rand", "Bar"},
{0, "rand", "math/rand", "Seed"},
{0, "rand", "bar.com/rand", "Bar"},
}
testConfig{
@ -2551,20 +2565,33 @@ func TestGetPackageCompletions(t *testing.T) {
},
},
}.test(t, func(t *goimportTest) {
candidates, err := getPackageExports(context.Background(), "rand", "x.go", t.env)
if err != nil {
t.Fatalf("getPackageCompletions() = %v", err)
}
var mu sync.Mutex
var got []res
for _, c := range candidates {
add := func(c PackageExport) {
mu.Lock()
defer mu.Unlock()
for _, csym := range c.Exports {
for _, w := range want {
if c.Fix.StmtInfo.ImportPath == w.path && csym == w.symbol {
got = append(got, res{c.Fix.IdentName, c.Fix.StmtInfo.ImportPath, csym})
got = append(got, res{c.Fix.Relevance, c.Fix.IdentName, c.Fix.StmtInfo.ImportPath, csym})
}
}
}
}
if err := getPackageExports(context.Background(), add, "rand", "x.go", t.env); err != nil {
t.Fatalf("getPackageCompletions() = %v", err)
}
// Sort, then clear out relevance so it doesn't mess up the DeepEqual.
sort.Slice(got, func(i, j int) bool {
ri, rj := got[i], got[j]
if ri.relevance != rj.relevance {
return ri.relevance > rj.relevance // Highest first.
}
return ri.name < rj.name
})
for i := range got {
got[i].relevance = 0
}
if !reflect.DeepEqual(want, got) {
t.Errorf("wanted stdlib results in order %v, got %v", want, got)
}

16
internal/imports/imports.go
View File

@ -118,21 +118,21 @@ func ApplyFixes(fixes []*ImportFix, filename string, src []byte, opt *Options, e
// GetAllCandidates gets all of the packages starting with prefix that can be
// imported by filename, sorted by import path.
func GetAllCandidates(ctx context.Context, prefix string, filename string, opt *Options) (pkgs []ImportFix, err error) {
_, opt, err = initialize(filename, nil, opt)
func GetAllCandidates(ctx context.Context, callback func(ImportFix), prefix string, filename string, opt *Options) error {
_, opt, err := initialize(filename, nil, opt)
if err != nil {
return nil, err
return err
}
return getAllCandidates(ctx, prefix, filename, opt.Env)
return getAllCandidates(ctx, callback, prefix, filename, opt.Env)
}
// GetPackageExports returns all known packages with name pkg and their exports.
func GetPackageExports(ctx context.Context, pkg, filename string, opt *Options) (exports []PackageExport, err error) {
_, opt, err = initialize(filename, nil, opt)
func GetPackageExports(ctx context.Context, callback func(PackageExport), pkg, filename string, opt *Options) error {
_, opt, err := initialize(filename, nil, opt)
if err != nil {
return nil, err
return err
}
return getPackageExports(ctx, pkg, filename, opt.Env)
return getPackageExports(ctx, callback, pkg, filename, opt.Env)
}
// initialize sets the values for opt and src.

8
internal/imports/mod.go
View File

@ -425,18 +425,18 @@ func (r *ModuleResolver) canonicalize(info directoryPackageInfo) (*pkg, error) {
importPathShort: info.nonCanonicalImportPath,
dir: info.dir,
packageName: path.Base(info.nonCanonicalImportPath),
relevance: 0,
relevance: MaxRelevance,
}, nil
}
importPath := info.nonCanonicalImportPath
relevance := 3
relevance := MaxRelevance - 3
// Check if the directory is underneath a module that's in scope.
if mod := r.findModuleByDir(info.dir); mod != nil {
if mod.Indirect {
relevance = 2
relevance = MaxRelevance - 2
} else {
relevance = 1
relevance = MaxRelevance - 1
}
// It is. If dir is the target of a replace directive,
// our guessed import path is wrong. Use the real one.

35
internal/imports/mod_test.go
View File

@ -13,6 +13,7 @@ import (
"path/filepath"
"reflect"
"regexp"
"sort"
"strings"
"sync"
"testing"
@ -871,32 +872,42 @@ import _ "rsc.io/quote"
}
type res struct {
relevance int
name, path string
}
want := []res{
// Stdlib
{"bytes", "bytes"},
{"http", "net/http"},
{7, "bytes", "bytes"},
{7, "http", "net/http"},
// Direct module deps
{"quote", "rsc.io/quote"},
{6, "quote", "rsc.io/quote"},
{6, "rpackage", "example.com/rpackage"},
// Indirect deps
{"rpackage", "example.com/rpackage"},
{"language", "golang.org/x/text/language"},
{5, "language", "golang.org/x/text/language"},
// Out of scope modules
{"quote", "rsc.io/quote/v2"},
}
candidates, err := getAllCandidates(context.Background(), "", "foo.go", mt.env)
if err != nil {
t.Fatalf("getAllCandidates() = %v", err)
{4, "quote", "rsc.io/quote/v2"},
}
var mu sync.Mutex
var got []res
for _, c := range candidates {
add := func(c ImportFix) {
mu.Lock()
defer mu.Unlock()
for _, w := range want {
if c.StmtInfo.ImportPath == w.path {
got = append(got, res{c.IdentName, c.StmtInfo.ImportPath})
got = append(got, res{c.Relevance, c.IdentName, c.StmtInfo.ImportPath})
}
}
}
if err := getAllCandidates(context.Background(), add, "", "foo.go", mt.env); err != nil {
t.Fatalf("getAllCandidates() = %v", err)
}
sort.Slice(got, func(i, j int) bool {
ri, rj := got[i], got[j]
if ri.relevance != rj.relevance {
return ri.relevance > rj.relevance // Highest first.
}
return ri.name < rj.name
})
if !reflect.DeepEqual(want, got) {
t.Errorf("wanted candidates in order %v, got %v", want, got)
}

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

@ -14,6 +14,7 @@ import (
"math"
"strconv"
"strings"
"sync"
"time"
"golang.org/x/tools/go/ast/astutil"
@ -245,6 +246,13 @@ func (p Selection) Suffix() string {
return p.content[p.cursor-p.spanRange.Start:]
}
func (c *completer) deepCompletionContext() (context.Context, context.CancelFunc) {
if c.opts.Budget == 0 {
return context.WithCancel(c.ctx)
}
return context.WithDeadline(c.ctx, c.startTime.Add(c.opts.Budget))
}
func (c *completer) setSurrounding(ident *ast.Ident) {
if c.surrounding != nil {
return
@ -622,15 +630,11 @@ 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 {
pkgExports, err := PackageExports(c.ctx, c.snapshot.View(), id.Name, c.filename)
if err != nil {
return err
}
ctx, cancel := c.deepCompletionContext()
defer cancel()
known := c.snapshot.KnownImportPaths()
for _, pkgExport := range pkgExports {
if len(c.items) >= unimportedTarget {
break
}
add := func(pkgExport imports.PackageExport) {
// 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{
@ -638,22 +642,30 @@ func (c *completer) selector(sel *ast.SelectorExpr) error {
name: pkgExport.Fix.StmtInfo.Name,
pkg: knownPkg,
})
continue
} else {
// Otherwise, continue with untyped proposals.
pkg := types.NewPackage(pkgExport.Fix.StmtInfo.ImportPath, pkgExport.Fix.IdentName)
for _, export := range pkgExport.Exports {
score := 0.01 * float64(pkgExport.Fix.Relevance)
c.found(candidate{
obj: types.NewVar(0, pkg, export, nil),
score: score,
imp: &importInfo{
importPath: pkgExport.Fix.StmtInfo.ImportPath,
name: pkgExport.Fix.StmtInfo.Name,
},
})
}
}
// Otherwise, continue with untyped proposals.
pkg := types.NewPackage(pkgExport.Fix.StmtInfo.ImportPath, pkgExport.Fix.IdentName)
for _, export := range pkgExport.Exports {
c.found(candidate{
obj: types.NewVar(0, pkg, export, nil),
score: 0.07,
imp: &importInfo{
importPath: pkgExport.Fix.StmtInfo.ImportPath,
name: pkgExport.Fix.StmtInfo.Name,
},
})
if len(c.items) >= unimportedTarget {
cancel()
}
}
if err := c.snapshot.View().RunProcessEnvFunc(ctx, func(opts *imports.Options) error {
return imports.GetPackageExports(ctx, add, id.Name, c.filename, opts)
}); err != nil && err != context.Canceled {
return err
}
}
return nil
}
@ -830,39 +842,52 @@ func (c *completer) lexical() error {
}
if c.opts.Unimported && len(c.items) < unimportedTarget {
ctx, cancel := context.WithDeadline(c.ctx, c.startTime.Add(c.opts.Budget))
ctx, cancel := c.deepCompletionContext()
defer cancel()
// Suggest packages that have not been imported yet.
prefix := ""
if c.surrounding != nil {
prefix = c.surrounding.Prefix()
}
pkgs, err := CandidateImports(ctx, prefix, c.snapshot.View(), c.filename)
if err != nil {
return err
}
score := stdScore
// Rank unimported packages significantly lower than other results.
score *= 0.07
var mu sync.Mutex
add := func(pkg imports.ImportFix) {
mu.Lock()
defer mu.Unlock()
if _, ok := seen[pkg.IdentName]; ok {
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,
},
})
for _, pkg := range pkgs {
if len(c.items) >= unimportedTarget {
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
// 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,
},
})
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, opts)
}); err != nil && err != context.Canceled {
return err
}
}

31
internal/lsp/source/format.go
View File

@ -303,37 +303,6 @@ func trimToFirstNonImport(fset *token.FileSet, f *ast.File, src []byte, err erro
return src[0:fset.Position(end).Offset], true
}
// CandidateImports returns every import that could be added to filename.
func CandidateImports(ctx context.Context, prefix string, view View, filename string) ([]imports.ImportFix, error) {
ctx, done := trace.StartSpan(ctx, "source.CandidateImports")
defer done()
var imps []imports.ImportFix
importFn := func(opts *imports.Options) error {
var err error
imps, err = imports.GetAllCandidates(ctx, prefix, filename, opts)
return err
}
err := view.RunProcessEnvFunc(ctx, importFn)
return imps, err
}
// PackageExports returns all the packages named pkg that could be imported by
// filename, and their exports.
func PackageExports(ctx context.Context, view View, pkg, filename string) ([]imports.PackageExport, error) {
ctx, done := trace.StartSpan(ctx, "source.PackageExports")
defer done()
var pkgs []imports.PackageExport
importFn := func(opts *imports.Options) error {
var err error
pkgs, err = imports.GetPackageExports(ctx, pkg, filename, opts)
return err
}
err := view.RunProcessEnvFunc(ctx, importFn)
return pkgs, err
}
// hasParseErrors returns true if the given file has parse errors.
func hasParseErrors(pkg Package, uri span.URI) bool {
for _, e := range pkg.GetErrors() {

2
internal/lsp/tests/tests.go
View File

@ -20,6 +20,7 @@ import (
"strings"
"sync"
"testing"
"time"
"golang.org/x/tools/go/expect"
"golang.org/x/tools/go/packages"
@ -190,6 +191,7 @@ func DefaultOptions() source.Options {
}
o.HoverKind = source.SynopsisDocumentation
o.InsertTextFormat = protocol.SnippetTextFormat
o.Completion.Budget = time.Minute
return o
}