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.
@@ -35,31 +38,47 @@ 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,
     /// `true` if temp var for class has been inserted
     pub temp_var_is_created: bool,
 }
 
+impl<'a> Default for ClassBindings<'a> {
+    fn default() -> Self {
+        Self {
+            name: None,
+            temp: None,
+            outer_hoist_scope_id: ScopeId::new(0),
+            static_private_fields_use_temp: false,
+            temp_var_is_created: false,
+        }
+    }
+}
+
 impl<'a> ClassBindings<'a> {
     /// Create `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,
         }
     }
@@ -88,7 +107,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 +121,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

@@ -12,6 +12,8 @@ use super::{ClassBindings, ClassProperties, FxIndexMap};
 pub(super) struct ClassDetails<'a> {
     /// `true` for class declaration, `false` for class expression
     pub is_declaration: bool,
+    /// `true` if class requires no transformation
+    pub 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`.

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.
+    /// * Replace old key with `_x = x()` assignment.
     /// * Return `_x`.
     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 1st pass, computed keys for instance properties are converted to assignments to temp vars.
+    /// `class C { [foo()] = 123 }`
+    /// -> `class C { [_foo = foo()] = null; constructor() { this[_foo] = 123; } }`
+    ///
+    /// Now in 2nd pass, extract this assignment and move it to before class, and replace with
+    /// reference to temp var.
+    ///
+    /// `class C { [_foo = foo()] = null; constructor() { this[_foo] = 123; } }`
+    /// -> `_foo = foo(); class C { [_foo] = null; constructor() { this[_foo] = 123; } }`
+    ///
+    /// We do this in 2 passes so that the computed key is still present within the class during
+    /// traversal of the class body, and any other transforms necessary 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!(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);
     }
 }
 

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

@@ -207,54 +207,17 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
         (insert_scopes, insert_location)
     }
 
-    /// Insert instance property initializers.
-    pub(super) fn insert_instance_inits(
-        &mut self,
-        class: &mut Class<'a>,
-        inits: Vec<Expression<'a>>,
-        insertion_location: &InstanceInitsInsertLocation<'a>,
-        constructor_index: usize,
-        ctx: &mut TraverseCtx<'a>,
-    ) {
-        // TODO: Handle private props in constructor params `class C { #x; constructor(x = this.#x) {} }`.
-
-        match insertion_location {
-            InstanceInitsInsertLocation::NewConstructor => {
-                self.insert_constructor(class, inits, ctx);
-            }
-            InstanceInitsInsertLocation::ExistingConstructor(stmt_index) => {
-                self.insert_inits_into_constructor_as_statements(
-                    class,
-                    inits,
-                    constructor_index,
-                    *stmt_index,
-                    ctx,
-                );
-            }
-            InstanceInitsInsertLocation::SuperFnInsideConstructor(super_binding) => {
-                self.create_super_function_inside_constructor(
-                    class,
-                    inits,
-                    super_binding,
-                    constructor_index,
-                    ctx,
-                );
-            }
-            InstanceInitsInsertLocation::SuperFnOutsideClass(super_binding) => {
-                self.create_super_function_outside_constructor(inits, super_binding, ctx);
-            }
-        }
-    }
+    // TODO: Handle private props in constructor params `class C { #x; constructor(x = this.#x) {} }`.
 
     /// Add a constructor to class containing property initializers.
-    fn insert_constructor(
+    pub(super) fn insert_constructor(
         &self,
-        class: &mut Class<'a>,
+        body: &mut ClassBody<'a>,
         inits: Vec<Expression<'a>>,
+        has_super_class: bool,
         ctx: &mut TraverseCtx<'a>,
     ) {
-        // Create statements to go in function body.
-        let has_super_class = class.super_class.is_some();
+        // Create statements to go in function body
         let mut stmts = ctx.ast.vec_with_capacity(inits.len() + usize::from(has_super_class));
 
         // Add `super(..._args);` statement and `..._args` param if class has a super class.
@@ -307,20 +270,18 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
         ));
 
         // TODO(improve-on-babel): Could push constructor onto end of elements, instead of inserting as first
