From 84ab570110b3eba77a314c79f9f7734fe681a60c Mon Sep 17 00:00:00 2001 From: Danish Dua Date: Thu, 20 Aug 2020 18:49:00 -0400 Subject: [PATCH] internal/lsp: add completion suggestions for import statements This change adds completion within import blocks. Completions are suggested by directory depth of import so end user isn't shown a large list of possible imports at once. As an example, searching import for prefix "golang" would suggest "golang.org/" and then subdirectories under that (ex: "golang.org/x/"") on successive completion request and so on until a complete package path is selected. Change-Id: I962d32f2b7eef2c6b2ce8dc8a326ea34c726aa36 Reviewed-on: https://go-review.googlesource.com/c/tools/+/250301 Run-TryBot: Danish Dua TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/imports/fix.go | 2 +- internal/lsp/source/completion.go | 106 +++++++++++++++++- .../importedcomplit/imported_complit.go | 26 ----- .../importedcomplit/imported_complit.go.in | 41 +++++++ internal/lsp/testdata/lsp/summary.txt.golden | 2 +- 5 files changed, 146 insertions(+), 31 deletions(-) delete mode 100644 internal/lsp/testdata/lsp/primarymod/importedcomplit/imported_complit.go create mode 100644 internal/lsp/testdata/lsp/primarymod/importedcomplit/imported_complit.go.in diff --git a/internal/imports/fix.go b/internal/imports/fix.go index 62d9fe86a0..613afc4d66 100644 --- a/internal/imports/fix.go +++ b/internal/imports/fix.go @@ -615,7 +615,7 @@ func getCandidatePkgs(ctx context.Context, wrappedCallback *scanCallback, filena packageName: path.Base(importPath), relevance: MaxRelevance, } - if notSelf(p) && wrappedCallback.packageNameLoaded(p) { + if notSelf(p) && wrappedCallback.dirFound(p) && wrappedCallback.packageNameLoaded(p) { wrappedCallback.exportsLoaded(p, exports) } } diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index e539461d18..7a08f522ce 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -492,7 +492,12 @@ func Completion(ctx context.Context, snapshot Snapshot, fh FileHandle, protoPos // Check if completion at this position is valid. If not, return early. switch n := path[0].(type) { case *ast.BasicLit: - // Skip completion inside any kind of literal. + // Skip completion inside literals except for ImportSpec + if len(path) > 1 { + if _, ok := path[1].(*ast.ImportSpec); ok { + break + } + } return nil, nil, nil case *ast.CallExpr: if n.Ellipsis.IsValid() && pos > n.Ellipsis && pos <= n.Ellipsis+token.Pos(len("...")) { @@ -567,7 +572,18 @@ func Completion(ctx context.Context, snapshot Snapshot, fh FileHandle, protoPos defer c.sortItems() - // If we're inside a comment return comment completions + // Inside import blocks, return completions for unimported packages. + for _, importSpec := range pgf.File.Imports { + if !(importSpec.Path.Pos() <= rng.Start && rng.Start <= importSpec.Path.End()) { + continue + } + if err := c.populateImportCompletions(ctx, importSpec); err != nil { + return nil, nil, err + } + return c.items, c.getSurrounding(), nil + } + + // Inside comments, offer completions for the name of the relevant symbol. for _, comment := range pgf.File.Comments { if comment.Pos() < rng.Start && rng.Start <= comment.End() { // deep completion doesn't work properly in comments since we don't @@ -735,7 +751,91 @@ func (c *completer) emptySwitchStmt() bool { } } -// populateCommentCompletions yields completions for comments preceding or in declarations +// populateImportCompletions yields completions for an import path around the cursor. +// +// Completions are suggested at the directory depth of the given import path so +// that we don't overwhelm the user with a large list of possibilities. As an +// example, a completion for the prefix "golang" results in "golang.org/". +// Completions for "golang.org/" yield its subdirectories +// (i.e. "golang.org/x/"). The user is meant to accept completion suggestions +// until they reach a complete import path. +func (c *completer) populateImportCompletions(ctx context.Context, searchImport *ast.ImportSpec) error { + c.surrounding = &Selection{ + content: searchImport.Path.Value, + cursor: c.pos, + mappedRange: newMappedRange(c.snapshot.FileSet(), c.mapper, searchImport.Path.Pos(), searchImport.Path.End()), + } + + seenImports := make(map[string]struct{}) + for _, importSpec := range c.file.Imports { + if importSpec.Path.Value == searchImport.Path.Value { + continue + } + importPath, err := strconv.Unquote(importSpec.Path.Value) + if err != nil { + return err + } + seenImports[importPath] = struct{}{} + } + + prefixEnd := c.pos - searchImport.Path.ValuePos + // Extract the text between the quotes (if any) in an import spec. + // prefix is the part of import path before the cursor. + prefix := strings.Trim(searchImport.Path.Value[:prefixEnd], `"`) + + // The number of directories in the import path gives us the depth at + // which to search. + depth := len(strings.Split(prefix, "/")) - 1 + + var mu sync.Mutex // guard c.items locally, since searchImports is called in parallel + seen := make(map[string]struct{}) + searchImports := func(pkg imports.ImportFix) { + path := pkg.StmtInfo.ImportPath + if _, ok := seenImports[path]; ok { + return + } + + // Any package path containing fewer directories than the search + // prefix is not a match. + pkgDirList := strings.Split(path, "/") + if len(pkgDirList) < depth+1 { + return + } + pkgToConsider := strings.Join(pkgDirList[:depth+1], "/") + + score := float64(pkg.Relevance) + if len(pkgDirList)-1 == depth { + score *= highScore + } else { + // For incomplete package paths, add a terminal slash to indicate that the + // user should keep triggering completions. + pkgToConsider += "/" + } + + if _, ok := seen[pkgToConsider]; ok { + return + } + seen[pkgToConsider] = struct{}{} + + mu.Lock() + defer mu.Unlock() + + obj := types.NewPkgName(0, nil, pkg.IdentName, types.NewPackage(pkgToConsider, pkg.IdentName)) + // Running goimports logic in completions is expensive, and the + // (*completer).found method imposes a 100ms budget. Work-around this + // by adding to c.items directly. + cand := candidate{obj: obj, name: `"` + pkgToConsider + `"`, score: score} + if item, err := c.item(ctx, cand); err == nil { + c.items = append(c.items, item) + } + } + + return c.snapshot.View().RunProcessEnvFunc(ctx, func(opts *imports.Options) error { + return imports.GetImportPaths(ctx, searchImports, prefix, c.filename, c.pkg.GetTypes().Name(), opts.Env) + }) +} + +// populateCommentCompletions yields completions for comments preceding or in declarations. func (c *completer) populateCommentCompletions(ctx context.Context, comment *ast.CommentGroup) { // If the completion was triggered by a period, ignore it. These types of // completions will not be useful in comments. diff --git a/internal/lsp/testdata/lsp/primarymod/importedcomplit/imported_complit.go b/internal/lsp/testdata/lsp/primarymod/importedcomplit/imported_complit.go deleted file mode 100644 index 99424ec945..0000000000 --- a/internal/lsp/testdata/lsp/primarymod/importedcomplit/imported_complit.go +++ /dev/null @@ -1,26 +0,0 @@ -package importedcomplit - -import ( - "golang.org/x/tools/internal/lsp/foo" -) - -func _() { - var V int //@item(icVVar, "V", "int", "var") - _ = foo.StructFoo{V} //@complete("}", Value, icVVar) -} - -func _() { - var ( - aa string //@item(icAAVar, "aa", "string", "var") - ab int //@item(icABVar, "ab", "int", "var") - ) - - _ = foo.StructFoo{a} //@complete("}", abVar, aaVar) - - var s struct { - AA string //@item(icFieldAA, "AA", "string", "field") - AB int //@item(icFieldAB, "AB", "int", "field") - } - - _ = foo.StructFoo{s.} //@complete("}", icFieldAB, icFieldAA) -} diff --git a/internal/lsp/testdata/lsp/primarymod/importedcomplit/imported_complit.go.in b/internal/lsp/testdata/lsp/primarymod/importedcomplit/imported_complit.go.in new file mode 100644 index 0000000000..dddf20df7e --- /dev/null +++ b/internal/lsp/testdata/lsp/primarymod/importedcomplit/imported_complit.go.in @@ -0,0 +1,41 @@ +package importedcomplit + +import ( + "golang.org/x/tools/internal/lsp/foo" + + // import completions + "fm" //@complete("\" //", fmtImport) + "go/pars" //@complete("\" //", parserImport) + "golang.org/x/tools/internal/lsp/signa" //@complete("na\" //", signatureImport) + "golang.org/x/too" //@complete("\" //", toolsImport) + "crypto/elli" //@complete("\" //", cryptoImport) + "golang.org/x/tools/internal/lsp/sign" //@complete("\" //", signatureImport) + namedParser "go/pars" //@complete("\" //", parserImport) +) + +func _() { + var V int //@item(icVVar, "V", "int", "var") + _ = foo.StructFoo{V} //@complete("}", Value, icVVar) +} + +func _() { + var ( + aa string //@item(icAAVar, "aa", "string", "var") + ab int //@item(icABVar, "ab", "int", "var") + ) + + _ = foo.StructFoo{a} //@complete("}", abVar, aaVar) + + var s struct { + AA string //@item(icFieldAA, "AA", "string", "field") + AB int //@item(icFieldAB, "AB", "int", "field") + } + + _ = foo.StructFoo{s.} //@complete("}", icFieldAB, icFieldAA) +} + +/* "fmt" */ //@item(fmtImport, "\"fmt\"", "\"fmt\"", "package") +/* "go/parser" */ //@item(parserImport, "\"go/parser\"", "\"go/parser\"", "package") +/* "golang.org/x/tools/internal/lsp/signature" */ //@item(signatureImport, "\"golang.org/x/tools/internal/lsp/signature\"", "\"golang.org/x/tools/internal/lsp/signature\"", "package") +/* "golang.org/x/tools/" */ //@item(toolsImport, "\"golang.org/x/tools/\"", "\"golang.org/x/tools/\"", "package") +/* "crypto/elliptic" */ //@item(cryptoImport, "\"crypto/elliptic\"", "\"crypto/elliptic\"", "package") diff --git a/internal/lsp/testdata/lsp/summary.txt.golden b/internal/lsp/testdata/lsp/summary.txt.golden index 36120de60f..416c54934b 100644 --- a/internal/lsp/testdata/lsp/summary.txt.golden +++ b/internal/lsp/testdata/lsp/summary.txt.golden @@ -1,7 +1,7 @@ -- summary -- CallHierarchyCount = 1 CodeLensCount = 5 -CompletionsCount = 247 +CompletionsCount = 254 CompletionSnippetCount = 85 UnimportedCompletionsCount = 6 DeepCompletionsCount = 5