For is_token_kind() and is_rule_kind(), I wonder why do they take multiple kinds, rather than only one? is it an optimization for NAPI? does that mean that we won't do that if it was a native TS API?

That is for alignment with the Cursor API, where you can search for more than one at a time without having to do multiple searches over multiple Cursors. In TS it will be is_token_kind(TokenKind, ...).

Note that it also returns the matched TokenNode, meaning no match is required, and it functions therefore as a typeguard.

Note also the following is_token_matching which is also for alignment with the cursor API.

I think is_XX() usually indicates returning booleans, a cheap check that can be chained in a boolean condition with other conditions. Thoughts about something like as_token()? which indicates a "cast" that can succeed/fail (thus returning an option).

agreed, done

regarding node() and node_or_none(), I might be missing something, but I would have expected the rust idiomatic way is to return Option<Node> only/directly for things like this, and let the user expect()/unwrap() if needed? why the extra API?

Because there are many cases where the client know that node isn't none e.g. after a successful go_to_*, and I wanted to not require .unwrap() there, and instead catch it in one place. Happy to change the method names if you have a suggestion?

I think this is idiomatic to let the user make that decision, and to make it explicit at the call site (a strong direction in rust). I think similar to how a vector/map/set would offer get() -> Option<T> without (the often convenient) get_and_unwrap() -> T

trait Index provides [] for this, and it is implemented on Vec, HashMap, and BTreeMap for example.

But in any case, solved with an explicit completed state. Also allows the cursor to be reset

not sure I understand the use case for path_rule_nodes(). I see it is currently unused?

So that you can access the parent RuleNodes when using a cursor to navigate the tree.

I think I'm missing something. This would convert it to a plain JS object, however, us typing it with RuleNode | TokenNode means that it will have the usual APIs on it, like the ones that need round tripping to Rust. How does that work?

this is related to other to_js() uses as well.

In this case we have to return JsObject because the result is polymorphic. And fundamentally, JsObject is what is passed even if the Rust type were explicit.

So on the receiver side, working in NodeJS land, they would get a plain JS object.
It wouldn't have the native Rust APIs? not sure how would calling its methods would round trip.

... I think I will wait for this to be ready and play with it for a bit to understand more.

TOL, I wonder if we should be exposing the cursor to the visitor API, even if it cannot be used/mutated by the caller? what is the use case here?

For two reasons

1. You can start a Visitor at any Cursor position. It is nice in the Visitor to be able to get to the Cursor and do some more internal iteration. Apart from being convenient, it makes internal and external iterators nicely symmetric.
2. Regardless of the symmetry, it is convenient in the visitor to be able to access things like the text_range from the Cursor, and also the path_rule_nodes. As the Cursor API is extended, the Visitor implementations can use those facilities.

I suggest splitting into three tests for readability.

I thought about that, but because the 'test' nature of this is not actually for testing, but just to validate that the example is ok, I decided that the clarity I could gain by simply repeating a section of code with exactly the same result but using different API patterns was worth having the repeated functionality.

I think it is also for "testing", since the Rust test infra is the same in all projects, this is a quick way for someone to just copy a test locally, and edit/run it for experimenting.

It also indicates to readers that these are separate, instead of having side effects on later statements.

nit: ensure!() to be consistent with bail!() instead of panicking?

done

nit: I suggest quitting early when possible (like the existing visitor) for readability, and to decrease nesting. But not a blocker at all.

if !interesting {
  return;
}
do_something();

rather than:

if interesting {
  do_something();
}
return;

As a functional programming fan, I prefer to have a single control flow, and therefore not use guards as often.

TOL: thoughts about unifying findXXXX() and findXXXXWithoutRecursing() to a default argument findXXXX(recursive = true)?

I wouldn't want to add a recurse param to find* if the use cases were not equaly distributed, which I suspect they wouldn't be.

But in any case I've removed the *WithoutRecursing forms because they cannot work - the movement to the next has to happen between calls to those methods.