gopls/internal/lsp/source: ignore lambdas in call hierarchy
Previously, anonymous functions were treated as nodes
in the call hierarchy view, but it doesn't make sense to
do so because we have no reliable way to find references
to them, except from within their enclosing function.
So, this change treats anonymous functions and their
enclosing functions as a single item.
Fixes golang/go#64451
Change-Id: I3841adcbad4b13ab190fad58daf38c1bbc6f8baa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/546736
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/golang/call_hierarchy.go b/gopls/internal/golang/call_hierarchy.go
index 4971208..04dc9de 100644
--- a/gopls/internal/golang/call_hierarchy.go
+++ b/gopls/internal/golang/call_hierarchy.go
@@ -125,48 +125,59 @@
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 named function, if any.
+ //
+ // It is tempting to treat anonymous functions as nodes in the
+ // call hierarchy, and historically we used to do that,
+ // poorly; see #64451. However, it is impossible to track
+ // references to anonymous functions without much deeper
+ // analysis. Local analysis is tractable, but ultimately it
+ // can only detect calls from the outer function to the inner
+ // function.
+ //
+ // It is simpler and clearer to treat the top-level named
+ // function and all its nested functions as one entity, and it
+ // allows users to recursively expand the tree where, before,
+ // the chain would be broken by each lambda.
+ //
+ // If the selection is in a global var initializer,
+ // default to the file's package declaration.
path, _ := astutil.PathEnclosingInterval(pgf.File, start, end)
-outer:
+ var (
+ name = pgf.File.Name.Name
+ kind = protocol.Package
+ )
+ start, end = pgf.File.Name.Pos(), pgf.File.Name.End()
for _, node := range path {
- switch n := node.(type) {
+ switch node := node.(type) {
case *ast.FuncDecl:
- funcDecl = n
- break outer
+ name = node.Name.Name
+ start, end = node.Name.Pos(), node.Name.End()
+ kind = protocol.Function
+
case *ast.FuncLit:
- litCount++
- if litCount > 1 {
- continue
- }
- funcLit = n
+ // If the call comes from a FuncLit with
+ // no enclosing FuncDecl, then use the
+ // FuncLit's extent.
+ name = "func"
+ start, end = node.Pos(), node.Type.End() // signature, sans body
+ kind = protocol.Function
+
+ case *ast.ValueSpec:
+ // If the call comes from a var (or,
+ // theoretically, const) initializer outside
+ // any function, then use the ValueSpec.Names span.
+ name = "init"
+ start, end = node.Names[0].Pos(), node.Names[len(node.Names)-1].End()
+ kind = protocol.Variable
}
}
- nameIdent := path[len(path)-1].(*ast.File).Name
- kind := protocol.Package
- if funcDecl != nil {
- nameIdent = funcDecl.Name
- kind = protocol.Function
- }
-
- nameStart, nameEnd := nameIdent.Pos(), nameIdent.End()
- if funcLit != nil {
- nameStart, nameEnd = funcLit.Type.Func, funcLit.Type.Params.Pos()
- kind = protocol.Function
- }
- rng, err := pgf.PosRange(nameStart, nameEnd)
+ rng, err := pgf.PosRange(start, end)
if err != nil {
return protocol.CallHierarchyItem{}, err
}
- name := nameIdent.Name
- for i := 0; i < litCount; i++ {
- name += ".func()"
- }
-
return protocol.CallHierarchyItem{
Name: name,
Kind: kind,
@@ -216,7 +227,6 @@
return nil, err
}
- // Use TypecheckFull as we want to inspect the body of the function declaration.
declPkg, declPGF, err := NarrowestPackageForFile(ctx, snapshot, uri)
if err != nil {
return nil, err
diff --git a/gopls/internal/test/marker/doc.go b/gopls/internal/test/marker/doc.go
index c2935f9..5acc4ab 100644
--- a/gopls/internal/test/marker/doc.go
+++ b/gopls/internal/test/marker/doc.go
@@ -175,6 +175,8 @@
- incomingcalls(src location, want ...location): makes a
callHierarchy/incomingCalls query at the src location, and checks that
the set of call.From locations matches want.
+ (These locations are the declarations of the functions enclosing
+ the calls, not the calls themselves.)
- item(label, details, kind): defines a completion item with the provided
fields. This information is not positional, and therefore @item markers
diff --git a/gopls/internal/test/marker/testdata/callhierarchy/callhierarchy.txt b/gopls/internal/test/marker/testdata/callhierarchy/callhierarchy.txt
index 2621f67..43fbdd6 100644
--- a/gopls/internal/test/marker/testdata/callhierarchy/callhierarchy.txt
+++ b/gopls/internal/test/marker/testdata/callhierarchy/callhierarchy.txt
@@ -26,36 +26,34 @@
}
-- hierarchy.go --
-package callhierarchy
+package callhierarchy //@loc(hPkg, "callhierarchy")
import "golang.org/lsptests/callhierarchy/outgoing"
-func a() { //@loc(hierarchyA, "a")
+func a() { //@loc(hA, "a")
D()
}
-func b() { //@loc(hierarchyB, "b")
+func b() { //@loc(hB, "b")
D()
}
// C is an exported function
-func C() { //@loc(hierarchyC, "C")
+func C() { //@loc(hC, "C")
D()
D()
}
// To test hierarchy across function literals
-var x = func() { //@loc(hierarchyLiteral, "func"),loc(hierarchyLiteralOut, "x")
- D()
-}
+var x = func() { D() } //@loc(hX, "x"),loc(hXGlobal, "x")
// D is exported to test incoming/outgoing calls across packages
-func D() { //@loc(hierarchyD, "D"),incomingcalls(hierarchyD, hierarchyA, hierarchyB, hierarchyC, hierarchyLiteral, incomingA),outgoingcalls(hierarchyD, hierarchyE, hierarchyF, hierarchyG, hierarchyLiteralOut, outgoingB, hierarchyFoo, hierarchyH, hierarchyI, hierarchyJ, hierarchyK)
+func D() { //@loc(hD, "D"),incomingcalls(hD, hA, hB, hC, hXGlobal, incomingA),outgoingcalls(hD, hE, hF, hG, hX, outgoingB, hFoo, hH, hI, hJ, hK)
e()
x()
F()
outgoing.B()
- foo := func() {} //@loc(hierarchyFoo, "foo"),incomingcalls(hierarchyFoo, hierarchyD),outgoingcalls(hierarchyFoo)
+ foo := func() {} //@loc(hFoo, "foo"),incomingcalls(hFoo, hD),outgoingcalls(hFoo)
foo()
func() {
@@ -71,16 +69,16 @@
s.K()
}
-func e() {} //@loc(hierarchyE, "e")
+func e() {} //@loc(hE, "e")
// F is an exported function
-func F() {} //@loc(hierarchyF, "F")
+func F() {} //@loc(hF, "F")
-func g() {} //@loc(hierarchyG, "g")
+func g() {} //@loc(hG, "g")
type Interface interface {
- H() //@loc(hierarchyH, "H")
- I() //@loc(hierarchyI, "I")
+ H() //@loc(hH, "H")
+ I() //@loc(hI, "I")
}
type impl struct{}
@@ -89,6 +87,6 @@
func (i impl) I() {}
type Struct struct {
- J func() //@loc(hierarchyJ, "J")
- K func() //@loc(hierarchyK, "K")
+ J func() //@loc(hJ, "J")
+ K func() //@loc(hK, "K")
}
diff --git a/gopls/internal/test/marker/testdata/callhierarchy/issue64451.txt b/gopls/internal/test/marker/testdata/callhierarchy/issue64451.txt
new file mode 100644
index 0000000..618d6ed
--- /dev/null
+++ b/gopls/internal/test/marker/testdata/callhierarchy/issue64451.txt
@@ -0,0 +1,51 @@
+This test checks call hierarchy queries involving lambdas, which are
+treated as mere statements of their enclosing name function, since
+we can't track calls to them.
+
+Calls from a global var decl are reported at the ValueSpec.Names.
+
+See golang/go#64451.
+
+-- go.mod --
+module example.com
+go 1.0
+
+-- a/a.go --
+package a
+
+func foo() { //@ loc(foo, "foo")
+ bar()
+}
+
+func bar() { //@ loc(bar, "bar")
+ go func() { baz() }()
+}
+
+func baz() { //@ loc(baz, "baz")
+ bluh()
+}
+
+func bluh() { //@ loc(bluh, "bluh")
+ print()
+}
+
+var _ = func() int { //@ loc(global, "_")
+ baz()
+ return 0
+}()
+
+func init() { //@ loc(init, "init")
+ baz()
+}
+
+//@ outgoingcalls(foo, bar)
+//@ outgoingcalls(bar, baz)
+//@ outgoingcalls(baz, bluh)
+//@ outgoingcalls(bluh)
+//@ outgoingcalls(init, baz)
+
+//@ incomingcalls(foo)
+//@ incomingcalls(bar, foo)
+//@ incomingcalls(baz, bar, global, init)
+//@ incomingcalls(bluh, baz)
+//@ incomingcalls(init)
