fix(transformer/class-properties): run other transforms on static pro…

…perties, static blocks, and computed keys (#7982)

Large re-architecting of class properties transform. Split transform into 3 phases:

1. Transform instance properties when entering class.
2. Transform private fields during traversal of class body.
3. Transform static properties and static blocks when exiting class.

This ensures that code which has to be moved outside of the class (static property initializers, static blocks, computed keys) can get transformed by other transforms before they're moved out.

Also fixes a problem where we previously registered private properties too early - on entering the class, rather than the class *body*, so private fields in `extends` clause of a nested class were misinterpretted.

crates/oxc_transformer/src/es2022/class_properties/class_bindings.rs

@@ -1,4 +1,7 @@
-use oxc_syntax::symbol::SymbolId;
+use oxc_syntax::{
+    scope::ScopeId,
+    symbol::{SymbolFlags, SymbolId},
+};
 use oxc_traverse::{BoundIdentifier, TraverseCtx};
 
 /// Store for bindings for class.
@@ -9,12 +12,12 @@ use oxc_traverse::{BoundIdentifier, TraverseCtx};
 /// Temp var is required in the following circumstances:
 ///
 /// * Class expression has static properties.
-///   e.g. `C = class { x = 1; }`
+///   e.g. `C = class { static x = 1; }`
 /// * Class declaration has static properties and one of the static prop's initializers contains:
 ///   a. `this`
-///      e.g. `class C { x = this; }`
+///      e.g. `class C { static x = this; }`
 ///   b. Reference to class name
-///      e.g. `class C { x = C; }`
+///      e.g. `class C { static x = C; }`
 ///   c. A private field referring to one of the class's static private props.
 ///      e.g. `class C { static #x; static y = obj.#x; }`
 ///
@@ -35,13 +38,14 @@ use oxc_traverse::{BoundIdentifier, TraverseCtx};
 ///
 /// `static_private_fields_use_temp` is updated as transform moves through the class,
 /// to indicate which binding to use.
-#[derive(Default, Clone)]
 pub(super) struct ClassBindings<'a> {
     /// Binding for class name, if class has name
     pub name: Option<BoundIdentifier<'a>>,
     /// Temp var for class.
     /// e.g. `_Class` in `_Class = class {}, _Class.x = 1, _Class`
     pub temp: Option<BoundIdentifier<'a>>,
+    /// `ScopeId` of hoist scope outside class (which temp `var` binding would be created in)
+    pub outer_hoist_scope_id: ScopeId,
     /// `true` if should use temp binding for references to class in transpiled static private fields,
     /// `false` if can use name binding
     pub static_private_fields_use_temp: bool,
@@ -50,20 +54,30 @@ pub(super) struct ClassBindings<'a> {
 }
 
 impl<'a> ClassBindings<'a> {
-    /// Create `ClassBindings`.
+    /// Create new `ClassBindings`.
     pub fn new(
         name_binding: Option<BoundIdentifier<'a>>,
         temp_binding: Option<BoundIdentifier<'a>>,
+        outer_scope_id: ScopeId,
+        static_private_fields_use_temp: bool,
         temp_var_is_created: bool,
     ) -> Self {
         Self {
             name: name_binding,
             temp: temp_binding,
-            static_private_fields_use_temp: true,
+            outer_hoist_scope_id: outer_scope_id,
+            static_private_fields_use_temp,
             temp_var_is_created,
         }
     }
 
+    /// Create dummy `ClassBindings`.
+    ///
+    /// Used when class needs no transform, and for dummy entry at top of `ClassesStack`.
+    pub fn dummy() -> Self {
+        Self::new(None, None, ScopeId::new(0), false, false)
+    }
+
     /// Get `SymbolId` of name binding.
     pub fn name_symbol_id(&self) -> Option<SymbolId> {
         self.name.as_ref().map(|binding| binding.symbol_id)
@@ -88,7 +102,9 @@ impl<'a> ClassBindings<'a> {
     ) -> &BoundIdentifier<'a> {
         if self.static_private_fields_use_temp {
             // Create temp binding if doesn't already exist
-            self.temp.get_or_insert_with(|| Self::create_temp_binding(self.name.as_ref(), ctx))
+            self.temp.get_or_insert_with(|| {
+                Self::create_temp_binding(self.name.as_ref(), self.outer_hoist_scope_id, ctx)
+            })
         } else {
             // `static_private_fields_use_temp` is always `true` for class expressions.
             // Class declarations always have a name binding if they have any static props.
@@ -100,12 +116,13 @@ impl<'a> ClassBindings<'a> {
     /// Generate binding for temp var.
     pub fn create_temp_binding(
         name_binding: Option<&BoundIdentifier<'a>>,
+        outer_hoist_scope_id: ScopeId,
         ctx: &mut TraverseCtx<'a>,
     ) -> BoundIdentifier<'a> {
         // Base temp binding name on class name, or "Class" if no name.
         // TODO(improve-on-babel): If class name var isn't mutated, no need for temp var for
         // class declaration. Can just use class binding.
         let name = name_binding.map_or("Class", |binding| binding.name.as_str());
-        ctx.generate_uid_in_current_hoist_scope(name)
+        ctx.generate_uid(name, outer_hoist_scope_id, SymbolFlags::FunctionScopedVariable)
     }
 }

crates/oxc_transformer/src/es2022/class_properties/class_details.rs

@@ -8,10 +8,11 @@ use super::{ClassBindings, ClassProperties, FxIndexMap};
 /// Details of a class.
 ///
 /// These are stored in `ClassesStack`.
-#[derive(Default)]
 pub(super) struct ClassDetails<'a> {
     /// `true` for class declaration, `false` for class expression
     pub is_declaration: bool,
+    /// `true` if class requires no transformation
+    pub is_transform_required: bool,
     /// Private properties.
     /// Mapping private prop name to binding for temp var.
     /// This is then used as lookup when transforming e.g. `this.#x`.
@@ -21,13 +22,28 @@ pub(super) struct ClassDetails<'a> {
     pub bindings: ClassBindings<'a>,
 }
 
+impl<'a> ClassDetails<'a> {
+    /// Create empty `ClassDetails`.
+    ///
+    /// Used when class needs no transform, and for dummy entry at top of `ClassesStack`.
+    pub fn empty(is_declaration: bool) -> Self {
+        Self {
+            is_declaration,
+            is_transform_required: false,
+            private_props: None,
+            bindings: ClassBindings::dummy(),
+        }
+    }
+}
+
 /// Details of a private property.
 pub(super) struct PrivateProp<'a> {
     pub binding: BoundIdentifier<'a>,
     pub is_static: bool,
 }
 
 /// Stack of `ClassDetails`.