-        class.body.body.insert(0, ctor);
+        body.body.insert(0, ctor);
     }
 
     /// Insert instance property initializers into constructor body at `insertion_index`.
-    fn insert_inits_into_constructor_as_statements(
+    pub(super) fn insert_inits_into_constructor_as_statements(
         &mut self,
-        class: &mut Class<'a>,
+        constructor: &mut Function<'a>,
         inits: Vec<Expression<'a>>,
-        constructor_index: usize,
         insertion_index: usize,
         ctx: &mut TraverseCtx<'a>,
     ) {
         // Rename any symbols in constructor which clash with references in inits
-        let constructor = Self::get_constructor(class, constructor_index);
         self.rename_clashing_symbols(constructor, ctx);
 
         // Insert inits into constructor body
@@ -331,16 +292,14 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
     /// Create `_super` function containing instance property initializers,
     /// and insert at top of constructor body.
     /// `var _super = (..._args) => (super(..._args), <inits>, this);`
-    fn create_super_function_inside_constructor(
+    pub(super) fn create_super_function_inside_constructor(
         &mut self,
-        class: &mut Class<'a>,
+        constructor: &mut Function<'a>,
         inits: Vec<Expression<'a>>,
         super_binding: &BoundIdentifier<'a>,
-        constructor_index: usize,
         ctx: &mut TraverseCtx<'a>,
     ) {
         // Rename any symbols in constructor which clash with references in inits
-        let constructor = Self::get_constructor(class, constructor_index);
         self.rename_clashing_symbols(constructor, ctx);
 
         // `(super(..._args), <inits>, this)`
@@ -405,14 +364,15 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
     /// Create `_super` function containing instance property initializers,
     /// and insert it outside class.
     /// `let _super = function() { <inits>; return this; }`
-    fn create_super_function_outside_constructor(
+    pub(super) fn create_super_function_outside_constructor(
         &mut self,
         inits: Vec<Expression<'a>>,
         super_binding: &BoundIdentifier<'a>,
         ctx: &mut TraverseCtx<'a>,
     ) {
         // Add `"use strict"` directive if outer scope is not strict mode
-        let directives = if ctx.current_scope_flags().is_strict_mode() {
+        let outer_scope_id = ctx.current_block_scope_id();
+        let directives = if ctx.scopes().get_flags(outer_scope_id).is_strict_mode() {
             ctx.ast.vec()
         } else {
             ctx.ast.vec1(ctx.ast.use_strict_directive())
@@ -490,20 +450,6 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
         // Empty `clashing_constructor_symbols` hashmap for reuse on next class
         clashing_symbols.clear();
     }
-
-    /// Get `Function` for constructor, given constructor's index within class elements.
-    fn get_constructor<'b>(
-        class: &'b mut Class<'a>,
-        constructor_index: usize,
-    ) -> &'b mut Function<'a> {
-        let Some(ClassElement::MethodDefinition(method)) =
-            class.body.body.get_mut(constructor_index)
-        else {
-            unreachable!()
-        };
-        debug_assert!(method.kind == MethodDefinitionKind::Constructor);
-        &mut method.value
-    }
 }
 
 /// Visitor for transforming `super()` in class constructor params.
@@ -550,7 +496,7 @@ impl<'a, 'c> ConstructorParamsSuperReplacer<'a, 'c> {
         let insert_location = InstanceInitsInsertLocation::SuperFnOutsideClass(super_binding);
 
         // Create scope for `_super` function
-        let outer_scope_id = self.ctx.current_scope_id();
+        let outer_scope_id = self.ctx.current_block_scope_id();
         let super_func_scope_id = self.ctx.scopes_mut().add_scope(
             Some(outer_scope_id),
             NodeId::DUMMY,
@@ -631,7 +577,7 @@ impl<'a, 'c> ConstructorParamsSuperReplacer<'a, 'c> {
         let super_binding = self.super_binding.get_or_insert_with(|| {
             self.ctx.generate_uid(
                 "super",
-                self.ctx.current_scope_id(),
+                self.ctx.current_block_scope_id(),
                 SymbolFlags::BlockScopedVariable,
             )
         });