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.

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 this and class name in static blocks #8035

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 1 addition & 3 deletions crates/oxc_transformer/src/es2022/class_properties/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,7 @@
//! * `prop_decl.rs`: Transform of property declarations (instance and static).
//! * `constructor.rs`: Insertion of property initializers into class constructor.
//! * `instance_prop_init.rs`: Transform of instance property initializers.
//! * `static_prop_init.rs`: Transform of static property initializers.
//! * `static_block.rs`: Transform of static blocks.
//! * `static_prop_init.rs`: Transform of static property initializers and static blocks.
//! * `computed_key.rs`: Transform of property/method computed keys.
//! * `private_field.rs`: Transform of private fields (`this.#prop`).
//! * `super.rs`: Transform `super` expressions.
Expand Down Expand Up @@ -216,7 +215,6 @@ mod constructor;
mod instance_prop_init;
mod private_field;
mod prop_decl;
mod static_block;
mod static_prop_init;
mod supers;
mod utils;
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! ES2022: Class Properties
//! Transform of static property initializers.
//! Transform of static property initializers and static blocks.
use std::cell::Cell;

Expand All @@ -10,47 +10,130 @@ use oxc_ast::{
use oxc_syntax::scope::{ScopeFlags, ScopeId};
use oxc_traverse::TraverseCtx;

use super::super::ClassStaticBlock;
use super::ClassProperties;

impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
/// Transform static property initializer.
///
/// Replace `this`, and references to class name, with temp var for class.
/// Replace `this`, and references to class name, with temp var for class. Transform `super`.
/// See below for full details of transforms.
pub(super) fn transform_static_initializer(
&mut self,
value: &mut Expression<'a>,
ctx: &mut TraverseCtx<'a>,
) {
let mut replacer = StaticInitializerVisitor::new(self, ctx);
let make_sloppy_mode = !ctx.current_scope_flags().is_strict_mode();
let mut replacer = StaticVisitor::new(make_sloppy_mode, true, self, ctx);
replacer.visit_expression(value);
}

/// Transform static block.
///
/// Transform to an `Expression` and insert after class body.
///
/// `static { x = 1; }` -> `x = 1`
/// `static { x = 1; y = 2; } -> `(() => { x = 1; y = 2; })()`
///
/// Replace `this`, and references to class name, with temp var for class. Transform `super`.
/// See below for full details of transforms.
///
/// TODO: Add tests for this if there aren't any already.
/// Include tests for evaluation order inc that static block goes before class expression
/// unless also static properties, or static block uses class name.
pub(super) fn convert_static_block(
&mut self,
block: &mut StaticBlock<'a>,
ctx: &mut TraverseCtx<'a>,
) {
let replacement = self.convert_static_block_to_expression(block, ctx);
self.insert_expr_after_class(replacement, ctx);
}

fn convert_static_block_to_expression(
&mut self,
block: &mut StaticBlock<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Expression<'a> {
let scope_id = block.scope_id();
let outer_scope_strict_flag = ctx.current_scope_flags() & ScopeFlags::StrictMode;
let make_sloppy_mode = outer_scope_strict_flag == ScopeFlags::empty();

// If block contains only a single `ExpressionStatement`, no need to wrap in an IIFE.
// `static { foo }` -> `foo`
// TODO(improve-on-babel): If block has no statements, could remove it entirely.
let stmts = &mut block.body;
if stmts.len() == 1 {
if let Statement::ExpressionStatement(stmt) = stmts.first_mut().unwrap() {
return self.convert_static_block_with_single_expression_to_expression(
&mut stmt.expression,
scope_id,
make_sloppy_mode,
ctx,
);
}
}

// Wrap statements in an IIFE.
// Note: Do not reparent scopes.
let mut replacer = StaticVisitor::new(make_sloppy_mode, false, self, ctx);
replacer.visit_statements(stmts);

let scope_flags = outer_scope_strict_flag | ScopeFlags::Function | ScopeFlags::Arrow;
*ctx.scopes_mut().get_flags_mut(scope_id) = scope_flags;

let outer_scope_id = ctx.current_scope_id();
ctx.scopes_mut().change_parent_id(scope_id, Some(outer_scope_id));

ClassStaticBlock::wrap_statements_in_iife(stmts, scope_id, ctx)
}

fn convert_static_block_with_single_expression_to_expression(
&mut self,
expr: &mut Expression<'a>,
scope_id: ScopeId,
make_sloppy_mode: bool,
ctx: &mut TraverseCtx<'a>,
) -> Expression<'a> {
// Note: Reparent scopes
let mut replacer = StaticVisitor::new(make_sloppy_mode, true, self, ctx);
replacer.visit_expression(expr);

// Delete scope for static block
ctx.scopes_mut().delete_scope(scope_id);

ctx.ast.move_expression(expr)
}
}

