oxc icon indicating copy to clipboard operation
oxc copied to clipboard

Alternative API for getting `TraverseCtx` in

Open overlookmotel opened this issue 9 months ago • 3 comments

@Dunqing was asking in #3216 for a more ergonomic way to pass TraverseCtx around in transformers.

#3219 is one option. Here's another, which is more like Babel's Path API:

Instead of receiving AST node and TraverseCtx, you'd only get Path, and have to get the AST node from it:

impl<'a> Traverse<'a> for MyTransform {
  fn enter_binary_expression(&mut self, path: &mut Path) {
    let bin_expr: &mut BinaryExpression<'a> = path.node();
    // Do stuff with `bin_expr`
  }
}

Is that any better?

Personally I don't much like it - .node() is boilerplate for every single enter_* or exit_* method.

BUT if we want to support mutating upwards, we'd need an API something like that anyway. That'd work something like this:

impl<'a> Traverse<'a> for MyTransform {
  fn enter_binary_expression(&mut self, path: &mut Path) {
    match path.parent_mut() {
      AncestorMut::ExpressionStatement(expr_stmt) => {
        // `expr_stmt` is a `&mut ExpressionStatement` - you can mutate it
      }
    }
  }
}

With upwards mutation, the function params can't include a reference to the current node, as it would be UB to also get a &mut ref to parent at same time. Both current AST node and parent AST node have to both come from path, so path can enforce that you can only get one or the other at any time.

NB: I am not 100% sure upward mutation can be made safe, but I think it can with an API like the above.

overlookmotel avatar May 13 '24 17:05 overlookmotel

I like this API.

Instead of receiving AST node and TraverseCtx, you'd only get Path, and have to get the AST node from it:

impl<'a> Traverse<'a> for MyTransform {
  fn enter_binary_expression(&mut self, path: &mut Path) {
    let bin_expr: &mut BinaryExpression<'a> = path.node();
    // Do stuff with `bin_expr`
  }
}

But your example looks incorrect. What do I get when I call path.node()?

Dunqing avatar May 14 '24 02:05 Dunqing

Path is a generic Path<'a, T>. I am hoping that Rust would allow eliding T, but maybe I'm wrong about that - maybe you can only elide lifetimes, not generic types.

With the full function signature, it would be:

impl<'a> Traverse<'a> for MyTransform {
  fn enter_binary_expression(&mut self, path: &mut Path<'a, BinaryExpression<'a>>) {
    let bin_expr = path.node();
    // Do stuff with `bin_expr`
  }
}

bin_expr is a &mut BinaryExpression<'a> - same as what gets passed as node param in current API.

overlookmotel avatar May 14 '24 10:05 overlookmotel

I can try and build it so we can evaluate it more clearly - should not take long to do that.

overlookmotel avatar May 14 '24 10:05 overlookmotel