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

…perties, static blocks, and computed keys

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);
     }
 }