/// Visitor to transform:
///
/// 1. `this` to class temp var.
/// * 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, _C)`
/// * Class declaration:
/// * `class C { static x = this.y; }` -> `var _C; class C {}; _C = C; C.x = _C.y;`
/// * `class C { static { this.x(); } }` -> `var _C; class C {}; _C = C; _C.x();`
/// * Class expression:
/// * `x = class C { static x = this.y; }` -> `var _C; x = (_C = class C {}, _C.x = _C.y, _C)`
/// * `C = class C { static { this.x(); } }` -> `var _C; C = (_C = class C {}, _C.x(), _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)`
/// * Class declaration:
/// * `class C { static x = C.y; }` -> `var _C; class C {}; _C = C; C.x = _C.y;`
/// * `class C { static { C.x(); } }` -> `var _C; class C {}; _C = C; _C.x();`
/// * Class expression:
/// * `x = class C { static x = C.y; }` -> `var _C; x = (_C = class C {}, _C.x = _C.y, _C)`
/// * `x = class C { static { C.x(); } }` -> `var _C; x = (_C = class C {}, _C.x(), _C)`
///
/// Also:
/// * Update parent `ScopeId` of first level of scopes in initializer.
/// * Update parent `ScopeId` of first level of scopes, if `reparent_scopes == true`.
/// * Set `ScopeFlags` of scopes to sloppy mode if code outside the class is sloppy mode.
///
/// Reason we need to transform `this` is because the initializer is being moved from inside the class
/// to outside. `this` outside the class refers to a different `this`. So we need to transform it.
/// Reason we need to transform `this` is because the initializer/block is being moved from inside
/// the class to outside. `this` outside the class refers to a different `this`. So we need to transform it.
///
/// 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` or class name.
/// 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.
/// This is because class binding `C` could be mutated, and the initializer/block may contain functions
/// which are not executed immediately, so the mutation occurs before that code runs.
///
/// ```js
/// class C {
Expand All @@ -73,9 +156,9 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
// code runs immediately, before any mutation of the class name binding can occur.
//
// TODO(improve-on-babel): Updating `ScopeFlags` for strict mode makes semantic correctly for the output,
// but actually the transform isn't right. Should wrap initializer in a strict mode IIFE so that
// initializer code runs in strict mode, as it was before within class body.
struct StaticInitializerVisitor<'a, 'ctx, 'v> {
// but actually the transform isn't right. Should wrap initializer/block in a strict mode IIFE so that
// code runs in strict mode, as it was before within class body.
struct StaticVisitor<'a, 'ctx, 'v> {
/// `true` if class has name, or `ScopeFlags` need updating.
/// Either of these neccesitates walking the whole tree. If neither applies, we only need to walk
/// as far as functions and other constructs which define a `this`.
Expand All @@ -90,29 +173,38 @@ struct StaticInitializerVisitor<'a, 'ctx, 'v> {
/// Note: `scope_depth` does not aim to track scope depth completely accurately.
/// Only requirement is to ensure that `scope_depth == 0` only when we're in first-level scope.
/// So we don't bother incrementing + decrementing for scopes which are definitely not first level.
/// e.g. `BlockStatement` or `ForStatement` must be in a function, and therefore we're already in a
/// nested scope.
/// In a static property initializer, e.g. `BlockStatement` or `ForStatement` must be in a function,
/// and therefore we're already in a nested scope.
/// In a static block which contains statements, we're wrapping it in an IIFE which takes on
/// the `ScopeId` of the old static block, so we don't need to reparent scopes anyway,
/// so `scope_depth` is ignored.
scope_depth: u32,
/// Main transform instance.
class_properties: &'v mut ClassProperties<'a, 'ctx>,
/// `TraverseCtx` object.
ctx: &'v mut TraverseCtx<'a>,
}

