go/ast/inspector: FindByPos returns the first innermost node

Fix golang/go#76872

Change-Id: I35d9c6cb0abfe2d380bf326bdd1e547a3f1abc89
Reviewed-on: https://go-review.googlesource.com/c/tools/+/732521
Auto-Submit: Hongxiang Jiang <hxjiang@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

Author: h9jiang

go/ast/inspector/cursor.go

@@ -453,6 +453,9 @@ func (c Cursor) FindNode(n ast.Node) (Cursor, bool) {
 // rooted at c such that n.Pos() <= start && end <= n.End().
 // (For an *ast.File, it uses the bounds n.FileStart-n.FileEnd.)
 //
+// An empty range (start == end) between two adjacent nodes is
+// considered to belong to the first node.
+//
 // It returns zero if none is found.
 // Precondition: start <= end.
 //
@@ -501,10 +504,17 @@ func (c Cursor) FindByPos(start, end token.Pos) (Cursor, bool) {
 					break // disjoint, after; stop
 				}
 			}
+
 			// Inv: node.{Pos,FileStart} <= start
 			if end <= nodeEnd {
 				// node fully contains target range
 				best = i
+
+				// Don't search beyond end of the first match.
+				// This is important only for an empty range (start=end)
+				// between two adjoining nodes, which would otherwise
+				// match both nodes; we want to match only the first.
+				limit = ev.index
 			} else if nodeEnd < start {
 				i = ev.index // disjoint, before; skip forward
 			}

go/ast/inspector/cursor_test.go

@@ -225,7 +225,7 @@ func TestCursor_FindNode(t *testing.T) {
 	inspect := netInspect
 
 	// Enumerate all nodes of a particular type,
-	// then check that FindPos can find them,
+	// then check that FindByPos can find them,
 	// starting at the root.
 	//
 	// (We use BasicLit because they are numerous.)
@@ -263,15 +263,15 @@ func TestCursor_FindNode(t *testing.T) {
 	}
 }
 
-// TestCursor_FindPos_order ensures that FindPos does not assume files are in Pos order.
-func TestCursor_FindPos_order(t *testing.T) {
+// TestCursor_FindByPos_order ensures that FindByPos does not assume files are in Pos order.
+func TestCursor_FindByPos_order(t *testing.T) {
 	// Pick an arbitrary decl.
 	target := netFiles[7].Decls[0]
 
 	// Find the target decl by its position.
 	cur, ok := netInspect.Root().FindByPos(target.Pos(), target.End())
 	if !ok || cur.Node() != target {
-		t.Fatalf("unshuffled: FindPos(%T) = (%v, %t)", target, cur, ok)
+		t.Fatalf("unshuffled: FindByPos(%T) = (%v, %t)", target, cur, ok)
 	}
 
 	// Shuffle the files out of Pos order.
@@ -284,7 +284,7 @@ func TestCursor_FindPos_order(t *testing.T) {
 	inspect := inspector.New(files)
 	cur, ok = inspect.Root().FindByPos(target.Pos(), target.End())
 	if !ok || cur.Node() != target {
-		t.Fatalf("shuffled: FindPos(%T) = (%v, %t)", target, cur, ok)
+		t.Fatalf("shuffled: FindByPos(%T) = (%v, %t)", target, cur, ok)
 	}
 }
 
@@ -379,6 +379,59 @@ func TestCursor_Edge(t *testing.T) {
 	}
 }
 
+// Regression test for mutilple matching nodes in FindByPos (#76872).
+func TestCursor_FindByPos_Boundary(t *testing.T) {
+	// This test verifies that when a cursor position is on the boundary of two
+	// adjacent nodes (e.g. "foo|("), FindByPos returns the first node
+	// encountered in traversal order (which is usually the node "to the left").
+	//
+	// Note: The source is intentionally unformatted (no space between ')' and
+	// '{') to ensure the nodes are strictly adjacent at the boundary.
+	const src = `package p; func foo(a int){}`
+	var (
+		fset    = token.NewFileSet()
+		f, _    = parser.ParseFile(fset, "p.go", src, 0)
+		tokFile = fset.File(f.FileStart)
+		inspect = inspector.New([]*ast.File{f})
+	)
+
+	d := f.Decls[0].(*ast.FuncDecl)
+
+	format := func(pos token.Pos) string {
+		off := tokFile.Offset(pos)
+		return fmt.Sprintf("...%s<<>>%s...", src[off-1:off], src[off:off+1])
+	}
+
+	for _, test := range []struct {
+		name string
+		pos  token.Pos
+		want ast.Node
+	}{
+		{
+			// "foo|(" Ident
+			pos:  d.Type.Params.Opening,
+			want: d.Name,
+		},
+		{
+			// ")|{" FieldList
+			pos:  d.Body.Pos(),
+			want: d.Type.Params,
+		},
+	} {
+		cur, ok := inspect.Root().FindByPos(test.pos, test.pos)
+		if !ok {
+			t.Fatalf("FindByPos(%d) %s found nothing", test.pos, format(test.pos))
+		}
+
+		if cur.Node() != test.want {
+			t.Errorf("FindByPos(%d) %s:\ngot  %T (%v)\nwant %T (%v)",
+				test.pos, format(test.pos),
+				cur.Node(), cur.Node(),
+				test.want, test.want)
+		}
+	}
+}
+
 // Regression test for FuncDecl.Type irregularity in FindByPos (#75997).
 func TestCursor_FindByPos(t *testing.T) {
 	// Observe that the range of FuncType has a hole between
@@ -615,12 +668,12 @@ func BenchmarkCursor_FindNode(b *testing.B) {
 	})
 
 	// This method is about 100x (!) faster than Cursor.Preorder.
-	b.Run("Cursor.FindPos", func(b *testing.B) {
+	b.Run("Cursor.FindByPos", func(b *testing.B) {
 		needleNode := needle.Node()
 		for b.Loop() {
 			found, ok := root.FindByPos(needleNode.Pos(), needleNode.End())
 			if !ok || found != needle {
-				b.Errorf("FindPos search failed: got %v, want %v", found, needle)
+				b.Errorf("FindByPos search failed: got %v, want %v", found, needle)
 			}
 		}
 	})

gopls/internal/test/marker/testdata/references/issue76872.txt

@@ -0,0 +1,16 @@
+This test exercises a references query at a position where an empty selection
+might be considered enclosed both by the node before (Ident) and the node after
+(FuncType). It ensures that we find the reference of the syntax node before the
+cursor.
+
+See https://github.com/golang/go/issues/76872.
+
+-- main.go --
+package main
+
+// re"foo()" matches the zero-width position after "foo", not the call parens.
+func foo() {} //@ loc(decl, "foo"), refs(re"foo()", decl, call)
+
+func _() {
+	foo() //@ loc(call, "foo")
+}