From bb22c67974ab1e3b5fd6b24722a80bb7648ba51f Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed, 11 Dec 2024 03:18:29 +0000 Subject: [PATCH] fix(transformer/class-properties): fix `ScopeId`s in static prop initializers (#7791) Code in static property initializers moves from inside the class to outside. Update parent `ScopeId`s for first level scopes in initializers. --- .../class_properties/static_prop_init.rs | 54 ++++- .../snapshots/babel.snap.md | 202 +----------------- .../snapshots/oxc.snap.md | 38 +--- 3 files changed, 53 insertions(+), 241 deletions(-) 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 86ecf6e3f3315..4a91750f1ec4b 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 @@ -70,7 +70,9 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { /// * Class expression: `x = class C { static #x = 123; static y = this.#x; }` /// -> `var _C, _x; x = (_C = class C {}, _x = { _: 123 }, _C.y = _assertClassBrand(_C, _C, _x)._), _C)` /// -/// Also sets `ScopeFlags` of scopes to sloppy mode if code outside the class is sloppy mode. +/// Also: +/// * Updates parent `ScopeId` of first level of scopes in initializer. +/// * Sets `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`, and private fields are only valid @@ -107,8 +109,6 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { // 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. -// -// TODO: Also re-parent child scopes. struct StaticInitializerVisitor<'a, 'ctx, 'v> { /// `true` if class has name, or class has private properties, or `ScopeFlags` need updating. /// Any of these neccesitates walking the whole tree. If none of those apply, we only need to @@ -119,6 +119,9 @@ struct StaticInitializerVisitor<'a, 'ctx, 'v> { /// Incremented when entering a different `this` context, decremented when exiting it. /// `this` should be transformed when `this_depth == 0`. this_depth: u32, + /// Incremented when entering a scope, decremented when exiting it. + /// Parent `ScopeId` should be updated when `scope_depth == 0`. + scope_depth: u32, /// Main transform instance. class_properties: &'v mut ClassProperties<'a, 'ctx>, /// `TraverseCtx` object. @@ -137,6 +140,7 @@ impl<'a, 'ctx, 'v> StaticInitializerVisitor<'a, 'ctx, 'v> { || class_properties.private_props_stack.last().is_some(), make_sloppy_mode, this_depth: 0, + scope_depth: 0, class_properties, ctx, } @@ -238,13 +242,31 @@ impl<'a, 'ctx, 'v> VisitMut<'a> for StaticInitializerVisitor<'a, 'ctx, 'v> { self.replace_class_name_with_temp_var(ident); } - /// Convert scope to sloppy mode if `self.make_sloppy_mode == true` + /// Update parent scope for first level of scopes. + /// Convert scope to sloppy mode if `self.make_sloppy_mode == true`. fn enter_scope(&mut self, _flags: ScopeFlags, scope_id: &Cell>) { + let scope_id = scope_id.get().unwrap(); + + // TODO: Not necessary to do this check for all scopes. + // In JS, only `Function`, `ArrowFunctionExpression` or `Class` can be the first-level scope, + // as all other types which have a scope are statements or `StaticBlock` which would need to be + // inside a function or class. But some TS types with scopes could be first level via + // e.g. `TaggedTemplateExpression::type_parameters`, which contains `TSType`. + // Not sure if that matters though, as they'll be stripped out anyway by TS transform. + if self.scope_depth == 0 { + self.reparent_scope(scope_id); + } + self.scope_depth += 1; + if self.make_sloppy_mode { - *self.ctx.scopes_mut().get_flags_mut(scope_id.get().unwrap()) -= ScopeFlags::StrictMode; + *self.ctx.scopes_mut().get_flags_mut(scope_id) -= ScopeFlags::StrictMode; } } + fn leave_scope(&mut self) { + self.scope_depth -= 1; + } + // 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. @@ -267,6 +289,10 @@ impl<'a, 'ctx, 'v> VisitMut<'a> for StaticInitializerVisitor<'a, 'ctx, 'v> { self.this_depth += 1; walk_mut::walk_function(self, func, flags); self.this_depth -= 1; + } else if self.scope_depth == 0 { + // Need to call `reparent_scope` because not calling `walk_function`, + // which is what calls `enter_scope`, and this can be first-level scope + self.reparent_scope(func.scope_id()); } self.make_sloppy_mode = parent_sloppy_mode; @@ -302,6 +328,9 @@ impl<'a, 'ctx, 'v> VisitMut<'a> for StaticInitializerVisitor<'a, 'ctx, 'v> { self.this_depth += 1; walk_mut::walk_static_block(self, block); self.this_depth -= 1; + } else { + // Not possible that `self.scope_depth == 0` here, because a `StaticBlock` + // can only be in a class, and that class would be the first-level scope } } @@ -317,6 +346,9 @@ impl<'a, 'ctx, 'v> VisitMut<'a> for StaticInitializerVisitor<'a, 'ctx, 'v> { self.this_depth += 1; walk_mut::walk_ts_module_block(self, block); self.this_depth -= 1; + } else { + // Not possible that `self.scope_depth == 0` here, because a `TSModuleBlock` + // can only be in a function, and that function would be the first-level scope } self.make_sloppy_mode = parent_sloppy_mode; @@ -345,6 +377,9 @@ impl<'a, 'ctx, 'v> VisitMut<'a> for StaticInitializerVisitor<'a, 'ctx, 'v> { self.visit_expression(value); self.this_depth -= 1; } + } else { + // Not possible that `self.scope_depth == 0` here, because a `PropertyDefinition` + // can only be in a class, and that class would be the first-level scope } } @@ -363,6 +398,9 @@ impl<'a, 'ctx, 'v> VisitMut<'a> for StaticInitializerVisitor<'a, 'ctx, 'v> { self.visit_expression(value); self.this_depth -= 1; } + } else { + // Not possible that `self.scope_depth == 0` here, because an `AccessorProperty` + // can only be in a class, and that class would be the first-level scope } } } @@ -406,6 +444,12 @@ impl<'a, 'ctx, 'v> StaticInitializerVisitor<'a, 'ctx, 'v> { *expr = self.ctx.ast.expression_boolean_literal(span, true); } } + + /// Update parent of scope to scope above class. + fn reparent_scope(&mut self, scope_id: ScopeId) { + let current_scope_id = self.ctx.current_scope_id(); + self.ctx.scopes_mut().change_parent_id(scope_id, Some(current_scope_id)); + } } impl<'a, 'ctx> ClassProperties<'a, 'ctx> { diff --git a/tasks/transform_conformance/snapshots/babel.snap.md b/tasks/transform_conformance/snapshots/babel.snap.md index 037ef32cab428..879dd0754aa1e 100644 --- a/tasks/transform_conformance/snapshots/babel.snap.md +++ b/tasks/transform_conformance/snapshots/babel.snap.md @@ -1,6 +1,6 @@ commit: 54a8389f -Passed: 485/846 +Passed: 503/846 # All Passed: * babel-plugin-transform-class-static-block @@ -276,7 +276,7 @@ x Output mismatch x Output mismatch -# babel-plugin-transform-class-properties (158/264) +# babel-plugin-transform-class-properties (176/264) * assumption-constantSuper/complex-super-class/input.js x Output mismatch @@ -338,17 +338,6 @@ Scope parent mismatch: after transform: ScopeId(6): Some(ScopeId(5)) rebuilt : ScopeId(9): Some(ScopeId(8)) -* assumption-setPublicClassFields/static-class-binding/input.js -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 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 @@ -358,17 +347,6 @@ x Output mismatch * assumption-setPublicClassFields/static-super-loose/input.js x Output mismatch -* assumption-setPublicClassFields/static-this/input.js -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 parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(2): Some(ScopeId(0)) - * assumption-setPublicClassFields/super-with-collision/input.js x Output mismatch @@ -443,28 +421,6 @@ x Output mismatch * private/nested-class-extends-computed-redeclared/input.js x Output mismatch -* private/optional-chain-before-member-call/input.js -Scope children mismatch: -after transform: ScopeId(0): [ScopeId(1)] -rebuilt : ScopeId(0): [ScopeId(1), ScopeId(6)] -Scope children mismatch: -after transform: ScopeId(1): [ScopeId(2), ScopeId(3), ScopeId(4)] -rebuilt : ScopeId(1): [ScopeId(2), ScopeId(3)] -Scope parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(6): Some(ScopeId(0)) - -* private/optional-chain-before-member-call-with-transform/input.js -Scope children mismatch: -after transform: ScopeId(0): [ScopeId(1)] -rebuilt : ScopeId(0): [ScopeId(1), ScopeId(6)] -Scope children mismatch: -after transform: ScopeId(1): [ScopeId(2), ScopeId(3), ScopeId(4)] -rebuilt : ScopeId(1): [ScopeId(2), ScopeId(3)] -Scope parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(6): Some(ScopeId(0)) - * private/optional-chain-cast-to-boolean/input.js x Output mismatch @@ -477,56 +433,12 @@ x Output mismatch * private/optional-chain-in-function-param-with-transform/input.js x Output mismatch -* private/optional-chain-member-optional-call/input.js -Scope children mismatch: -after transform: ScopeId(0): [ScopeId(1)] -rebuilt : ScopeId(0): [ScopeId(1), ScopeId(6)] -Scope children mismatch: -after transform: ScopeId(1): [ScopeId(2), ScopeId(3), ScopeId(4)] -rebuilt : ScopeId(1): [ScopeId(2), ScopeId(3)] -Scope parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(6): Some(ScopeId(0)) - * private/optional-chain-member-optional-call-spread-arguments/input.js x Output mismatch * private/optional-chain-member-optional-call-with-transform/input.js x Output mismatch -* private/optional-chain-optional-member-call/input.js -Scope children mismatch: -after transform: ScopeId(0): [ScopeId(1)] -rebuilt : ScopeId(0): [ScopeId(1), ScopeId(6)] -Scope children mismatch: -after transform: ScopeId(1): [ScopeId(2), ScopeId(3), ScopeId(4)] -rebuilt : ScopeId(1): [ScopeId(2), ScopeId(3)] -Scope parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(6): Some(ScopeId(0)) - -* private/optional-chain-optional-member-call-with-transform/input.js -Scope children mismatch: -after transform: ScopeId(0): [ScopeId(1)] -rebuilt : ScopeId(0): [ScopeId(1), ScopeId(6)] -Scope children mismatch: -after transform: ScopeId(1): [ScopeId(2), ScopeId(3), ScopeId(4)] -rebuilt : ScopeId(1): [ScopeId(2), ScopeId(3)] -Scope parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(6): Some(ScopeId(0)) - -* private/parenthesized-optional-member-call/input.js -Scope children mismatch: -after transform: ScopeId(0): [ScopeId(1)] -rebuilt : ScopeId(0): [ScopeId(1), ScopeId(5)] -Scope children mismatch: -after transform: ScopeId(1): [ScopeId(2), ScopeId(3), ScopeId(4)] -rebuilt : ScopeId(1): [ScopeId(2), ScopeId(3)] -Scope parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(5): Some(ScopeId(0)) - * private/parenthesized-optional-member-call-with-transform/input.js x Output mismatch @@ -559,45 +471,12 @@ Scope parent mismatch: after transform: ScopeId(6): Some(ScopeId(5)) rebuilt : ScopeId(9): Some(ScopeId(8)) -* private/static-call/input.js -Scope children mismatch: -after transform: ScopeId(0): [ScopeId(1)] -rebuilt : ScopeId(0): [ScopeId(1), ScopeId(3)] -Scope children mismatch: -after transform: ScopeId(1): [ScopeId(2), ScopeId(3)] -rebuilt : ScopeId(1): [ScopeId(2)] -Scope parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(3): Some(ScopeId(0)) - -* private/static-class-binding/input.js -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 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-shadow/input.js x Output mismatch -* private/static-this/input.js -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 parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(2): Some(ScopeId(0)) - * private-loose/call/input.js Scope children mismatch: after transform: ScopeId(1): [ScopeId(2), ScopeId(3), ScopeId(4)] @@ -694,42 +573,9 @@ x Output mismatch * private-loose/parenthesized-optional-member-call-with-transform/input.js x Output mismatch -* private-loose/static-call/input.js -Scope children mismatch: -after transform: ScopeId(0): [ScopeId(1)] -rebuilt : ScopeId(0): [ScopeId(1), ScopeId(3)] -Scope children mismatch: -after transform: ScopeId(1): [ScopeId(2), ScopeId(3)] -rebuilt : ScopeId(1): [ScopeId(2)] -Scope parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(3): Some(ScopeId(0)) - -* private-loose/static-class-binding/input.js -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 parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(2): Some(ScopeId(0)) - * private-loose/static-infer-name/input.js x Output mismatch -* private-loose/static-this/input.js -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 parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(2): Some(ScopeId(0)) - * public/call/input.js Scope children mismatch: after transform: ScopeId(1): [ScopeId(2), ScopeId(3), ScopeId(4)] @@ -796,34 +642,12 @@ Scope parent mismatch: after transform: ScopeId(6): Some(ScopeId(5)) rebuilt : ScopeId(9): Some(ScopeId(8)) -* public/static-class-binding/input.js -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 parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(2): Some(ScopeId(0)) - * public/static-infer-name/input.js x Output mismatch * public/static-super/input.js x Output mismatch -* public/static-this/input.js -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 parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(2): Some(ScopeId(0)) - * public/super-with-collision/input.js x Output mismatch @@ -876,34 +700,12 @@ Scope parent mismatch: after transform: ScopeId(6): Some(ScopeId(5)) rebuilt : ScopeId(9): Some(ScopeId(8)) -* public-loose/static-class-binding/input.js -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 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 * public-loose/static-super/input.js x Output mismatch -* public-loose/static-this/input.js -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 parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(2): Some(ScopeId(0)) - * public-loose/super-with-collision/input.js x Output mismatch diff --git a/tasks/transform_conformance/snapshots/oxc.snap.md b/tasks/transform_conformance/snapshots/oxc.snap.md index bd67702933ae2..125727470b353 100644 --- a/tasks/transform_conformance/snapshots/oxc.snap.md +++ b/tasks/transform_conformance/snapshots/oxc.snap.md @@ -1,6 +1,6 @@ commit: 54a8389f -Passed: 103/117 +Passed: 105/117 # All Passed: * babel-plugin-transform-class-static-block @@ -16,7 +16,7 @@ Passed: 103/117 * regexp -# babel-plugin-transform-class-properties (4/7) +# babel-plugin-transform-class-properties (6/7) * private-loose-tagged-template/input.js Scope children mismatch: after transform: ScopeId(1): [ScopeId(2), ScopeId(3), ScopeId(4)] @@ -28,40 +28,6 @@ Scope parent mismatch: after transform: ScopeId(2): Some(ScopeId(1)) rebuilt : ScopeId(3): Some(ScopeId(2)) -* private-loose-tagged-template-static/input.js -Scope children mismatch: -after transform: ScopeId(0): [ScopeId(1)] -rebuilt : ScopeId(0): [ScopeId(1), ScopeId(3)] -Scope children mismatch: -after transform: ScopeId(1): [ScopeId(2), ScopeId(3)] -rebuilt : ScopeId(1): [ScopeId(2)] -Scope parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(3): Some(ScopeId(0)) - -* static-prop-initializer-strict-mode/input.js -Scope children mismatch: -after transform: ScopeId(0): [ScopeId(1)] -rebuilt : ScopeId(0): [ScopeId(1), ScopeId(2), ScopeId(8), ScopeId(14), ScopeId(17), ScopeId(20)] -Scope children mismatch: -after transform: ScopeId(1): [ScopeId(2), ScopeId(8), ScopeId(14), ScopeId(17), ScopeId(20)] -rebuilt : ScopeId(1): [] -Scope parent mismatch: -after transform: ScopeId(2): Some(ScopeId(1)) -rebuilt : ScopeId(2): Some(ScopeId(0)) -Scope parent mismatch: -after transform: ScopeId(8): Some(ScopeId(1)) -rebuilt : ScopeId(8): Some(ScopeId(0)) -Scope parent mismatch: -after transform: ScopeId(14): Some(ScopeId(1)) -rebuilt : ScopeId(14): Some(ScopeId(0)) -Scope parent mismatch: -after transform: ScopeId(17): Some(ScopeId(1)) -rebuilt : ScopeId(17): Some(ScopeId(0)) -Scope parent mismatch: -after transform: ScopeId(20): Some(ScopeId(1)) -rebuilt : ScopeId(20): Some(ScopeId(0)) - # babel-plugin-transform-async-to-generator (14/15) * super/nested/input.js