impl<'a, 'ctx, 'v> StaticInitializerVisitor<'a, 'ctx, 'v> {
impl<'a, 'ctx, 'v> StaticVisitor<'a, 'ctx, 'v> {
fn new(
make_sloppy_mode: bool,
reparent_scopes: bool,
class_properties: &'v mut ClassProperties<'a, 'ctx>,
ctx: &'v mut TraverseCtx<'a>,
) -> Self {
let make_sloppy_mode = !ctx.current_scope_flags().is_strict_mode();
let walk_deep =
make_sloppy_mode || class_properties.current_class().bindings.name.is_some();

Self { walk_deep, make_sloppy_mode, this_depth: 0, scope_depth: 0, class_properties, ctx }
// Set `scope_depth` to 1 initially if don't need to reparent scopes
// (static block where converting to IIFE)
#[expect(clippy::bool_to_int_with_if)]
let scope_depth = if reparent_scopes { 0 } else { 1 };

Self { walk_deep, make_sloppy_mode, this_depth: 0, scope_depth, class_properties, ctx }
}
}

impl<'a, 'ctx, 'v> VisitMut<'a> for StaticInitializerVisitor<'a, 'ctx, 'v> {
impl<'a, 'ctx, 'v> VisitMut<'a> for StaticVisitor<'a, 'ctx, 'v> {
#[inline]
fn visit_expression(&mut self, expr: &mut Expression<'a>) {
match expr {
Expand Down Expand Up @@ -324,8 +416,12 @@ impl<'a, 'ctx, 'v> VisitMut<'a> for StaticInitializerVisitor<'a, 'ctx, 'v> {

// Remaining visitors are the only other types which have a scope which can be first-level
// when starting traversal from an `Expression`.
// `BlockStatement` and all other statements would need to be within a function,
// and that function would be the first-level scope.
//
// In a static property initializer, `BlockStatement` and all other statements would need to be
// within a function, and that function would be the first-level scope.
//
// In a static block which contains statements, we're wrapping it in an IIFE which takes on
// the `ScopeId` of the old static block, so we don't need to reparent scopes anyway.

#[inline]
fn visit_ts_conditional_type(&mut self, conditional: &mut TSConditionalType<'a>) {
Expand Down Expand Up @@ -376,7 +472,7 @@ impl<'a, 'ctx, 'v> VisitMut<'a> for StaticInitializerVisitor<'a, 'ctx, 'v> {
}
}

impl<'a, 'ctx, 'v> StaticInitializerVisitor<'a, 'ctx, 'v> {
impl<'a, 'ctx, 'v> StaticVisitor<'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 {
Expand Down
18 changes: 13 additions & 5 deletions crates/oxc_transformer/src/es2022/class_static_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
use itoa::Buffer as ItoaBuffer;

use oxc_allocator::String as ArenaString;
use oxc_allocator::{String as ArenaString, Vec as ArenaVec};
use oxc_ast::{ast::*, NONE};
use oxc_span::SPAN;
use oxc_syntax::scope::{ScopeFlags, ScopeId};
Expand Down Expand Up @@ -130,10 +130,7 @@ impl ClassStaticBlock {
/// Convert static block to expression which will be value of private field.
/// `static { foo }` -> `foo`
/// `static { foo; bar; }` -> `(() => { foo; bar; })()`
///
/// This function also used by `ClassProperties` transform.
/// TODO: Make this function non-pub if no longer use it for `ClassProperties`.
pub fn convert_block_to_expression<'a>(
fn convert_block_to_expression<'a>(
block: &mut StaticBlock<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Expression<'a> {
Expand Down Expand Up @@ -161,6 +158,17 @@ impl ClassStaticBlock {
*ctx.scopes_mut().get_flags_mut(scope_id) =
ScopeFlags::Function | ScopeFlags::Arrow | ScopeFlags::StrictMode;

Self::wrap_statements_in_iife(stmts, scope_id, ctx)
}

/// Wrap statements in an IIFE.
///
/// This function also used by `ClassProperties` transform.
pub(super) fn wrap_statements_in_iife<'a>(
stmts: &mut ArenaVec<'a, Statement<'a>>,
scope_id: ScopeId,
ctx: &mut TraverseCtx<'a>,
) -> Expression<'a> {
let stmts = ctx.ast.move_vec(stmts);
let params = ctx.ast.alloc_formal_parameters(
SPAN,
Expand Down
21 changes: 7 additions & 14 deletions tasks/transform_conformance/snapshots/oxc.snap.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
commit: 54a8389f

Passed: 116/133
Passed: 117/135

# All Passed:
* babel-plugin-transform-class-static-block
Expand All @@ -16,26 +16,14 @@ Passed: 116/133
* regexp


# babel-plugin-transform-class-properties (17/22)
# babel-plugin-transform-class-properties (18/24)
* interaction-with-other-transforms/input.js
Bindings mismatch:
after transform: ScopeId(0): ["C", "C2", "_ref", "_ref2"]
rebuilt : ScopeId(0): ["C", "C2", "_a", "_e", "_g", "_ref", "_ref2"]
Scope children mismatch:
after transform: ScopeId(0): [ScopeId(1), ScopeId(3)]
rebuilt : ScopeId(0): [ScopeId(1), ScopeId(3), ScopeId(4)]
Bindings mismatch:
after transform: ScopeId(1): ["_a", "_e", "_g"]
rebuilt : ScopeId(1): []
Scope children mismatch:
after transform: ScopeId(1): [ScopeId(2), ScopeId(6)]
rebuilt : ScopeId(1): [ScopeId(2)]
Scope flags mismatch:
after transform: ScopeId(2): ScopeFlags(StrictMode | Function | Arrow)
rebuilt : ScopeId(3): ScopeFlags(Function | Arrow)
Scope parent mismatch:
after transform: ScopeId(2): Some(ScopeId(1))
rebuilt : ScopeId(3): Some(ScopeId(0))
Symbol scope ID mismatch for "_a":
after transform: SymbolId(4): ScopeId(1)
rebuilt : SymbolId(0): ScopeId(0)
Expand All @@ -46,6 +34,11 @@ Symbol scope ID mismatch for "_g":
after transform: SymbolId(6): ScopeId(1)
rebuilt : SymbolId(2): ScopeId(0)

* static-block-this-and-class-name/input.js
Symbol flags mismatch for "inner":
after transform: SymbolId(8): SymbolFlags(BlockScopedVariable | Function)
rebuilt : SymbolId(14): SymbolFlags(FunctionScopedVariable)

* static-super-assignment-target/input.js
x Output mismatch

Expand Down
Loading
Loading