fix(transformer/class-properties): replace references to class name w…

…ith temp var in static prop initializers (#7610)

Similar to #7516. Fix class properties transform to replace references to class name in static prop initializers with temp var.

Input:

```js
class C {
  static getSelf = () => C;
}
const C2 = C;
C = 123;
assert(C2.getSelf() === C);
```

Output:

```js
var _C;
class C {}
_C = C;
_defineProperty(C, "getSelf", () => _C);
const C2 = C;
C = 123;
assert(C2.getSelf() === C);
```

Previously, temp var wasn't used so code was `_defineProperty(C, "getSelf", () => C);`. `C` is altered later by `C = 123`, so `C2.getSelf()` returned `123`, instead of reference to the class.

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

@@ -6,22 +6,20 @@ use oxc_ast::{
     visit::{walk_mut, VisitMut},
 };
 use oxc_syntax::scope::ScopeFlags;
-use oxc_traverse::TraverseCtx;
+use oxc_traverse::{BoundIdentifier, TraverseCtx};
 
 use super::ClassProperties;
 
 impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
     /// Transform static property initializer.
     ///
-    /// Replace `this` with temp var for class. Transform private fields.
+    /// Replace `this`, and references to class name, with temp var for class. Transform private fields.
     /// See below for full details of transforms.
     pub(super) fn transform_static_initializer(
         &mut self,
         value: &mut Expression<'a>,
         ctx: &mut TraverseCtx<'a>,
     ) {
-        // TODO: Replace references to class name with temp var
-
         self.set_is_transforming_static_property_initializers(true);
 
         let mut replacer = StaticInitializerVisitor::new(self, ctx);
@@ -58,8 +56,13 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
 ///    * Class declaration: `class C { static x = this.y; }`
 ///      -> `var _C; class C {}; _C = C; C.x = _C.y;`
 ///    * Class expression: `x = class C { static x = this.y; }`
-///      -> `var _C; x = (_C = class C {}, _C.x = _C.y)`
-/// 2. Private fields which refer to private props of this class.
+///      -> `var _C; x = (_C = class C {}, _C.x = _C.y, _C)`
+/// 2. Reference to class name to class temp var.
+///    * Class declaration: `class C { static x = C.y; }`
+///      -> `var _C; class C {}; _C = C; C.x = _C.y;`
+///    * Class expression: `x = class C { static x = C.y; }`
+///      -> `var _C; x = (_C = class C {}, _C.x = _C.y, _C)`
+/// 3. Private fields which refer to private props of this class.
 ///    * Class declaration: `class C { static #x = 123; static y = this.#x; }`
 ///      -> `var _C; class C {}; _C = C; var _x = { _: 123 }; C.y = _assertClassBrand(_C, _C, _x)._;`
 ///    * Class expression: `x = class C { static #x = 123; static y = this.#x; }`
@@ -70,19 +73,38 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
 /// class body. So we need to transform them.
 ///
 /// Note that for class declarations, assignments are made to properties of original class name `C`,
-/// but temp var `_C` is used in replacements for `this` and private fields. This is because class binding
-/// `C` can be mutated, and the initializer may contain functions which are not executed immediately.
+/// but temp var `_C` is used in replacements for `this` or class name, and private fields.
+/// This is because class binding `C` could be mutated, and the initializer may contain functions which
+/// are not executed immediately, so the mutation occurs before that initializer code runs.
+///
+/// ```js
+/// class C {
+///   static getSelf = () => this;
+///   static getSelf2 = () => C;
+/// }
+/// const C2 = C;
+/// C = 123;
+/// assert(C2.getSelf() === C); // Would fail if `this` was replaced with `C`, instead of temp var
+/// assert(C2.getSelf2() === C); // Would fail if `C` in `getSelf2` was not replaced with temp var
+/// ```
 ///
-/// If this class defines no private properties, we only need to transform `this`, so can skip traversing
-/// into functions and other contexts which have their own `this`.
+/// If this class defines no private properties and class has no name, we only need to transform `this`,
+/// so can skip traversing into functions and other contexts which have their own `this`.
 ///
 /// Note: Those functions could contain private fields referring to a *parent* class's private props,
 /// but we don't need to transform them here as they remain in same class scope.
 //
+// TODO(improve-on-babel): Unnecessary to create temp var for class declarations if either:
+// 1. Class name binding is not mutated.
+// 2. `this` / reference to class name / private field is not in a nested function, so we know the
+//    code runs immediately, before any mutation of the class name binding can occur.
+//
 // TODO: Also re-parent child scopes.
+// TODO: Alter scope flags on all scopes to remove `StrictMode`, if outside class is sloppy mode.
+// Or fix this properly by wrapping all static prop initializers in a strict mode arrow function IIFE.
 struct StaticInitializerVisitor<'a, 'ctx, 'v> {
-    /// `true` if class has private properties.
-    class_has_private_props: bool,
+    /// `true` if class has name or private properties.
+    class_has_name_or_private_props: bool,
     /// Incremented when entering a different `this` context, decremented when exiting it.
     /// `this` should be transformed when `this_depth == 0`.
     this_depth: u32,
@@ -98,7 +120,8 @@ impl<'a, 'ctx, 'v> StaticInitializerVisitor<'a, 'ctx, 'v> {
         ctx: &'v mut TraverseCtx<'a>,
     ) -> Self {
         Self {
-            class_has_private_props: class_properties.private_props_stack.last().is_some(),
+            class_has_name_or_private_props: class_properties.class_bindings.name.is_some()
+                || class_properties.private_props_stack.last().is_some(),
             this_depth: 0,
             class_properties,
             ctx,
@@ -182,10 +205,16 @@ impl<'a, 'ctx, 'v> VisitMut<'a> for StaticInitializerVisitor<'a, 'ctx, 'v> {
 
     #[inline]
     fn visit_assignment_target(&mut self, target: &mut AssignmentTarget<'a>) {
+        // `[object.#prop] = []`
         self.class_properties.transform_assignment_target(target, self.ctx);
         walk_mut::walk_assignment_target(self, target);
     }
 
+    /// Transform reference to class name to temp var
+    fn visit_identifier_reference(&mut self, ident: &mut IdentifierReference<'a>) {
+        self.replace_class_name_with_temp_var(ident);
+    }
+
     // Increment `this_depth` when entering code where `this` refers to a different `this`
     // from `this` within this class, and decrement it when exiting.
     // Therefore `this_depth == 0` when `this` refers to the `this` which needs to be transformed.
@@ -194,7 +223,7 @@ impl<'a, 'ctx, 'v> VisitMut<'a> for StaticInitializerVisitor<'a, 'ctx, 'v> {
     // need to be transformed, so no point searching for them.
     #[inline]
     fn visit_function(&mut self, func: &mut Function<'a>, flags: ScopeFlags) {
-        if self.class_has_private_props {
+        if self.class_has_name_or_private_props {
             self.this_depth += 1;
             walk_mut::walk_function(self, func, flags);
             self.this_depth -= 1;
@@ -203,7 +232,7 @@ impl<'a, 'ctx, 'v> VisitMut<'a> for StaticInitializerVisitor<'a, 'ctx, 'v> {
 
     #[inline]
     fn visit_static_block(&mut self, block: &mut StaticBlock<'a>) {
-        if self.class_has_private_props {
+        if self.class_has_name_or_private_props {
             self.this_depth += 1;
             walk_mut::walk_static_block(self, block);
             self.this_depth -= 1;
@@ -212,7 +241,7 @@ impl<'a, 'ctx, 'v> VisitMut<'a> for StaticInitializerVisitor<'a, 'ctx, 'v> {
 
     #[inline]
     fn visit_ts_module_block(&mut self, block: &mut TSModuleBlock<'a>) {
-        if self.class_has_private_props {
+        if self.class_has_name_or_private_props {
             self.this_depth += 1;
             walk_mut::walk_ts_module_block(self, block);
             self.this_depth -= 1;
@@ -236,7 +265,7 @@ impl<'a, 'ctx, 'v> VisitMut<'a> for StaticInitializerVisitor<'a, 'ctx, 'v> {
             self.visit_property_key(&mut prop.key);
         }
 
-        if self.class_has_private_props {
+        if self.class_has_name_or_private_props {
             if let Some(value) = &mut prop.value {
                 self.this_depth += 1;
                 self.visit_expression(value);
@@ -254,7 +283,7 @@ impl<'a, 'ctx, 'v> VisitMut<'a> for StaticInitializerVisitor<'a, 'ctx, 'v> {
             self.visit_property_key(&mut prop.key);
         }
 
-        if self.class_has_private_props {
+        if self.class_has_name_or_private_props {
             if let Some(value) = &mut prop.value {
                 self.this_depth += 1;
                 self.visit_expression(value);
@@ -268,23 +297,53 @@ impl<'a, 'ctx, 'v> StaticInitializerVisitor<'a, 'ctx, 'v> {
     /// Replace `this` with reference to temp var for class.
     fn replace_this_with_temp_var(&mut self, expr: &mut Expression<'a>, span: Span) {
         if self.this_depth == 0 {
-            // `PrivateProps` is the source of truth for bindings if class has private props
-            // because other visitors which transform private fields may create a temp binding
-            // and store it on `PrivateProps`
-            let class_bindings = match self.class_properties.private_props_stack.last_mut() {
-                Some(private_props) => &mut private_props.class_bindings,
-                None => &mut self.class_properties.class_bindings,
-            };
-
-            let temp_binding = class_bindings.get_or_init_temp_binding(self.ctx);
+            let temp_binding = self.class_properties.get_temp_binding(self.ctx);
             *expr = temp_binding.create_spanned_read_expression(span, self.ctx);
         }
     }
 
+    /// Replace reference to class name with reference to temp var for class.
+    fn replace_class_name_with_temp_var(&mut self, ident: &mut IdentifierReference<'a>) {
+        // Check identifier is reference to class name
+        let class_name_symbol_id = self.class_properties.class_bindings.name_symbol_id();
+        let Some(class_name_symbol_id) = class_name_symbol_id else { return };
+
+        let reference_id = ident.reference_id();
+        let reference = self.ctx.symbols().get_reference(reference_id);
+        let Some(symbol_id) = reference.symbol_id() else { return };
+
+        if symbol_id != class_name_symbol_id {
+            return;
+        }
+
+        // Identifier is reference to class name. Rename it.
+        let temp_binding = self.class_properties.get_temp_binding(self.ctx);
+        ident.name = temp_binding.name.clone();
+
+        let symbols = self.ctx.symbols_mut();
+        symbols.get_reference_mut(reference_id).set_symbol_id(temp_binding.symbol_id);
+        symbols.delete_resolved_reference(symbol_id, reference_id);
+        symbols.add_resolved_reference(temp_binding.symbol_id, reference_id);
+    }
+
     /// Replace `delete this` with `true`.
     fn replace_delete_this_with_true(&self, expr: &mut Expression<'a>, span: Span) {
         if self.this_depth == 0 {
             *expr = self.ctx.ast.expression_boolean_literal(span, true);
         }
     }
 }
+
+impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
+    fn get_temp_binding(&mut self, ctx: &mut TraverseCtx<'a>) -> &BoundIdentifier<'a> {
+        // `PrivateProps` is the source of truth for bindings if class has private props
+        // because other visitors which transform private fields may create a temp binding
+        // and store it on `PrivateProps`
+        let class_bindings = match self.private_props_stack.last_mut() {
+            Some(private_props) => &mut private_props.class_bindings,
+            None => &mut self.class_bindings,
+        };
+
+        class_bindings.get_or_init_temp_binding(ctx)
+    }
+}

tasks/transform_conformance/snapshots/babel.snap.md

@@ -1,6 +1,6 @@
 commit: 54a8389f
 
-Passed: 414/846
+Passed: 415/846
 
 # All Passed:
 * babel-plugin-transform-class-static-block
@@ -276,7 +276,7 @@ x Output mismatch
 x Output mismatch
 
 
-# babel-plugin-transform-class-properties (87/264)
+# babel-plugin-transform-class-properties (88/264)
 * assumption-constantSuper/complex-super-class/input.js
 x Output mismatch
 
@@ -339,7 +339,18 @@ after transform: ScopeId(6): Some(ScopeId(5))
 rebuilt        : ScopeId(9): Some(ScopeId(8))
 
 * assumption-setPublicClassFields/static-class-binding/input.js
-x Output mismatch
+Scope children mismatch:
+after transform: ScopeId(0): [ScopeId(1)]
+rebuilt        : ScopeId(0): [ScopeId(1), ScopeId(2)]
+Scope children mismatch:
+after transform: ScopeId(1): [ScopeId(2)]
+rebuilt        : ScopeId(1): []
+Scope flags mismatch:
+after transform: ScopeId(2): ScopeFlags(StrictMode | Function | Arrow)
+rebuilt        : ScopeId(2): ScopeFlags(Function | Arrow)
+Scope parent mismatch:
+after transform: ScopeId(2): Some(ScopeId(1))
+rebuilt        : ScopeId(2): Some(ScopeId(0))
 
 * assumption-setPublicClassFields/static-infer-name/input.js
 x Output mismatch
@@ -549,14 +560,22 @@ after transform: ScopeId(2): Some(ScopeId(1))
 rebuilt        : ScopeId(3): Some(ScopeId(0))
 
 * private/static-class-binding/input.js
-x Output mismatch
+Scope children mismatch:
+after transform: ScopeId(0): [ScopeId(1)]
+rebuilt        : ScopeId(0): [ScopeId(1), ScopeId(2)]
+Scope children mismatch:
+after transform: ScopeId(1): [ScopeId(2)]
+rebuilt        : ScopeId(1): []
+Scope flags mismatch:
+after transform: ScopeId(2): ScopeFlags(StrictMode | Function | Arrow)
+rebuilt        : ScopeId(2): ScopeFlags(Function | Arrow)
+Scope parent mismatch:
+after transform: ScopeId(2): Some(ScopeId(1))
+rebuilt        : ScopeId(2): Some(ScopeId(0))
 
 * private/static-infer-name/input.js
 x Output mismatch
 
-* private/static-self-field/input.js
-x Output mismatch
-
 * private/static-shadow/input.js
 x Output mismatch
 
@@ -863,7 +882,18 @@ after transform: ScopeId(6): Some(ScopeId(5))
 rebuilt        : ScopeId(9): Some(ScopeId(8))
 
 * public/static-class-binding/input.js
-x Output mismatch
+Scope children mismatch:
+after transform: ScopeId(0): [ScopeId(1)]
+rebuilt        : ScopeId(0): [ScopeId(1), ScopeId(2)]
+Scope children mismatch:
+after transform: ScopeId(1): [ScopeId(2)]
+rebuilt        : ScopeId(1): []
+Scope flags mismatch:
+after transform: ScopeId(2): ScopeFlags(StrictMode | Function | Arrow)
+rebuilt        : ScopeId(2): ScopeFlags(Function | Arrow)
+Scope parent mismatch:
+after transform: ScopeId(2): Some(ScopeId(1))
+rebuilt        : ScopeId(2): Some(ScopeId(0))
 
 * public/static-infer-name/input.js
 x Output mismatch
@@ -936,7 +966,18 @@ after transform: ScopeId(6): Some(ScopeId(5))
 rebuilt        : ScopeId(9): Some(ScopeId(8))
 
 * public-loose/static-class-binding/input.js
-x Output mismatch
+Scope children mismatch:
+after transform: ScopeId(0): [ScopeId(1)]
+rebuilt        : ScopeId(0): [ScopeId(1), ScopeId(2)]
+Scope children mismatch:
+after transform: ScopeId(1): [ScopeId(2)]
+rebuilt        : ScopeId(1): []
+Scope flags mismatch:
+after transform: ScopeId(2): ScopeFlags(StrictMode | Function | Arrow)
+rebuilt        : ScopeId(2): ScopeFlags(Function | Arrow)
+Scope parent mismatch:
+after transform: ScopeId(2): Some(ScopeId(1))
+rebuilt        : ScopeId(2): Some(ScopeId(0))
 
 * public-loose/static-infer-name/input.js
 x Output mismatch