qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-18 18:54:42 -07:00
Code Releases Activity

internal/lsp/source: attach Package to completions when available

Browse Source 
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>
This commit is contained in:
Heschi Kreinick 2019-11-05 14:53:55 -05:00
parent dc038396d1
commit 818555187f
6 changed files with 75 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
internal/lsp/cache/pkg.go vendored
View File

@ -147,5 +147,5 @@ func findFileInPackage(ctx context.Context, uri span.URI, pkg source.Package) (s
}
}
}
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())
}

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

@ -14,7 +14,6 @@ import (
"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 @@ type compLitInfo struct {
maybeInFieldName bool
}
type importInfo struct {
importPath string
name string
pkg Package
}
type methodSetKey struct {
typ types.Type
addressable bool
@ -284,7 +289,7 @@ func (c *completer) getSurrounding() *Selection {
// 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 @@ type candidate struct {
// 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 @@ func (c *completer) selector(sel *ast.SelectorExpr) error {
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 @@ func (c *completer) lexical() error {
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 @@ func (c *completer) lexical() error {
// 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,
})
}
}
}

18
internal/lsp/source/completion_format.go
View File

@ -13,7 +13,6 @@ import (
"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 @@ func (c *completer) item(cand candidate) (CompletionItem, error) {
if detail != "" {
detail += " "
}
detail += fmt.Sprintf("(from %q)", cand.imp.ImportPath)
detail += fmt.Sprintf("(from %q)", cand.imp.importPath)
}
}
@ -135,7 +134,14 @@ func (c *completer) item(cand candidate) (CompletionItem, error) {
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 @@ func (c *completer) item(cand candidate) (CompletionItem, error) {
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 @@ func (c *completer) item(cand candidate) (CompletionItem, error) {
}
// 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
}

3
internal/lsp/source/completion_literal.go
View File

@ -11,7 +11,6 @@ import (
"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 @@ import (
// 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
}

4
internal/lsp/source/deep_completion.go
View File

@ -8,8 +8,6 @@ import (
"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 @@ func (c *completer) shouldPrune() bool {
// 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
}

68
internal/lsp/source/imports_test.go
View File

@ -35,11 +35,11 @@ type test struct {
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 @@ import (
"os"
)
`,
want: []importInfo{
importInfo{
want: []imp{
{
name: "",
path: "os",
},
@ -67,8 +67,8 @@ import (
pkg: "os",
in: `package main
`,
want: []importInfo{
importInfo{
want: []imp{
{
name: "",
path: "os",
},
@ -78,8 +78,8 @@ import (
name: "package statement no new line",
pkg: "os",
in: `package main`,
want: []importInfo{
importInfo{
want: []imp{
{
name: "",
path: "os",
},
@ -92,8 +92,8 @@ import (
in: `// Here is a comment before
package main
`,
want: []importInfo{
importInfo{
want: []imp{
{
name: "",
path: "os",
},
@ -104,8 +104,8 @@ package main
pkg: "os",
in: `package main // Here is a comment after
`,
want: []importInfo{
importInfo{
want: []imp{
{
name: "",
path: "os",
},
@ -116,8 +116,8 @@ package main
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 @@ package main // Here is a comment after`,
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 @@ further down*/`,
import "C"
`,
want: []importInfo{
importInfo{
want: []imp{
{
name: "",
path: "os",
},
importInfo{
{
name: "",
path: "C",
},
@ -161,12 +161,12 @@ import "C"
import "io"
`,
want: []importInfo{
importInfo{
want: []imp{
{
name: "",
path: "os",
},
importInfo{
{
name: "",
path: "io",
},
@ -179,12 +179,12 @@ import "io"
import "io" // A comment
`,
want: []importInfo{
importInfo{
want: []imp{
{
name: "",
path: "os",
},
importInfo{
{
name: "",
path: "io",
},
@ -199,12 +199,12 @@ import "io" /* A comment
that
extends */
`,
want: []importInfo{
importInfo{
want: []imp{
{
name: "",
path: "os",
},
importInfo{
{
name: "",
path: "io",
},
@ -216,8 +216,8 @@ extends */
pkg: "os",
in: `package main
`,
want: []importInfo{
importInfo{
want: []imp{
{
name: "o",
path: "os",
},
@ -280,12 +280,12 @@ func TestDoubleAddNamedImport(t *testing.T) {
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 @@ func TestDoubleAddNamedImport(t *testing.T) {
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