oxc icon indicating copy to clipboard operation
oxc copied to clipboard

Add scopes support to `Traverse` / transformer

Open overlookmotel opened this issue 1 year ago • 5 comments

The intent is to replace VisitMut in transformer with the new oxc_traverse::Traverse. #3182 is a starting point for that.

However, transformers require scope information, which is not yet implemented.

VisitMut provides only fairly minimal information - just ScopeFlags indicating:

  • What kind of environment visitor is in (top level / function / method / class static block / etc).
  • Whether strict mode or not.

This would be fairly easy to replicate in Traverse.

However, I assume transformers actually need more info than this, probably more like the info available from semantic. Opening this issue to discuss:

  1. What scope info transformers need.
  2. How to implement it.

overlookmotel avatar May 07 '24 10:05 overlookmotel

What scope info transformers need.

All we need are the scope flags.

During traversing, we keep a stack of scopes, and manage binding information while entering / leaving scopes.

How to implement it.

The coolest approach is to treat scope as an individual AST node, something like

struct Scope<T> {
  inner: T
}

pub struct TryStatement<'a> {
    pub block: Box<'a, Scope<BlockStatement<'a>>>,
    pub handler: Option<Box<'a, Scope<CatchClause<'a>>>>,
    pub finalizer: Option<Box<'a, Scope<BlockStatement<'a>>>>,
}

But actually I've got no idea on how to implement it until I get my hands dirty on the new traverse API.

Boshen avatar May 07 '24 10:05 Boshen

If that's all we need, how about TraverseCtx containing stack of ScopeFlags which it pushes to when entering a scope, and pops off when leaving it?

Traverse can handle updating whether context is strict mode or not itself, including inheriting previous state. No need for enter_scope and exit_scope to do that.

You'd use it as follows:

fn enter_binary_expression(&mut self, fn: &mut BinaryExpression<'a>, ctx: &TraverseCtx<'a>) {
  if ctx.scope.is_strict_mode() {
    // ...
  }
}

We could signal to the codegen when to update the scope stack with something like:

#[visited_node(scope = "Function")]
pub struct Function<'a> {
    pub r#type: FunctionType,
    // ...
}

NB: #[visited_node] macro is a no-op. It passes through the input unchanged without even parsing it, so is very cheap. It's just a way to convey information to the codegen which generates Traverse trait and walk_* functions etc.

overlookmotel avatar May 07 '24 12:05 overlookmotel

I need to store things associated with a scope, but I think I can work with what you proposed to get things started.

Boshen avatar May 07 '24 12:05 Boshen

Actually you don't need ScopeFlags to answer the question "am I in a function?".

Something like this would do the same:

let in_function = ctx.find_ancestor(
  |ancestor| if ancestor.is_function() || ancestor.is_arrow_function_expression() {
    FinderRet::Found(())
  } else {
    FinderRet::Continue
  }
).is_some();

ScopeFlags is worth implementing to make this cheaper if it's a question we need to ask a lot in visitors. I assume it is, right?

overlookmotel avatar May 07 '24 13:05 overlookmotel

Related to #2859.

overlookmotel avatar May 07 '24 19:05 overlookmotel

#3314 gave Traverse full scopes support.

overlookmotel avatar May 16 '24 22:05 overlookmotel