Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(transformer/class-properties): replace references to class name with temp var in static prop initializers #7610

Merged
merged 1 commit into from
Dec 3, 2024
Merged

fix(transformer/class-properties): replace references to class name with temp var in static prop initializers #7610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 86 additions & 27 deletions crates/oxc_transformer/src/es2022/class_properties/static_prop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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; }`
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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.
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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)
}
}
59 changes: 50 additions & 9 deletions tasks/transform_conformance/snapshots/babel.snap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
commit: 54a8389f

Passed: 414/846
Passed: 415/846

# All Passed:
* babel-plugin-transform-class-static-block
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading