refactor(transformer/class-properties): simplify logic for when to cr…

…eate temp binding

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

@@ -257,17 +257,20 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
                 .insert_many_after(&stmt_address, self.insert_after_stmts.drain(..));
         }
 
-        // Update class bindings prior to traversing class body and insertion of statements/expressions
-        // before/after the class. See comments on `ClassBindings`.
+        // Flag that static private fields should be transpiled using name binding,
+        // while traversing class body.
+        //
         // Static private fields reference class name (not temp var) in class declarations.
         // `class Class { static #prop; method() { return obj.#prop; } }`
         // -> `method() { return _assertClassBrand(Class, obj, _prop)._; }`
         // (note `Class` in `_assertClassBrand(Class, ...)`, not `_Class`)
-        // So set "temp" binding to actual class name while visiting class body.
+        //
+        // Also see comments on `ClassBindings`.
+        //
         // Note: If declaration is `export default class {}` with no name, and class has static props,
         // then class has had name binding created already in `transform_class`.
         // So name binding is always `Some`.
-        class_details.bindings.temp = class_details.bindings.name.clone();
+        class_details.bindings.static_private_fields_use_temp = false;
     }
 
     /// `_classPrivateFieldLooseKey("prop")`

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

@@ -22,31 +22,29 @@ use oxc_traverse::{BoundIdentifier, TraverseCtx};
 /// is unfortunately rather complicated.
 ///
 /// Transpiled private fields referring to a static private prop use:
-/// * Class name when field is within class body and class has a name
-///   e.g. `class C { static #x; method() { return obj.#x; } }`
-/// * Temp var when field is within class body and class has no name
-///   e.g. `C = class { static #x; method() { return obj.#x; } }`
-/// * Temp var when field is within a static prop initializer.
-///   e.g. `class C { static #x; y = obj.#x; }`
-///
-/// To cover all these cases, the meaning of `temp` binding here changes while traversing the class body.
-/// [`ClassProperties::transform_class_declaration`] sets `temp` binding to be a copy of the
-/// `name` binding before that traversal begins. So the name `temp` is misleading at that point.
 ///
-/// Debug assertions are used to make sure this complex logic is correct.
+/// * Class name when field is within body of class declaration
+///   e.g. `class C { static #x; method() { return obj.#x; } }`
+///   -> `_assertClassBrand(C, obj, _x)._`
+/// * Temp var when field is within body of class expression
+///   e.g. `C = class C { static #x; method() { return obj.#x; } }`
+///   -> `_assertClassBrand(_C, obj, _x)._`
+/// * Temp var when field is within a static prop initializer
+///   e.g. `class C { static #x; static y = obj.#x; }`
+///   -> `_assertClassBrand(_C, obj, _x)._`
 ///
