Standardize conventions for transforms

[overlookmotel]

We are planning to build out a ton more transforms over next few months. The transformer is going to get a lot more complicated and unwieldy. To avoid it becoming hard to navigate and understand, I suggest that we adopt a standardized coding style to use on all transforms.

This is prompted in part from the difficulties I found in reviewing #4587.

I propose:

1. Make clear what are the "entry points" to transform

• Entry points of transform implemented as impl Traverse for MyTransform.
• Those methods have to be called enter_* and exit_*.
• Those entry points go at top of the file.
• Internal methods implemented lower down in an impl MyTransform block. Those methods can be named descriptively.

2. Encapsulate logic

All logic for each transform should live in that specific file, with no "leaking" into the parent transform. Each transform is only called into via the standard enter_*/exit_* entry points.

Only exception is that parent can check if child transform is enabled or not.

i.e. Instead of the parent being quite "fat":

oxc/crates/oxc_transformer/src/react/mod.rs

Lines 75 to 87 in a9c729a

	pub fn transform_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
	if self.jsx_plugin {
	match expr {
	Expression::JSXElement(e) => {
	*expr = self.jsx.transform_jsx_element(e, ctx);
	}
	Expression::JSXFragment(e) => {
	*expr = self.jsx.transform_jsx_fragment(e, ctx);
	}
	_ => {}
	}
	}
	}

Move the JSX-related logic into jsx.rs:

// src/react/mod.rs
fn enter_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) { 
     if self.jsx_plugin {
         self.jsx.enter_expression(self, expr, ctx);
     } 
 }

// src/react/jsx.rs
fn enter_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
    match expr {
        Expression::JSXElement(e) => {
            *expr = self.transform_jsx_element(e, ctx);
        }
        Expression::JSXFragment(e) => {
            *expr = self.transform_jsx_fragment(e, ctx);
        }
        _ => {}
    }
}

3. Comments

Top of file

Each transform should include comment at top of file including:

• High level explanation of what transform does.
• One "before / after" example.
• Link to Babel plugin.
• Note of any ways in which our implementation diverges from Babel's, and why.

Methods

If it's a complicated transform with multiple visitors which interact with each other, add comments explaining how the pieces fit together.

Code snippets

AstBuilder calls are often very verbose. Preface each chunk of AstBuilder calls with a short comment of what this code produces. e.g.:

// `let Foo;`
let declarations = {
    let ident = BindingIdentifier::new(SPAN, "Foo");
    let pattern_kind = self.ast.binding_pattern_identifier(ident);
    let binding = self.ast.binding_pattern(pattern_kind, None, false);
    let decl = self.ast.variable_declarator(SPAN, VariableDeclarationKind::Let, binding, None, false);
    self.ast.new_vec_single(decl)
};
let var_decl = Declaration::VariableDeclaration(self.ast.variable_declaration(
    SPAN,
    kind,
    declarations,
    Modifiers::empty(),
));

Where we can improve on Babel

Where we feel Babel's implementation is inefficient, but we have to follow it at present to pass their tests, make a // TODO(improve-on-babel): Babel's impl is poor because X, we could do better by Y comment.

Boring!

Sorry this is rather boring! Much of the above we're doing already, but I feel it's worth the effort now to formalize some conventions and then go "by the book". It's tedious, but I feel that if we're a bit "stiff" now, we'll benefit in long run from a more maintainable and contributor-friendly transformer.

My suggestions above are not set in stone. If anyone would like to propose different conventions, go ahead. I just feel we should have some conventions, and stick to them.

[Boshen]

TODO: add this to crate README