gopls/internal/lsp/source: simplify legacy 'references' func
The function used to accept three booleans, the middle of which
was always false. By simplifying under that invariant, the
other two booleans fall away, along with the 'implementations'
query that it used to make.
Small steps...
Change-Id: Ib520661b822afd85d6f0311253779a6202afd33d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/463687
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/lsp/source/implementation.go b/gopls/internal/lsp/source/implementation.go
index 5052026..70bb0bd 100644
--- a/gopls/internal/lsp/source/implementation.go
+++ b/gopls/internal/lsp/source/implementation.go
@@ -14,145 +14,12 @@
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/safetoken"
- "golang.org/x/tools/gopls/internal/lsp/source/methodsets"
"golang.org/x/tools/gopls/internal/span"
"golang.org/x/tools/internal/event"
)
var ErrNotAType = errors.New("not a type name or method")
-// implementations returns the concrete implementations of the specified
-// interface, or the interfaces implemented by the specified concrete type.
-// It populates only the definition-related fields of qualifiedObject.
-// (Arguably it should return a smaller data type.)
-//
-// This is the legacy implementation using the type-check-the-world
-// strategy. It is still needed by the 'references' command for now,
-// but we are moving one step at a time. See implementation2.go for
-// the incremental algorithm used by the 'implementations' command.
-// TODO(adonovan): eliminate this implementation.
-func implementations(ctx context.Context, s Snapshot, f FileHandle, pp protocol.Position) ([]qualifiedObject, error) {
- // Find all named types, even local types
- // (which can have methods due to promotion).
- var (
- allNamed []*types.Named
- pkgs = make(map[*types.Package]Package)
- )
- metas, err := s.AllMetadata(ctx)
- if err != nil {
- return nil, err
- }
- ids := make([]PackageID, len(metas))
- for i, m := range metas {
- ids[i] = m.ID
- }
- packages, err := s.TypeCheck(ctx, TypecheckFull, ids...)
- if err != nil {
- return nil, err
- }
- for _, pkg := range packages {
- pkgs[pkg.GetTypes()] = pkg
- for _, obj := range pkg.GetTypesInfo().Defs {
- obj, ok := obj.(*types.TypeName)
- // We ignore aliases 'type M = N' to avoid duplicate reporting
- // of the Named type N.
- if !ok || obj.IsAlias() {
- continue
- }
- if named, ok := obj.Type().(*types.Named); ok {
- allNamed = append(allNamed, named)
- }
- }
- }
-
- qos, err := qualifiedObjsAtProtocolPos(ctx, s, f.URI(), pp)
- if err != nil {
- return nil, err
- }
- var (
- impls []qualifiedObject
- seen = make(map[token.Position]bool)
- )
- for _, qo := range qos {
- // Ascertain the query identifier (type or method).
- var (
- queryType types.Type
- queryMethod *types.Func
- )
- switch obj := qo.obj.(type) {
- case *types.Func:
- queryMethod = obj
- if recv := obj.Type().(*types.Signature).Recv(); recv != nil {
- queryType = methodsets.EnsurePointer(recv.Type())
- }
- case *types.TypeName:
- queryType = methodsets.EnsurePointer(obj.Type())
- }
-
- if queryType == nil {
- return nil, ErrNotAType
- }
-
- if types.NewMethodSet(queryType).Len() == 0 {
- return nil, nil
- }
-
- // Find all the named types that match our query.
- for _, named := range allNamed {
- var (
- candObj types.Object = named.Obj()
- candType = methodsets.EnsurePointer(named)
- )
-
- if !concreteImplementsIntf(candType, queryType) {
- continue
- }
-
- ms := types.NewMethodSet(candType)
- if ms.Len() == 0 {
- // Skip empty interfaces.
- continue
- }
-
- // If client queried a method, look up corresponding candType method.
- if queryMethod != nil {
- sel := ms.Lookup(queryMethod.Pkg(), queryMethod.Name())
- if sel == nil {
- continue
- }
- candObj = sel.Obj()
- }
-
- pos := safetoken.StartPosition(s.FileSet(), candObj.Pos())
- if candObj == queryMethod || seen[pos] {
- continue
- }
-
- pkg := pkgs[candObj.Pkg()] // may be nil (e.g. error)
-
- // TODO(adonovan): the logic below assumes there is only one
- // predeclared (pkg=nil) object of interest, the error type.
- // That could change in a future version of Go.
-
- var posn token.Position
- if pkg != nil {
- posn = safetoken.StartPosition(pkg.FileSet(), candObj.Pos())
- }
- if seen[posn] {
- continue
- }
- seen[posn] = true
-
- impls = append(impls, qualifiedObject{
- obj: candObj,
- pkg: pkg,
- })
- }
- }
-
- return impls, nil
-}
-
// concreteImplementsIntf returns true if a is an interface type implemented by
// concrete type b, or vice versa.
func concreteImplementsIntf(a, b types.Type) bool {
diff --git a/gopls/internal/lsp/source/references.go b/gopls/internal/lsp/source/references.go
index 5f3fdbb..36f7c3a 100644
--- a/gopls/internal/lsp/source/references.go
+++ b/gopls/internal/lsp/source/references.go
@@ -6,7 +6,6 @@
import (
"context"
- "errors"
"fmt"
"go/ast"
"go/token"
@@ -20,7 +19,6 @@
// ReferenceInfo holds information about reference to an identifier in Go source.
type ReferenceInfo struct {
- Name string
MappedRange protocol.MappedRange
ident *ast.Ident
obj types.Object
@@ -49,7 +47,7 @@
// (Arguably it should accept a smaller data type.)
//
// This implementation serves Server.rename. TODO(adonovan): obviate it.
-func references(ctx context.Context, snapshot Snapshot, qos []qualifiedObject, includeDeclaration, includeInterfaceRefs, includeEmbeddedRefs bool) ([]*ReferenceInfo, error) {
+func references(ctx context.Context, snapshot Snapshot, qos []qualifiedObject) ([]*ReferenceInfo, error) {
var (
references []*ReferenceInfo
seen = make(map[positionKey]bool)
@@ -71,16 +69,13 @@
return nil, err
}
// Make sure declaration is the first item in the response.
- if includeDeclaration {
- references = append(references, &ReferenceInfo{
- MappedRange: declIdent.MappedRange,
- Name: qos[0].obj.Name(),
- ident: declIdent.ident,
- obj: qos[0].obj,
- pkg: declIdent.pkg,
- isDeclaration: true,
- })
- }
+ references = append(references, &ReferenceInfo{
+ MappedRange: declIdent.MappedRange,
+ ident: declIdent.ident,
+ obj: qos[0].obj,
+ pkg: declIdent.pkg,
+ isDeclaration: true,
+ })
for _, qo := range qos {
var searchPkgs []Package
@@ -120,9 +115,6 @@
// If ident is not a use of qo.obj, skip it, with one exception:
// uses of an embedded field can be considered references of the
// embedded type name
- if !includeEmbeddedRefs {
- continue
- }
v, ok := obj.(*types.Var)
if !ok || !v.Embedded() {
continue
@@ -151,7 +143,6 @@
return nil, err
}
references = append(references, &ReferenceInfo{
- Name: ident.Name,
ident: ident,
pkg: pkg,
obj: obj,
@@ -161,25 +152,6 @@
}
}
- // When searching on type name, don't include interface references -- they
- // would be things like all references to Stringer for any type that
- // happened to have a String method.
- _, isType := declIdent.Declaration.obj.(*types.TypeName)
- if includeInterfaceRefs && !isType {
- // TODO(adonovan): opt: don't go back into the position domain:
- // we have complete type information already.
- declRange := declIdent.MappedRange.Range()
- fh, err := snapshot.GetFile(ctx, declIdent.MappedRange.URI())
- if err != nil {
- return nil, err
- }
- interfaceRefs, err := interfaceReferences(ctx, snapshot, fh, declRange.Start)
- if err != nil {
- return nil, err
- }
- references = append(references, interfaceRefs...)
- }
-
return references, nil
}
@@ -189,27 +161,3 @@
func equalOrigin(obj1, obj2 types.Object) bool {
return obj1.Pkg() == obj2.Pkg() && obj1.Pos() == obj2.Pos() && obj1.Name() == obj2.Name()
}
-
-// interfaceReferences returns the references to the interfaces implemented by
-// the type or method at the given position.
-func interfaceReferences(ctx context.Context, s Snapshot, f FileHandle, pp protocol.Position) ([]*ReferenceInfo, error) {
- implementations, err := implementations(ctx, s, f, pp)
- if err != nil {
- if errors.Is(err, ErrNotAType) {
- return nil, nil
- }
- return nil, err
- }
-
- // Make a separate call to references() for each element
- // since it treats the first qualifiedObject as a definition.
- var refs []*ReferenceInfo
- for _, impl := range implementations {
- implRefs, err := references(ctx, s, []qualifiedObject{impl}, false, false, false)
- if err != nil {
- return nil, err
- }
- refs = append(refs, implRefs...)
- }
- return refs, nil
-}
diff --git a/gopls/internal/lsp/source/rename.go b/gopls/internal/lsp/source/rename.go
index 385dff4..ca3c694 100644
--- a/gopls/internal/lsp/source/rename.go
+++ b/gopls/internal/lsp/source/rename.go
@@ -548,6 +548,17 @@
try++
localName = fmt.Sprintf("%s%d", newName, try)
}
+
+ // renameObj detects various conflicts, including:
+ // - new name conflicts with a package-level decl in this file;
+ // - new name hides a package-level decl in another file that
+ // is actually referenced in this file;
+ // - new name hides a built-in that is actually referenced
+ // in this file;
+ // - a reference in this file to the old package name would
+ // become shadowed by an intervening declaration that
+ // uses the new name.
+ // It returns the edits if no conflict was detected.
changes, err := renameObj(ctx, snapshot, localName, qos)
if err != nil {
return err
@@ -587,7 +598,7 @@
if !isValidIdentifier(newName) {
return nil, fmt.Errorf("invalid identifier to rename: %q", newName)
}
- refs, err := references(ctx, s, qos, true, false, true)
+ refs, err := references(ctx, s, qos)
if err != nil {
return nil, err
}
