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 <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: adonovan

gopls/internal/golang/call_hierarchy.go

@@ -125,48 +125,59 @@ func enclosingNodeCallItem(ctx context.Context, snapshot *cache.Snapshot, pkgPat
 		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 @@ func OutgoingCalls(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle
 		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

gopls/internal/test/marker/doc.go

@@ -175,6 +175,8 @@ The following markers are supported within marker tests:
   - 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

gopls/internal/test/marker/testdata/callhierarchy/callhierarchy.txt

@@ -26,36 +26,34 @@ func B() { //@loc(outgoingB, "B")
 }
 
 -- 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 @@ func D() { //@loc(hierarchyD, "D"),incomingcalls(hierarchyD, hierarchyA, hierarc
 	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) H() {}
 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")
 }

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)