-/// [`ClassProperties::transform_class_declaration`]: super::ClassProperties::transform_class_declaration
+/// `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>>,
-    /// `true` if currently transforming static property initializers.
-    /// Only used in debug builds to check logic is correct.
-    #[cfg(debug_assertions)]
-    pub currently_transforming_static_property_initializers: bool,
+    /// `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,
 }
 
 impl<'a> ClassBindings<'a> {
@@ -55,37 +53,43 @@ impl<'a> ClassBindings<'a> {
         name_binding: Option<BoundIdentifier<'a>>,
         temp_binding: Option<BoundIdentifier<'a>>,
     ) -> Self {
-        Self {
-            name: name_binding,
-            temp: temp_binding,
-            #[cfg(debug_assertions)]
-            currently_transforming_static_property_initializers: false,
-        }
+        Self { name: name_binding, temp: temp_binding, static_private_fields_use_temp: true }
     }
 
     /// Get `SymbolId` of name binding.
     pub fn name_symbol_id(&self) -> Option<SymbolId> {
         self.name.as_ref().map(|binding| binding.symbol_id)
     }
 
-    /// Create a binding for temp var, if there isn't one already.
-    pub fn get_or_init_temp_binding(&mut self, ctx: &mut TraverseCtx<'a>) -> &BoundIdentifier<'a> {
-        if self.temp.is_none() {
-            // This should only be possible if we are currently transforming static prop initializers
-            #[cfg(debug_assertions)]
-            {
-                assert!(
-                    self.currently_transforming_static_property_initializers,
-                    "Should be unreachable"
-                );
-            }
-
-            self.temp = Some(Self::create_temp_binding(self.name.as_ref(), ctx));
+    /// Get binding to use for referring to class in transpiled static private fields.
+    ///
+    /// e.g. `Class` in `_assertClassBrand(Class, object, _prop)._` (class name)
+    /// or `_Class` in `_assertClassBrand(_Class, object, _prop)._` (temp var)
+    ///
+    /// * In class expressions, this is always be temp binding.
+    /// * In class declarations, it's the name binding when code is inside class body,
+    ///   and temp binding when code is outside class body.
+    ///
+    /// `static_private_fields_use_temp` is set accordingly at the right moments
+    /// elsewhere in this transform.
+    ///
+    /// If a temp binding is required, and one doesn't already exist, a temp binding is created.
+    pub fn get_or_init_static_binding(
+        &mut self,
+        ctx: &mut TraverseCtx<'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))
+        } 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.
+            // So `unwrap` here cannot panic.
+            self.name.as_ref().unwrap()
         }
-        self.temp.as_ref().unwrap()
     }
 
-    /// Generate a binding for temp var.
+    /// Generate binding for temp var.
     pub fn create_temp_binding(
         name_binding: Option<&BoundIdentifier<'a>>,
         ctx: &mut TraverseCtx<'a>,

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

@@ -88,7 +88,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
                 Self::create_underscore_member_expression(prop_ident, span, ctx)
             } else {
                 // `_assertClassBrand(Class, object, _prop)._`
-                let class_binding = class_bindings.get_or_init_temp_binding(ctx);
+                let class_binding = class_bindings.get_or_init_static_binding(ctx);
                 let class_ident = class_binding.create_read_expression(ctx);
 
                 self.create_assert_class_brand_underscore(
@@ -282,7 +282,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
                     Self::create_underscore_member_expression(prop_ident, field_expr.span, ctx);
                 (callee, object)
             } else {
-                let class_binding = class_bindings.get_or_init_temp_binding(ctx);
+                let class_binding = class_bindings.get_or_init_static_binding(ctx);
                 let class_ident = class_binding.create_read_expression(ctx);
 
                 // Make 2 copies of `object`
@@ -383,7 +383,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
             // for shortcut/no shortcut and do the "can we shortcut?" check here.
             // Then only create temp var for the "no shortcut" branch, and clone the resulting binding
             // before passing it to the "no shortcut" method. What a palaver!
-            let class_binding = class_bindings.get_or_init_temp_binding(ctx);
+            let class_binding = class_bindings.get_or_init_static_binding(ctx);
             let class_binding = class_binding.clone();
             let class_symbol_id = class_bindings.name_symbol_id();
 
@@ -792,7 +792,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
                 let get_expr = Self::create_underscore_member_expression(prop_ident, SPAN, ctx);
                 (get_expr, object, None)
             } else {
-                let class_binding = class_bindings.get_or_init_temp_binding(ctx);
+                let class_binding = class_bindings.get_or_init_static_binding(ctx);
                 let class_ident = class_binding.create_read_expression(ctx);
                 let class_ident2 = class_binding.create_read_expression(ctx);
 

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

@@ -22,30 +22,8 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
         value: &mut Expression<'a>,
         ctx: &mut TraverseCtx<'a>,
     ) {
-        self.set_is_transforming_static_property_initializers(true);
-
         let mut replacer = StaticInitializerVisitor::new(self, ctx);
         replacer.visit_expression(value);
-
-        self.set_is_transforming_static_property_initializers(false);
-    }
-
-    /// Set flag on `ClassBindings` that we are/are not currently transforming static prop initializers.
-    ///
-    /// The logic around which bindings are used for transforming private fields is complex,
-    /// so we use this to make sure the logic is correct.
-    ///
-    /// In debug builds, `ClassBindings::get_or_init_temp_binding` will panic if we end up transforming
-    /// a static private field, and there's no `temp` binding - which should be impossible.
-    #[inline(always)] // `#[inline(always)]` because is no-op in release builds
-    #[allow(clippy::inline_always)]
-    #[cfg_attr(not(debug_assertions), expect(unused_variables, clippy::unused_self))]
-    fn set_is_transforming_static_property_initializers(&mut self, is_it: bool) {
-        #[cfg(debug_assertions)]
-        {
-            let class_details = self.current_class_mut();
-            class_details.bindings.currently_transforming_static_property_initializers = is_it;
-        }
     }
 }
 
@@ -471,7 +449,7 @@ impl<'a, 'ctx, 'v> StaticInitializerVisitor<'a, 'ctx, 'v> {
     fn replace_this_with_temp_var(&mut self, expr: &mut Expression<'a>, span: Span) {
         if self.this_depth == 0 {
             let class_details = self.class_properties.current_class_mut();
-            let temp_binding = class_details.bindings.get_or_init_temp_binding(self.ctx);
+            let temp_binding = class_details.bindings.get_or_init_static_binding(self.ctx);
             *expr = temp_binding.create_spanned_read_expression(span, self.ctx);
         }
     }
@@ -492,7 +470,7 @@ impl<'a, 'ctx, 'v> StaticInitializerVisitor<'a, 'ctx, 'v> {
         }
 
         // Identifier is reference to class name. Rename it.
-        let temp_binding = class_details.bindings.get_or_init_temp_binding(self.ctx);
+        let temp_binding = class_details.bindings.get_or_init_static_binding(self.ctx);
         ident.name = temp_binding.name.clone();
 
         let symbols = self.ctx.symbols_mut();

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

@@ -137,7 +137,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
         is_callee: bool,
         ctx: &mut TraverseCtx<'a>,
     ) -> Expression<'a> {
-        let temp_binding = self.current_class_mut().bindings.get_or_init_temp_binding(ctx);
+        let temp_binding = self.current_class_mut().bindings.get_or_init_static_binding(ctx);
 
         let ident1 = Argument::from(temp_binding.create_read_expression(ctx));
         let ident2 = Argument::from(temp_binding.create_read_expression(ctx));