gopls/internal/lsp/source: switch call hierarchy to references v2
This change removes the last use of the old referencesV1 function.
(Its 'references' helper function still has one remaining use,
from Server.rename.)
The IncomingCalls operation re-parses the files referenced by
the 'references' operation, rather than requesting a type-checked
package.
Also, inline toProtocolIncomingCalls into sole caller.
Change-Id: I33fbb210d42b7ca1a70cfebc4061275a153c3537
Reviewed-on: https://go-review.googlesource.com/c/tools/+/459515
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/call_hierarchy.go b/gopls/internal/lsp/source/call_hierarchy.go
index efe5f9a..cefc539 100644
--- a/gopls/internal/lsp/source/call_hierarchy.go
+++ b/gopls/internal/lsp/source/call_hierarchy.go
@@ -15,7 +15,6 @@
"golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/gopls/internal/lsp/protocol"
- "golang.org/x/tools/gopls/internal/span"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/event/tag"
)
@@ -65,13 +64,7 @@
ctx, done := event.Start(ctx, "source.IncomingCalls")
defer done()
- // TODO(adonovan): switch to referencesV2 here once it supports methods.
- // This will require that we parse files containing
- // references instead of accessing refs[i].pkg.
- // (We could use pre-parser trimming, either a scanner-based
- // implementation such as https://go.dev/play/p/KUrObH1YkX8
- // (~31% speedup), or a byte-oriented implementation (2x speedup).
- refs, err := referencesV1(ctx, snapshot, fh, pos, false)
+ refs, err := referencesV2(ctx, snapshot, fh, pos, false)
if err != nil {
if errors.Is(err, ErrNoIdentFound) || errors.Is(err, errNoObjectFound) {
return nil, nil
@@ -79,23 +72,14 @@
return nil, err
}
- return toProtocolIncomingCalls(ctx, snapshot, refs)
-}
-
-// toProtocolIncomingCalls returns an array of protocol.CallHierarchyIncomingCall for ReferenceInfo's.
-// References inside same enclosure are assigned to the same enclosing function.
-func toProtocolIncomingCalls(ctx context.Context, snapshot Snapshot, refs []*ReferenceInfo) ([]protocol.CallHierarchyIncomingCall, error) {
- // an enclosing node could have multiple calls to a reference, we only show the enclosure
- // once in the result but highlight all calls using FromRanges (ranges at which the calls occur)
- var incomingCalls = map[protocol.Location]*protocol.CallHierarchyIncomingCall{}
+ // Group references by their enclosing function declaration.
+ incomingCalls := make(map[protocol.Location]*protocol.CallHierarchyIncomingCall)
for _, ref := range refs {
- refRange := ref.MappedRange.Range()
- callItem, err := enclosingNodeCallItem(snapshot, ref.pkg, ref.MappedRange.URI(), ref.ident.NamePos)
+ callItem, err := enclosingNodeCallItem(ctx, snapshot, ref.PkgPath, ref.Location)
if err != nil {
event.Error(ctx, "error getting enclosing node", err, tag.Method.Of(ref.Name))
continue
}
-
loc := protocol.Location{
URI: callItem.URI,
Range: callItem.Range,
@@ -105,9 +89,10 @@
call = &protocol.CallHierarchyIncomingCall{From: callItem}
incomingCalls[loc] = call
}
- call.FromRanges = append(call.FromRanges, refRange)
+ call.FromRanges = append(call.FromRanges, ref.Location.Range)
}
+ // Flatten the map of pointers into a slice of values.
incomingCallItems := make([]protocol.CallHierarchyIncomingCall, 0, len(incomingCalls))
for _, callItem := range incomingCalls {
incomingCallItems = append(incomingCallItems, *callItem)
@@ -115,18 +100,31 @@
return incomingCallItems, nil
}
-// enclosingNodeCallItem creates a CallHierarchyItem representing the function call at pos
-func enclosingNodeCallItem(snapshot Snapshot, pkg Package, uri span.URI, pos token.Pos) (protocol.CallHierarchyItem, error) {
- pgf, err := pkg.File(uri)
+// enclosingNodeCallItem creates a CallHierarchyItem representing the function call at loc.
+func enclosingNodeCallItem(ctx context.Context, snapshot Snapshot, pkgPath PackagePath, loc protocol.Location) (protocol.CallHierarchyItem, error) {
+ // Parse the file containing the reference.
+ fh, err := snapshot.GetFile(ctx, loc.URI.SpanURI())
+ if err != nil {
+ return protocol.CallHierarchyItem{}, err
+ }
+ // TODO(adonovan): opt: before parsing, trim the bodies of functions
+ // that don't contain the reference, using either a scanner-based
+ // implementation such as https://go.dev/play/p/KUrObH1YkX8
+ // (~31% speedup), or a byte-oriented implementation (2x speedup).
+ pgf, err := snapshot.ParseGo(ctx, fh, ParseFull)
+ if err != nil {
+ return protocol.CallHierarchyItem{}, err
+ }
+ srcRng, err := pgf.RangeToTokenRange(loc.Range)
if err != nil {
return protocol.CallHierarchyItem{}, err
}
+ // Find the enclosing function, if any, and the number of func literals in between.
var funcDecl *ast.FuncDecl
var funcLit *ast.FuncLit // innermost function literal
var litCount int
- // Find the enclosing function, if any, and the number of func literals in between.
- path, _ := astutil.PathEnclosingInterval(pgf.File, pos, pos)
+ path, _ := astutil.PathEnclosingInterval(pgf.File, srcRng.Start, srcRng.End)
outer:
for _, node := range path {
switch n := node.(type) {
@@ -168,8 +166,8 @@
Name: name,
Kind: kind,
Tags: []protocol.SymbolTag{},
- Detail: fmt.Sprintf("%s • %s", pkg.PkgPath(), filepath.Base(uri.Filename())),
- URI: protocol.DocumentURI(uri),
+ Detail: fmt.Sprintf("%s • %s", pkgPath, filepath.Base(fh.URI().Filename())),
+ URI: loc.URI,
Range: rng,
SelectionRange: rng,
}, nil
diff --git a/gopls/internal/lsp/source/references.go b/gopls/internal/lsp/source/references.go
index afe03f1..f14d1d6 100644
--- a/gopls/internal/lsp/source/references.go
+++ b/gopls/internal/lsp/source/references.go
@@ -11,14 +11,11 @@
"go/ast"
"go/token"
"go/types"
- "sort"
- "strings"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/safetoken"
"golang.org/x/tools/gopls/internal/span"
"golang.org/x/tools/internal/bug"
- "golang.org/x/tools/internal/event"
)
// ReferenceInfo holds information about reference to an identifier in Go source.
@@ -31,114 +28,6 @@
isDeclaration bool
}
-// referencesV1 returns a list of references for a given identifier within the packages
-// containing pp. Declarations appear first in the result.
-//
-// Currently called only from Server.incomingCalls.
-// TODO(adonovan): switch over to referencesV2.
-func referencesV1(ctx context.Context, snapshot Snapshot, f FileHandle, pp protocol.Position, includeDeclaration bool) ([]*ReferenceInfo, error) {
- ctx, done := event.Start(ctx, "source.References")
- defer done()
-
- // Is the cursor within the package name declaration?
- pgf, inPackageName, err := parsePackageNameDecl(ctx, snapshot, f, pp)
- if err != nil {
- return nil, err
- }
- if inPackageName {
- // TODO(rfindley): this is redundant with package renaming. Refactor to share logic.
- metas, err := snapshot.MetadataForFile(ctx, f.URI())
- if err != nil {
- return nil, err
- }
- if len(metas) == 0 {
- return nil, fmt.Errorf("found no package containing %s", f.URI())
- }
- targetPkg := metas[len(metas)-1] // widest package
-
- // Find external direct references to the package (imports).
- rdeps, err := snapshot.ReverseDependencies(ctx, targetPkg.ID, false)
- if err != nil {
- return nil, err
- }
-
- var refs []*ReferenceInfo
- for _, rdep := range rdeps {
- for _, uri := range rdep.CompiledGoFiles {
- fh, err := snapshot.GetFile(ctx, uri)
- if err != nil {
- return nil, err
- }
- f, err := snapshot.ParseGo(ctx, fh, ParseHeader)
- if err != nil {
- return nil, err
- }
- for _, imp := range f.File.Imports {
- if rdep.DepsByImpPath[UnquoteImportPath(imp)] == targetPkg.ID {
- rng, err := f.PosMappedRange(imp.Pos(), imp.End())
- if err != nil {
- return nil, err
- }
- refs = append(refs, &ReferenceInfo{
- Name: pgf.File.Name.Name,
- MappedRange: rng,
- })
- }
- }
- }
- }
-
- // Find the package declaration of each file in the target package itself.
- for _, uri := range targetPkg.CompiledGoFiles {
- fh, err := snapshot.GetFile(ctx, uri)
- if err != nil {
- return nil, err
- }
- f, err := snapshot.ParseGo(ctx, fh, ParseHeader)
- if err != nil {
- return nil, err
- }
- rng, err := f.PosMappedRange(f.File.Name.Pos(), f.File.Name.End())
- if err != nil {
- return nil, err
- }
- refs = append(refs, &ReferenceInfo{
- Name: pgf.File.Name.Name,
- MappedRange: rng,
- })
- }
-
- return refs, nil
- }
-
- qualifiedObjs, err := qualifiedObjsAtProtocolPos(ctx, snapshot, f.URI(), pp)
- // Don't return references for builtin types.
- if errors.Is(err, errBuiltin) {
- return nil, nil
- }
- if err != nil {
- return nil, err
- }
-
- refs, err := references(ctx, snapshot, qualifiedObjs, includeDeclaration, true, false)
- if err != nil {
- return nil, err
- }
-
- toSort := refs
- if includeDeclaration {
- toSort = refs[1:]
- }
- sort.Slice(toSort, func(i, j int) bool {
- x, y := toSort[i], toSort[j]
- if cmp := strings.Compare(string(x.MappedRange.URI()), string(y.MappedRange.URI())); cmp != 0 {
- return cmp < 0
- }
- return x.ident.Pos() < y.ident.Pos()
- })
- return refs, nil
-}
-
// parsePackageNameDecl is a convenience function that parses and
// returns the package name declaration of file fh, and reports
// whether the position ppos lies within it.
@@ -159,8 +48,7 @@
// Only the definition-related fields of qualifiedObject are used.
// (Arguably it should accept a smaller data type.)
//
-// This implementation serves referencesV1 (the soon-to-be obsolete
-// portion of Server.references) and Server.rename.
+// This implementation serves Server.rename. TODO(adonovan): obviate it.
func references(ctx context.Context, snapshot Snapshot, qos []qualifiedObject, includeDeclaration, includeInterfaceRefs, includeEmbeddedRefs bool) ([]*ReferenceInfo, error) {
var (
references []*ReferenceInfo
diff --git a/gopls/internal/lsp/source/references2.go b/gopls/internal/lsp/source/references2.go
index 2a40065..4b6ba4c 100644
--- a/gopls/internal/lsp/source/references2.go
+++ b/gopls/internal/lsp/source/references2.go
@@ -39,8 +39,13 @@
// object as the subject of a References query.
type ReferenceInfoV2 struct {
IsDeclaration bool
- Name string // TODO(adonovan): same for all elements; factor out of the slice?
Location protocol.Location
+
+ // TODO(adonovan): these are the same for all elements; factor out of the slice.
+ // TODO(adonovan): Name is currently unused. If it's still unused when we
+ // eliminate 'references' (v1), delete it. Or replace both fields by a *Metadata.
+ PkgPath PackagePath
+ Name string
}
// References returns a list of all references (sorted with
@@ -51,11 +56,9 @@
if err != nil {
return nil, err
}
- // TODO(adonovan): eliminate references[i].Name field?
- // But it may be needed when replacing referencesV1.
- var locations []protocol.Location
- for _, ref := range references {
- locations = append(locations, ref.Location)
+ locations := make([]protocol.Location, len(references))
+ for i, ref := range references {
+ locations[i] = ref.Location
}
return locations, nil
}
@@ -68,14 +71,14 @@
defer done()
// Is the cursor within the package name declaration?
- pgf, inPackageName, err := parsePackageNameDecl(ctx, snapshot, f, pp)
+ _, inPackageName, err := parsePackageNameDecl(ctx, snapshot, f, pp)
if err != nil {
return nil, err
}
var refs []*ReferenceInfoV2
if inPackageName {
- refs, err = packageReferences(ctx, snapshot, f.URI(), pgf.File.Name.Name)
+ refs, err = packageReferences(ctx, snapshot, f.URI())
} else {
refs, err = ordinaryReferences(ctx, snapshot, f.URI(), pp)
}
@@ -110,7 +113,7 @@
// declaration of the specified name and uri by searching among the
// import declarations of all packages that directly import the target
// package.
-func packageReferences(ctx context.Context, snapshot Snapshot, uri span.URI, pkgname string) ([]*ReferenceInfoV2, error) {
+func packageReferences(ctx context.Context, snapshot Snapshot, uri span.URI) ([]*ReferenceInfoV2, error) {
metas, err := snapshot.MetadataForFile(ctx, uri)
if err != nil {
return nil, err
@@ -135,7 +138,7 @@
if narrowest.ForTest != "" && strings.HasSuffix(string(uri), "_test.go") {
for _, f := range narrowest.CompiledGoFiles {
if !strings.HasSuffix(string(f), "_test.go") {
- return packageReferences(ctx, snapshot, f, pkgname)
+ return packageReferences(ctx, snapshot, f)
}
}
// This package has no non-test files.
@@ -160,8 +163,9 @@
if rdep.DepsByImpPath[UnquoteImportPath(imp)] == narrowest.ID {
refs = append(refs, &ReferenceInfoV2{
IsDeclaration: false,
- Name: pkgname,
Location: mustLocation(f, imp),
+ PkgPath: narrowest.PkgPath,
+ Name: string(narrowest.Name),
})
}
}
@@ -187,8 +191,9 @@
}
refs = append(refs, &ReferenceInfoV2{
IsDeclaration: true, // (one of many)
- Name: pkgname,
Location: mustLocation(f, f.File.Name),
+ PkgPath: widest.PkgPath,
+ Name: string(widest.Name),
})
}
@@ -310,8 +315,9 @@
report := func(loc protocol.Location, isDecl bool) {
ref := &ReferenceInfoV2{
IsDeclaration: isDecl,
- Name: obj.Name(),
Location: loc,
+ PkgPath: pkg.PkgPath(),
+ Name: obj.Name(),
}
refsMu.Lock()
refs = append(refs, ref)
