internal/lsp/source: attach Package to completions when available
Unimported completions now try to pull Packages from everywhere, not
just the transitive dependencies of the current package. That confused
the import formatting code, which only looked at deps. Pass the Package
along with the import suggestion, and use it when it's present.
Also change some error messages to be different for diagnostic purposes.
Fixes golang/go#35359.
Change-Id: Ia8ca923e46723e855ddd2da7611e6eb13c02bb4f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205501
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
(cherry picked from commit 818555187f961fc0f6b11cdb7163349e41ba75f5)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205658
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go
index 613f809..bf398a2 100644
--- a/internal/lsp/cache/pkg.go
+++ b/internal/lsp/cache/pkg.go
@@ -147,5 +147,5 @@
}
}
}
- return nil, nil, errors.Errorf("no file for %s", uri)
+ return nil, nil, errors.Errorf("no file for %s in package %s", uri, pkg.ID())
}
diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index 70be745..2b79350 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -14,7 +14,6 @@
"time"
"golang.org/x/tools/go/ast/astutil"
- "golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/lsp/fuzzy"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/snippet"
@@ -221,6 +220,12 @@
maybeInFieldName bool
}
+type importInfo struct {
+ importPath string
+ name string
+ pkg Package
+}
+
type methodSetKey struct {
typ types.Type
addressable bool
@@ -284,7 +289,7 @@
// found adds a candidate completion. We will also search through the object's
// members for more candidates.
-func (c *completer) found(obj types.Object, score float64, imp *imports.ImportInfo) {
+func (c *completer) found(obj types.Object, score float64, imp *importInfo) {
if obj.Pkg() != nil && obj.Pkg() != c.pkg.GetTypes() && !obj.Exported() {
// obj is not accessible because it lives in another package and is not
// exported. Don't treat it as a completion candidate.
@@ -370,7 +375,7 @@
// imp is the import that needs to be added to this package in order
// for this candidate to be valid. nil if no import needed.
- imp *imports.ImportInfo
+ imp *importInfo
}
// ErrIsDefinition is an error that informs the user they got no
@@ -591,28 +596,35 @@
for _, pkgExport := range pkgExports {
// 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(), &pkgExport.Fix.StmtInfo)
+ c.packageMembers(knownPkg.GetTypes(), &importInfo{
+ importPath: pkgExport.Fix.StmtInfo.ImportPath,
+ name: pkgExport.Fix.StmtInfo.Name,
+ pkg: knownPkg,
+ })
continue
}
// Otherwise, continue with untyped proposals.
pkg := types.NewPackage(pkgExport.Fix.StmtInfo.ImportPath, pkgExport.Fix.IdentName)
for _, export := range pkgExport.Exports {
- c.found(types.NewVar(0, pkg, export, nil), 0.07, &pkgExport.Fix.StmtInfo)
+ c.found(types.NewVar(0, pkg, export, nil), 0.07, &importInfo{
+ importPath: pkgExport.Fix.StmtInfo.ImportPath,
+ name: pkgExport.Fix.StmtInfo.Name,
+ })
}
}
}
return nil
}
-func (c *completer) packageMembers(pkg *types.Package, imp *imports.ImportInfo) {
+func (c *completer) packageMembers(pkg *types.Package, imp *importInfo) {
scope := pkg.Scope()
for _, name := range scope.Names() {
c.found(scope.Lookup(name), stdScore, imp)
}
}
-func (c *completer) methodsAndFields(typ types.Type, addressable bool, imp *imports.ImportInfo) error {
+func (c *completer) methodsAndFields(typ types.Type, addressable bool, imp *importInfo) error {
mset := c.methodSetCache[methodSetKey{typ, addressable}]
if mset == nil {
if addressable && !types.IsInterface(typ) && !isPointer(typ) {
@@ -716,8 +728,9 @@
if _, ok := seen[pkg.Name()]; !ok && pkg != c.pkg.GetTypes() && !alreadyImports(c.file, pkg.Path()) {
seen[pkg.Name()] = struct{}{}
obj := types.NewPkgName(0, nil, pkg.Name(), pkg)
- c.found(obj, stdScore, &imports.ImportInfo{
- ImportPath: pkg.Path(),
+ c.found(obj, stdScore, &importInfo{
+ importPath: pkg.Path(),
+ name: pkg.Name(),
})
}
}
@@ -739,7 +752,10 @@
// 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(obj, score, &pkg.StmtInfo)
+ c.found(obj, score, &importInfo{
+ importPath: pkg.StmtInfo.ImportPath,
+ name: pkg.StmtInfo.Name,
+ })
}
}
}
diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go
index 000e664..05b36b9 100644
--- a/internal/lsp/source/completion_format.go
+++ b/internal/lsp/source/completion_format.go
@@ -13,7 +13,6 @@
"go/types"
"strings"
- "golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/snippet"
"golang.org/x/tools/internal/span"
@@ -108,7 +107,7 @@
if detail != "" {
detail += " "
}
- detail += fmt.Sprintf("(from %q)", cand.imp.ImportPath)
+ detail += fmt.Sprintf("(from %q)", cand.imp.importPath)
}
}
@@ -135,7 +134,14 @@
return item, nil
}
uri := span.FileURI(pos.Filename)
- ph, pkg, err := c.view.FindFileInPackage(c.ctx, uri, c.pkg)
+
+ // Find the source file of the candidate, starting from a package
+ // that should have it in its dependencies.
+ searchPkg := c.pkg
+ if cand.imp != nil && cand.imp.pkg != nil {
+ searchPkg = cand.imp.pkg
+ }
+ ph, pkg, err := c.view.FindFileInPackage(c.ctx, uri, searchPkg)
if err != nil {
return CompletionItem{}, err
}
@@ -144,7 +150,7 @@
return CompletionItem{}, err
}
if !(file.Pos() <= obj.Pos() && obj.Pos() <= file.End()) {
- return CompletionItem{}, errors.Errorf("no file for %s", obj.Name())
+ return CompletionItem{}, errors.Errorf("no file for completion object %s", obj.Name())
}
ident, err := findIdentifier(c.ctx, c.snapshot, pkg, file, obj.Pos())
if err != nil {
@@ -162,12 +168,12 @@
}
// importEdits produces the text edits necessary to add the given import to the current file.
-func (c *completer) importEdits(imp *imports.ImportInfo) ([]protocol.TextEdit, error) {
+func (c *completer) importEdits(imp *importInfo) ([]protocol.TextEdit, error) {
if imp == nil {
return nil, nil
}
- edit, err := addNamedImport(c.view.Session().Cache().FileSet(), c.file, imp.Name, imp.ImportPath)
+ edit, err := addNamedImport(c.view.Session().Cache().FileSet(), c.file, imp.name, imp.importPath)
if err != nil {
return nil, err
}
diff --git a/internal/lsp/source/completion_literal.go b/internal/lsp/source/completion_literal.go
index 110f9c6..b87f1ec 100644
--- a/internal/lsp/source/completion_literal.go
+++ b/internal/lsp/source/completion_literal.go
@@ -11,7 +11,6 @@
"strings"
"unicode"
- "golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/lsp/diff"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/snippet"
@@ -21,7 +20,7 @@
// literal generates composite literal, function literal, and make()
// completion items.
-func (c *completer) literal(literalType types.Type, imp *imports.ImportInfo) {
+func (c *completer) literal(literalType types.Type, imp *importInfo) {
if c.expectedType.objType == nil {
return
}
diff --git a/internal/lsp/source/deep_completion.go b/internal/lsp/source/deep_completion.go
index eddacac..8f6995e 100644
--- a/internal/lsp/source/deep_completion.go
+++ b/internal/lsp/source/deep_completion.go
@@ -8,8 +8,6 @@
"go/types"
"strings"
"time"
-
- "golang.org/x/tools/internal/imports"
)
// Limit deep completion results because in most cases there are too many
@@ -142,7 +140,7 @@
// deepSearch searches through obj's subordinate objects for more
// completion items.
-func (c *completer) deepSearch(obj types.Object, imp *imports.ImportInfo) {
+func (c *completer) deepSearch(obj types.Object, imp *importInfo) {
if c.deepState.maxDepth == 0 {
return
}
diff --git a/internal/lsp/source/imports_test.go b/internal/lsp/source/imports_test.go
index e10c082..0927c59 100644
--- a/internal/lsp/source/imports_test.go
+++ b/internal/lsp/source/imports_test.go
@@ -35,11 +35,11 @@
renamedPkg string
pkg string
in string
- want []importInfo
+ want []imp
unchanged bool // Expect added/deleted return value to be false.
}
-type importInfo struct {
+type imp struct {
name string
path string
}
@@ -54,8 +54,8 @@
"os"
)
`,
- want: []importInfo{
- importInfo{
+ want: []imp{
+ {
name: "",
path: "os",
},
@@ -67,8 +67,8 @@
pkg: "os",
in: `package main
`,
- want: []importInfo{
- importInfo{
+ want: []imp{
+ {
name: "",
path: "os",
},
@@ -78,8 +78,8 @@
name: "package statement no new line",
pkg: "os",
in: `package main`,
- want: []importInfo{
- importInfo{
+ want: []imp{
+ {
name: "",
path: "os",
},
@@ -92,8 +92,8 @@
in: `// Here is a comment before
package main
`,
- want: []importInfo{
- importInfo{
+ want: []imp{
+ {
name: "",
path: "os",
},
@@ -104,8 +104,8 @@
pkg: "os",
in: `package main // Here is a comment after
`,
- want: []importInfo{
- importInfo{
+ want: []imp{
+ {
name: "",
path: "os",
},
@@ -116,8 +116,8 @@
pkg: "os",
in: `// Here is a comment before
package main // Here is a comment after`,
- want: []importInfo{
- importInfo{
+ want: []imp{
+ {
name: "",
path: "os",
},
@@ -129,8 +129,8 @@
in: `package main /* This is a multiline comment
and it extends
further down*/`,
- want: []importInfo{
- importInfo{
+ want: []imp{
+ {
name: "",
path: "os",
},
@@ -143,12 +143,12 @@
import "C"
`,
- want: []importInfo{
- importInfo{
+ want: []imp{
+ {
name: "",
path: "os",
},
- importInfo{
+ {
name: "",
path: "C",
},
@@ -161,12 +161,12 @@
import "io"
`,
- want: []importInfo{
- importInfo{
+ want: []imp{
+ {
name: "",
path: "os",
},
- importInfo{
+ {
name: "",
path: "io",
},
@@ -179,12 +179,12 @@
import "io" // A comment
`,
- want: []importInfo{
- importInfo{
+ want: []imp{
+ {
name: "",
path: "os",
},
- importInfo{
+ {
name: "",
path: "io",
},
@@ -199,12 +199,12 @@
that
extends */
`,
- want: []importInfo{
- importInfo{
+ want: []imp{
+ {
name: "",
path: "os",
},
- importInfo{
+ {
name: "",
path: "io",
},
@@ -216,8 +216,8 @@
pkg: "os",
in: `package main
`,
- want: []importInfo{
- importInfo{
+ want: []imp{
+ {
name: "o",
path: "os",
},
@@ -280,12 +280,12 @@
got = applyEdits(got, edits)
gotFile = parse(t, name, got)
- want := []importInfo{
- importInfo{
+ want := []imp{
+ {
name: "o",
path: "os",
},
- importInfo{
+ {
name: "i",
path: "io",
},
@@ -293,7 +293,7 @@
compareImports(t, "", gotFile.Imports, want)
}
-func compareImports(t *testing.T, prefix string, got []*ast.ImportSpec, want []importInfo) {
+func compareImports(t *testing.T, prefix string, got []*ast.ImportSpec, want []imp) {
if len(got) != len(want) {
t.Errorf("%s\ngot %d imports\nwant %d", prefix, len(got), len(want))
return