+///
 /// Pushed to when entering a class, popped when exiting.
 ///
 /// We use a `NonEmptyStack` to make `last` and `last_mut` cheap (these are used a lot).
@@ -37,12 +53,17 @@ pub(super) struct PrivateProp<'a> {
 /// to work around borrow-checker. You can call `find_private_prop` and retain the return value
 /// without holding a mut borrow of the whole of `&mut ClassProperties`. This allows accessing other
 /// properties of `ClassProperties` while that borrow is held.
-#[derive(Default)]
 pub(super) struct ClassesStack<'a> {
     stack: NonEmptyStack<ClassDetails<'a>>,
 }
 
 impl<'a> ClassesStack<'a> {
+    /// Create new `ClassesStack`.
+    pub fn new() -> Self {
+        // Default stack capacity is 4. That's is probably good. More than 4 nested classes is rare.
+        Self { stack: NonEmptyStack::new(ClassDetails::empty(false)) }
+    }
+
     /// Push an entry to stack.
     #[inline]
     pub fn push(&mut self, class: ClassDetails<'a>) {

crates/oxc_transformer/src/es2022/class_properties/computed_key.rs

@@ -34,8 +34,9 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
         // 3. At least one property satisfying the above is after this method,
         //    or class contains a static block which is being transformed
         //    (static blocks are always evaluated after computed keys, regardless of order)
-        let key = ctx.ast.move_expression(key);
-        let temp_var = self.create_computed_key_temp_var(key, ctx);
+        let original_key = ctx.ast.move_expression(key);
+        let (assignment, temp_var) = self.create_computed_key_temp_var(original_key, ctx);
+        self.insert_before.push(assignment);
         method.key = PropertyKey::from(temp_var);
     }
 
@@ -52,43 +53,89 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
     /// This function:
     /// * Creates the `let _x;` statement and inserts it.
     /// * Creates the `_x = x()` assignment.
-    /// * Inserts assignment before class.
+    /// * If static prop, inserts assignment before class.
+    /// * If instance prop, replaces existing key with assignment (it'll be moved to before class later).
     /// * Returns `_x`.
     pub(super) fn create_computed_key_temp_var_if_required(
         &mut self,
         key: &mut Expression<'a>,
+        is_static: bool,
         ctx: &mut TraverseCtx<'a>,
     ) -> Expression<'a> {
-        let key = ctx.ast.move_expression(key);
-        if key_needs_temp_var(&key, ctx) {
-            self.create_computed_key_temp_var(key, ctx)
+        let original_key = ctx.ast.move_expression(key);
+        if key_needs_temp_var(&original_key, ctx) {
+            let (assignment, ident) = self.create_computed_key_temp_var(original_key, ctx);
+            if is_static {
+                self.insert_before.push(assignment);
+            } else {
+                *key = assignment;
+            }
+            ident
         } else {
-            key
+            original_key
         }
     }
 
-    /// * Create `let _x;` statement and insert it.
-    /// * Create `_x = x()` assignment.
-    /// * Insert assignment before class.
-    /// * Return `_x`.
+    /// Create `let _x;` statement and insert it.
+    /// Return `_x = x()` assignment, and `_x` identifier referencing same temp var.
     fn create_computed_key_temp_var(
         &mut self,
         key: Expression<'a>,
         ctx: &mut TraverseCtx<'a>,
-    ) -> Expression<'a> {
-        // We entered transform via `enter_expression` or `enter_statement`,
-        // so `ctx.current_scope_id()` is the scope outside the class
-        let parent_scope_id = ctx.current_scope_id();
+    ) -> (/* assignment */ Expression<'a>, /* identifier */ Expression<'a>) {
+        let outer_scope_id = ctx.current_block_scope_id();
         // TODO: Handle if is a class expression defined in a function's params.
         let binding =
-            ctx.generate_uid_based_on_node(&key, parent_scope_id, SymbolFlags::BlockScopedVariable);
+            ctx.generate_uid_based_on_node(&key, outer_scope_id, SymbolFlags::BlockScopedVariable);
 
         self.ctx.var_declarations.insert_let(&binding, None, ctx);
 
         let assignment = create_assignment(&binding, key, ctx);
-        self.insert_before.push(assignment);
+        let ident = binding.create_read_expression(ctx);
+
+        (assignment, ident)
+    }
+
+    /// Extract computed key if it's an assignment, and replace with identifier.
+    ///
+    /// In entry phase, computed keys for instance properties are converted to assignments to temp vars.
+    /// `class C { [foo()] = 123 }`
+    /// -> `class C { [_foo = foo()]; constructor() { this[_foo] = 123; } }`
+    ///
+    /// Now in exit phase, extract this assignment and move it to before class.
+    ///
+    /// `class C { [_foo = foo()]; constructor() { this[_foo] = 123; } }`
+    /// -> `_foo = foo(); class C { [null]; constructor() { this[_foo] = 123; } }`
+    /// (`[null]` property will be removed too by caller)
+    ///
+    /// We do this process in 2 passes so that the computed key is still present within the class during
+    /// traversal of the class body, so any other transforms can run on it.
+    /// Now that we're exiting the class, we can move the assignment `_foo = foo()` out of the class
+    /// to where it needs to be.
+    pub(super) fn extract_instance_prop_computed_key(
+        &mut self,
+        prop: &mut PropertyDefinition<'a>,
+        ctx: &TraverseCtx<'a>,
+    ) {
+        // Exit if computed key is not an assignment (wasn't processed in 1st pass).
+        let PropertyKey::AssignmentExpression(assign_expr) = &prop.key else { return };
+
+        // Debug checks that we're removing what we think we are
+        #[cfg(debug_assertions)]
+        {
+            assert!(assign_expr.span.is_empty());
+            let AssignmentTarget::AssignmentTargetIdentifier(ident) = &assign_expr.left else {
+                unreachable!();
+            };
+            assert!(ident.name.starts_with('_'));
+            assert!(ctx.symbols().get_reference(ident.reference_id()).symbol_id().is_some());
+            assert!(ident.span.is_empty());
+            assert!(prop.value.is_none());
+        }
 
-        binding.create_read_expression(ctx)
+        // Extract assignment from computed key and insert before class
+        let assignment = ctx.ast.move_property_key(&mut prop.key).into_expression();
+        self.insert_before.push(assignment);
     }
 }