feat: add visitors-core crate

[sonicfromnewyoke]

Problem

codama-rs can build an IDL but has no standard way to walk and rewrite one, so this PR adds the core "visitor" trait that does that.

Summary of Changes

• create codama-visitors-core crate + describe it in README
• add TransformVisitor trait
• cover by init tests

initial part of splitting a large #107 into smaller pieces

[lorisleiva]

Thanks for breaking this in smaller PRs! Now we can start talking about the core addition first: What is a node visitor in Rust? Your definition currently seems to be: none haha. You've got a TransformVisitor but we don't have the concept of a generic Visitor<T> like the node/visitor pattern enables.

One of the biggest value in having everything designed as a tree of nodes is you can define a Visitor<T> interface that runs through a tree of Node recursively (or a subset of it) and outputs any type T.

Visitor<Node> is just a special case that goes through the tree and return another tree. Perfect for transformation. Visitor<Node | null> can be pushed further to allow node deletion during the transform. Visitor<string> can be used to display the IDL. Visitor<UsageHistogram> can be used to aggregate some statistics such that another visitor can perform more complex tasks. You get the picture, one of the most valuable feature of the visitor pattern is how rich the API can be.

Now I understand that things may need to be implemented differently in Rust in order to be efficient. A mutable visitor isn't necessarily a bad idea but it should be in addition to the main visitor interface that can return any type T. I also think that being able to construct immutable trees via visitors would be a safer approach if we could implement it that way.

Side note: because Rust has pattern matching, we may not even need visitors to be this huge set of functions (one per node) like it is in TypeScript. Instead, we can just make it a single function that accepts any Node and returns T. It is then within that function that we can define all the different node behaviours using pattern matching.

[sonicfromnewyoke]

Thanks for breaking this in smaller PRs! Now we can start talking about the core addition first: What is a node visitor in Rust? Your definition currently seems to be: none haha. You've got a TransformVisitor but we don't have the concept of a generic Visitor<T> like the node/visitor pattern enables.

One of the biggest value in having everything designed as a tree of nodes is you can define a Visitor<T> interface that runs through a tree of Node recursively (or a subset of it) and outputs any type T.

Visitor<Node> is just a special case that goes through the tree and return another tree. Perfect for transformation. Visitor<Node | null> can be pushed further to allow node deletion during the transform. Visitor<string> can be used to display the IDL. Visitor<UsageHistogram> can be used to aggregate some statistics such that another visitor can perform more complex tasks. You get the picture, one of the most valuable feature of the visitor pattern is how rich the API can be.

Now I understand that things may need to be implemented differently in Rust in order to be efficient. A mutable visitor isn't necessarily a bad idea but it should be in addition to the main visitor interface that can return any type T. I also think that being able to construct immutable trees via visitors would be a safer approach if we could implement it that way.

Side note: because Rust has pattern matching, we may not even need visitors to be this huge set of functions (one per node) like it is in TypeScript. Instead, we can just make it a single function that accepts any Node and returns T. It is then within that function that we can define all the different node behaviours using pattern matching.

fair enough, sorry for that. Right now this PR is only the syn::fold part. TransformVisitor is really just a Visitor<Node>, and you're right it shouldn't be the base.

So, we can create 1 generic trait, and use pattern matching to dispatch. As you correctly mentioned that rust doesn't need a method per node like js has.

something like follows:

pub trait Visitor<T> {
    fn visit(&mut self, node: &Node) -> T;
}

here we pass:

• &Node so it never touches the input. Stays immutable, and we can run it over the same tree as over and over again (if needed)
• T is a type param (not an associated type) so one struct can be a Visitor<Histogram> AND a Visitor<usize>, and a map helper can turn one output type into another one. It's also object-safe, so something like &mut dyn Visitor<String> should work.

for examples you provided earlier:

impl Visitor<Histogram> for CountKinds {
    fn visit(&mut self, n: &Node) -> Histogram {
        walk_reduce(self, n, Histogram::one(n.kind()), |a, b| a.merge(b))
    }
}

Visitor<String> to print, Visitor<usize>/Visitor<Histogram> for stats, Visitor<Node> to transform, and Visitor<Option<Node>> to transform + delete (return None to drop a node). That last one is way better than take_deleted() flag which i have now, so ill move deletion over to returning Option.

