diff --git a/crates/oxc_transformer/src/es2022/class_properties/class.rs b/crates/oxc_transformer/src/es2022/class_properties/class.rs index 242f7cad3d0cbe..50d1bbfeb9afd4 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/class.rs +++ b/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")` diff --git a/crates/oxc_transformer/src/es2022/class_properties/class_bindings.rs b/crates/oxc_transformer/src/es2022/class_properties/class_bindings.rs index efaca62c690ad0..01bfa32a216067 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/class_bindings.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/class_bindings.rs @@ -22,20 +22,19 @@ 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 @@ -43,10 +42,9 @@ pub(super) struct ClassBindings<'a> { /// Temp var for class. /// e.g. `_Class` in `_Class = class {}, _Class.x = 1, _Class` pub temp: Option>, - /// `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,12 +53,7 @@ impl<'a> ClassBindings<'a> { name_binding: Option>, temp_binding: Option>, ) -> 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. @@ -68,24 +61,35 @@ impl<'a> ClassBindings<'a> { 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>, diff --git a/crates/oxc_transformer/src/es2022/class_properties/private_field.rs b/crates/oxc_transformer/src/es2022/class_properties/private_field.rs index 48fdd0e8c0eabb..cd2f257c8d780e 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/private_field.rs +++ b/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); diff --git a/crates/oxc_transformer/src/es2022/class_properties/static_prop_init.rs b/crates/oxc_transformer/src/es2022/class_properties/static_prop_init.rs index 5dce0179336d0c..77e835a3059084 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/static_prop_init.rs +++ b/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(); diff --git a/crates/oxc_transformer/src/es2022/class_properties/supers.rs b/crates/oxc_transformer/src/es2022/class_properties/supers.rs index 1616c6810d6bdd..cbeabac882dc42 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/supers.rs +++ b/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));