1
0
mirror of https://github.com/golang/go synced 2024-11-05 11:46:12 -07:00

internal/lsp: refactor references/rename/implementations

As part of investigating golang/go#38100, I noticed a few things that I
wanted to clean up. Mostly, for renames, we were calling
qualifiedObjAtProtocolPos twice, so I factored out a shared helper
function. I also added an error return for builtins so that callers
don't have to check.

Change-Id: I28c75c801cbec1611736af931cfa72befd219201
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225777
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
This commit is contained in:
Rebecca Stambler 2020-03-26 23:25:15 -04:00
parent 8db92c5f61
commit eca45d481d
5 changed files with 43 additions and 54 deletions

View File

@ -20,19 +20,16 @@ func (s *Server) references(ctx context.Context, params *protocol.ReferenceParam
if err != nil {
return nil, err
}
var locations []protocol.Location
for _, ref := range references {
refRange, err := ref.Range()
if err != nil {
return nil, err
}
locations = append(locations, protocol.Location{
URI: protocol.URIFromSpanURI(ref.URI()),
Range: refRange,
})
}
return locations, nil
}

View File

@ -6,6 +6,7 @@ package source
import (
"context"
"errors"
"fmt"
"go/ast"
"go/token"
@ -13,7 +14,6 @@ import (
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/telemetry/event"
errors "golang.org/x/xerrors"
)
func Implementation(ctx context.Context, s Snapshot, f FileHandle, pp protocol.Position) ([]protocol.Location, error) {
@ -24,7 +24,6 @@ func Implementation(ctx context.Context, s Snapshot, f FileHandle, pp protocol.P
if err != nil {
return nil, err
}
var locations []protocol.Location
for _, impl := range impls {
if impl.pkg == nil || len(impl.pkg.CompiledGoFiles()) == 0 {
@ -61,7 +60,6 @@ func implementations(ctx context.Context, s Snapshot, f FileHandle, pp protocol.
if err != nil {
return nil, err
}
for _, qo := range qos {
var (
queryType types.Type
@ -201,6 +199,8 @@ type qualifiedObject struct {
sourcePkg Package
}
var errBuiltin = errors.New("builtin object")
// qualifiedObjsAtProtocolPos returns info for all the type.Objects
// referenced at the given position. An object will be returned for
// every package that the file belongs to.
@ -212,11 +212,11 @@ func qualifiedObjsAtProtocolPos(ctx context.Context, s Snapshot, fh FileHandle,
// Check all the packages that the file belongs to.
var qualifiedObjs []qualifiedObject
for _, ph := range phs {
pkg, err := ph.Check(ctx)
searchpkg, err := ph.Check(ctx)
if err != nil {
return nil, err
}
astFile, pos, err := getASTFile(pkg, fh, pp)
astFile, pos, err := getASTFile(searchpkg, fh, pp)
if err != nil {
return nil, err
}
@ -230,10 +230,10 @@ func qualifiedObjsAtProtocolPos(ctx context.Context, s Snapshot, fh FileHandle,
// If leaf represents an implicit type switch object or the type
// switch "assign" variable, expand to all of the type switch's
// implicit objects.
if implicits := typeSwitchImplicits(pkg, path); len(implicits) > 0 {
if implicits := typeSwitchImplicits(searchpkg, path); len(implicits) > 0 {
objs = append(objs, implicits...)
} else {
obj := pkg.GetTypesInfo().ObjectOf(leaf)
obj := searchpkg.GetTypesInfo().ObjectOf(leaf)
if obj == nil {
return nil, fmt.Errorf("no object for %q", leaf.Name)
}
@ -241,22 +241,30 @@ func qualifiedObjsAtProtocolPos(ctx context.Context, s Snapshot, fh FileHandle,
}
case *ast.ImportSpec:
// Look up the implicit *types.PkgName.
obj := pkg.GetTypesInfo().Implicits[leaf]
obj := searchpkg.GetTypesInfo().Implicits[leaf]
if obj == nil {
return nil, fmt.Errorf("no object for import %q", importPath(leaf))
}
objs = append(objs, obj)
}
pkgs := make(map[*types.Package]Package)
pkgs[pkg.GetTypes()] = pkg
for _, imp := range pkg.Imports() {
pkgs[searchpkg.GetTypes()] = searchpkg
for _, imp := range searchpkg.Imports() {
pkgs[imp.GetTypes()] = imp
}
for _, obj := range objs {
if obj.Parent() == types.Universe {
return nil, fmt.Errorf("%w %q", errBuiltin, obj.Name())
}
pkg, ok := pkgs[obj.Pkg()]
if !ok {
event.Error(ctx, fmt.Sprintf("no package for obj %s: %v", obj, obj.Pkg()), err)
continue
}
qualifiedObjs = append(qualifiedObjs, qualifiedObject{
obj: obj,
pkg: pkgs[obj.Pkg()],
sourcePkg: pkg,
pkg: pkg,
sourcePkg: searchpkg,
node: path[0],
})
}
@ -264,7 +272,7 @@ func qualifiedObjsAtProtocolPos(ctx context.Context, s Snapshot, fh FileHandle,
// Return an error if no objects were found since callers will assume that
// the slice has at least 1 element.
if len(qualifiedObjs) == 0 {
return nil, errors.Errorf("no object found")
return nil, fmt.Errorf("no object found")
}
return qualifiedObjs, nil
}

View File

@ -6,6 +6,7 @@ package source
import (
"context"
"errors"
"go/ast"
"go/token"
"go/types"
@ -31,14 +32,19 @@ func References(ctx context.Context, s Snapshot, f FileHandle, pp protocol.Posit
defer done()
qualifiedObjs, err := qualifiedObjsAtProtocolPos(ctx, s, f, pp)
// Don't return references for builtin types.
if errors.Is(err, errBuiltin) {
return nil, nil
}
if err != nil {
return nil, err
}
// Don't return references for builtin types.
if qualifiedObjs[0].obj.Parent() == types.Universe {
return nil, nil
return references(ctx, s, qualifiedObjs, includeDeclaration)
}
// references is a helper function used by both References and Rename,
// to avoid recomputing qualifiedObjsAtProtocolPos.
func references(ctx context.Context, s Snapshot, qos []qualifiedObject, includeDeclaration bool) ([]*ReferenceInfo, error) {
var (
references []*ReferenceInfo
seen = make(map[token.Position]bool)
@ -47,23 +53,21 @@ func References(ctx context.Context, s Snapshot, f FileHandle, pp protocol.Posit
// Make sure declaration is the first item in the response.
if includeDeclaration {
rng, err := objToMappedRange(s.View(), qualifiedObjs[0].pkg, qualifiedObjs[0].obj)
rng, err := objToMappedRange(s.View(), qos[0].pkg, qos[0].obj)
if err != nil {
return nil, err
}
ident, _ := qualifiedObjs[0].node.(*ast.Ident)
ident, _ := qos[0].node.(*ast.Ident)
references = append(references, &ReferenceInfo{
mappedRange: rng,
Name: qualifiedObjs[0].obj.Name(),
Name: qos[0].obj.Name(),
ident: ident,
obj: qualifiedObjs[0].obj,
pkg: qualifiedObjs[0].pkg,
obj: qos[0].obj,
pkg: qos[0].pkg,
isDeclaration: true,
})
}
for _, qo := range qualifiedObjs {
for _, qo := range qos {
var searchPkgs []Package
// Only search dependents if the object is exported.
@ -72,7 +76,6 @@ func References(ctx context.Context, s Snapshot, f FileHandle, pp protocol.Posit
if err != nil {
return nil, err
}
for _, ph := range reverseDeps {
pkg, err := ph.Check(ctx)
if err != nil {
@ -81,22 +84,18 @@ func References(ctx context.Context, s Snapshot, f FileHandle, pp protocol.Posit
searchPkgs = append(searchPkgs, pkg)
}
}
// Add the package in which the identifier is declared.
searchPkgs = append(searchPkgs, qo.pkg)
for _, pkg := range searchPkgs {
for ident, obj := range pkg.GetTypesInfo().Uses {
if obj != qo.obj {
continue
}
pos := fset.Position(ident.Pos())
if seen[pos] {
continue
}
seen[pos] = true
rng, err := posToMappedRange(s.View(), pkg, ident.Pos(), ident.End())
if err != nil {
return nil, err
@ -111,6 +110,5 @@ func References(ctx context.Context, s Snapshot, f FileHandle, pp protocol.Posit
}
}
}
return references, nil
}

View File

@ -49,30 +49,22 @@ func PrepareRename(ctx context.Context, s Snapshot, f FileHandle, pp protocol.Po
if err != nil {
return nil, err
}
// Do not rename builtin identifiers.
if qos[0].obj.Parent() == types.Universe {
return nil, errors.Errorf("cannot rename builtin %q", qos[0].obj.Name())
}
mr, err := posToMappedRange(s.View(), qos[0].sourcePkg, qos[0].node.Pos(), qos[0].node.End())
node, obj, pkg := qos[0].node, qos[0].obj, qos[0].sourcePkg
mr, err := posToMappedRange(s.View(), pkg, node.Pos(), node.End())
if err != nil {
return nil, err
}
rng, err := mr.Range()
if err != nil {
return nil, err
}
if _, isImport := qos[0].node.(*ast.ImportSpec); isImport {
if _, isImport := node.(*ast.ImportSpec); isImport {
// We're not really renaming the import path.
rng.End = rng.Start
}
return &PrepareItem{
Range: rng,
Text: qos[0].obj.Name(),
Text: obj.Name(),
}, nil
}
@ -95,19 +87,13 @@ func Rename(ctx context.Context, s Snapshot, f FileHandle, pp protocol.Position,
if !isValidIdentifier(newName) {
return nil, errors.Errorf("invalid identifier to rename: %q", newName)
}
// Do not rename builtin identifiers.
if obj.Parent() == types.Universe {
return nil, errors.Errorf("cannot rename builtin %q", obj.Name())
}
if pkg == nil || pkg.IsIllTyped() {
return nil, errors.Errorf("package for %s is ill typed", f.Identity().URI)
}
refs, err := References(ctx, s, f, pp, true)
refs, err := references(ctx, s, qos, true)
if err != nil {
return nil, err
}
r := renamer{
ctx: ctx,
fset: s.View().Session().Cache().FileSet(),

View File

@ -10,4 +10,4 @@ func _() {
}
-- uint-rename --
cannot rename builtin "int"
builtin object "int"