For the transform part (like array->bytes, number->bool, flatten, unwrap and so on, which change kinds and dig into NestedTypeNode), id prefer to keep today's typed fold as a helper trait (maybe call it Fold), with deletion done by returning Option. It gives compile-time checks and clean wrapper handling the generic path can't. And it's not really a 99-method monster - like literal js port, the part that matters is about 16 union entry points plus the leaves.
But im not stuck on it. If you would like to have one trait and just live with Visitor<Node>/Visitor<Option<Node>> for transforms too (trading the compile-time safety for simplicity) im down.

few words about mutability: the current fold is already immutable - its by-value Node -> Node, never &mut Node. So immutable is the default already. A real in-place &mut Node version would be the "extra, for speed" which one you mentioned.

what do you think about the next 3 steps?:

1. Visitor<T> trait + the walk/walk_reduce helpers, with Visitor<String> and Visitor<usize>/Visitor<Histogram> as the first users. No transform code, just proves out the read/stats path.
2. transform path: Visitor<Node>/Visitor<Option<Node>> (plus the typed Fold if we agree on it), deletion via Option
3. port the visitor library on top.

[lorisleiva]

Hey @sonicfromnewyoke, sorry for the late reply. I wanted to take my time to think about this properly.

I think you've landed on the right core idea: a single generic trait plus pattern matching rather than a per-node method table. Fully on board with:

pub trait Visitor<T> {
    fn visit(&mut self, node: &Node) -> T;
}

The one thing I don't want us to lose is the "default walk" / next.

The real power of the JS visitor is the open recursion: override the node you care about, and let the default walk (next) handle everything else, recursing back through your visitor (self). If our only entry point is visit, then anyone who wants "default behaviour for everything except arrayTypeNode" has to hand-write the whole match and recurse manually. That throws away the biggest win.

You already had this with walk_reduce in your CountKinds example. I'd just make sure we also ship the identity/rebuild walk, not only the merge/reduce one. Side note: should we mirror the JS names here?

// dispatch (JS `visit(node, visitor)`)
pub fn visit<V: Visitor<T> + ?Sized, T>(visitor: &mut V, node: &Node) -> T;

// default walks: the `next`. Behaviour of the generated identityVisitor / mergeVisitor,
// but exposed as functions rather than visitor factories.
pub fn visit_identity<V: Visitor<Node> + ?Sized>(visitor: &mut V, node: &Node) -> Node;
pub fn visit_merge<V: Visitor<T> + ?Sized, T>(visitor: &mut V, node: &Node, /* leaf, merge */) -> T;

That way we end up with reusable visit_identity/visit_merge functions in Rust that mirror the composable identityVisitor/mergeVisitor in JS (not quite as flexible as JS but pretty good for Rust IMO).

Here's an example of what it may look like to extend the "identity visitor" with that pattern:

impl Visitor<Node> for MyTransformVisitor {
    fn visit(&mut self, node: &Node) -> Node {
        match node {
            Node::Type(RegisteredTypeNode::Array(_)) => { /* custom */ }
            other => visit_identity(self, other), // next: recurse via self
        }
    }
}

Regarding the separate typed Fold trait, I could be convinced otherwise but I'm not sure it's even necessary. It would create multiple core Visitor definitions which is a bit annoying in terms of clean API.

The main thing a Fold trait would buy us is compile-time guarantees that, when we rebuild a node, its children are the right kind (e.g. an arrayTypeNode only ever gets a valid TypeNode back). But I think we already get that for free: NestedTypeNode<T> has try_map_nested_type_node, and the TryFrom<Node>/TryFrom<TypeNode> conversions already re-check the kind whenever we plug a child back into a parent. JS does the same check at runtime with assertIsNode; ours just happens at compile time instead.

Also worth noting: a transform like array->bytes doesn't even need to be a visitor. It's just a plain TypeNode -> TypeNode function acting on a single node. The visitor is mainly the delivery mechanism that runs it across the tree. So we can keep these transforms as small standalone functions and wrap them in a Visitor<Node> to apply them over any subtree, much like JS's transformer visitors: you hand them a list of node-to-node functions and they handle the traversal.

So my vote is to land the generic Visitor<T> + walks as the only core abstraction. If a real gap shows up once you're porting the actual transforms (array->bytes, flatten, unwrap, etc.), we can close it with a typed helper function rather than a whole parallel trait.

Regarding your 3 steps: looks good! I'd just add the visit_identity function alongside visit_merge (your walk_reduce) so we can test that design first.

What do you think?