Skip to content

Commit 0b693ed

Browse files
adonovangopherbot
authored andcommitted
internal/astutil/cursor: FindPos: don't assume Files are in Pos order
+ test Change-Id: I75c4c3b7789feda874da7644bda8ba93f87f5efb Reviewed-on: https://go-review.googlesource.com/c/tools/+/650645 Reviewed-by: Robert Findley <[email protected]> Auto-Submit: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent cd01e86 commit 0b693ed

File tree

2 files changed

+37
-7
lines changed

2 files changed

+37
-7
lines changed

internal/astutil/cursor/cursor.go

+12-6
Original file line numberDiff line numberDiff line change
@@ -428,15 +428,21 @@ func (c Cursor) FindPos(start, end token.Pos) (Cursor, bool) {
428428
ev := events[i]
429429
if ev.index > i { // push?
430430
n := ev.node
431-
var nodeStart, nodeEnd token.Pos
431+
var nodeEnd token.Pos
432432
if file, ok := n.(*ast.File); ok {
433-
nodeStart, nodeEnd = file.FileStart, file.FileEnd
433+
nodeEnd = file.FileEnd
434+
// Note: files may be out of Pos order.
435+
if file.FileStart > start {
436+
i = ev.index // disjoint, after; skip to next file
437+
continue
438+
}
434439
} else {
435-
nodeStart, nodeEnd = n.Pos(), n.End()
436-
}
437-
if nodeStart > start {
438-
break // disjoint, after; stop
440+
nodeEnd = n.End()
441+
if n.Pos() > start {
442+
break // disjoint, after; stop
443+
}
439444
}
445+
// Inv: node.{Pos,FileStart} <= start
440446
if end <= nodeEnd {
441447
// node fully contains target range
442448
best = i

internal/astutil/cursor/cursor_test.go

+25-1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"go/token"
1515
"iter"
1616
"log"
17+
"math/rand"
1718
"path/filepath"
1819
"reflect"
1920
"slices"
@@ -332,8 +333,31 @@ func TestCursor_FindNode(t *testing.T) {
332333
}
333334
}
334335
}
336+
}
337+
338+
// TestCursor_FindPos_order ensures that FindPos does not assume files are in Pos order.
339+
func TestCursor_FindPos_order(t *testing.T) {
340+
// Pick an arbitrary decl.
341+
target := netFiles[7].Decls[0]
342+
343+
// Find the target decl by its position.
344+
cur, ok := cursor.Root(netInspect).FindPos(target.Pos(), target.End())
345+
if !ok || cur.Node() != target {
346+
t.Fatalf("unshuffled: FindPos(%T) = (%v, %t)", target, cur, ok)
347+
}
335348

336-
// TODO(adonovan): FindPos needs a test (not just a benchmark).
349+
// Shuffle the files out of Pos order.
350+
files := slices.Clone(netFiles)
351+
rand.Shuffle(len(files), func(i, j int) {
352+
files[i], files[j] = files[j], files[i]
353+
})
354+
355+
// Find it again.
356+
inspect := inspector.New(files)
357+
cur, ok = cursor.Root(inspect).FindPos(target.Pos(), target.End())
358+
if !ok || cur.Node() != target {
359+
t.Fatalf("shuffled: FindPos(%T) = (%v, %t)", target, cur, ok)
360+
}
337361
}
338362

339363
func TestCursor_Edge(t *testing.T) {

0 commit comments

Comments
 (0)