fix(transfomer/using): remove use of child ids

Author: camc314

crates/oxc_semantic/src/scoping.rs

@@ -677,6 +677,12 @@ impl Scoping {
         });
     }
 
+    pub fn remove_child_scope(&mut self, scope_id: ScopeId, child_scope_ids: ScopeId) {
+        self.cell.with_dependent_mut(|_allocator, cell| {
+            cell.scope_child_ids[scope_id.index()].retain(|scope_id| *scope_id != child_scope_ids);
+        });
+    }
+
     /// Create a scope.
     #[inline]
     pub fn add_scope(

crates/oxc_transformer/examples/transformer.rs

@@ -45,7 +45,6 @@ fn main() {
     let ret = SemanticBuilder::new()
         // Estimate transformer will triple scopes, symbols, references
         .with_excess_capacity(2.0)
-        .with_scope_tree_child_ids(true)
         .build(&program);
 
     if !ret.errors.is_empty() {

crates/oxc_transformer/src/proposals/explicit_resource_management.rs

@@ -158,10 +158,26 @@ impl<'a> Traverse<'a> for ExplicitResourceManagement<'a, '_> {
     /// ```
     fn exit_static_block(&mut self, block: &mut StaticBlock<'a>, ctx: &mut TraverseCtx<'a>) {
         let scope_id = block.scope_id();
-        if let Some(replacement) =
-            self.transform_statements(&mut block.body, None, scope_id, scope_id, ctx)
+        if let Some((new_stmts, needs_await, using_ctx)) =
+            self.transform_statements(&mut block.body, scope_id, ctx)
         {
-            block.body = ctx.ast.vec1(replacement);
+            let static_block_new_scope_id = ctx.insert_scope_between(
+                ctx.scoping().scope_parent_id(scope_id).unwrap(),
+                scope_id,
+                ScopeFlags::ClassStaticBlock,
+            );
+
+            ctx.scoping_mut().set_symbol_scope_id(using_ctx.symbol_id, static_block_new_scope_id);
+            ctx.scoping_mut().move_binding(scope_id, static_block_new_scope_id, &using_ctx.name);
+            *ctx.scoping_mut().scope_flags_mut(scope_id) = ScopeFlags::StrictMode;
+
+            block.set_scope_id(static_block_new_scope_id);
+            block.body = ctx.ast.vec1(Self::create_try_stmt(
+                ctx.ast.block_statement_with_scope_id(SPAN, new_stmts, scope_id),
+                Self::create_catch_clause(&using_ctx, static_block_new_scope_id, ctx),
+                Self::create_finally_block(&using_ctx, static_block_new_scope_id, needs_await, ctx),
+                ctx,
+            ));
         }
     }
 
@@ -186,14 +202,22 @@ impl<'a> Traverse<'a> for ExplicitResourceManagement<'a, '_> {
     /// }
     /// ```
     fn enter_function_body(&mut self, body: &mut FunctionBody<'a>, ctx: &mut TraverseCtx<'a>) {
-        if let Some(replacement) = self.transform_statements(
-            &mut body.statements,
-            None,
-            ctx.current_hoist_scope_id(),
-            ctx.current_hoist_scope_id(),
-            ctx,
-        ) {
-            body.statements = ctx.ast.vec1(replacement);
+        if let Some((new_stmts, needs_await, using_ctx)) =
+            self.transform_statements(&mut body.statements, ctx.current_hoist_scope_id(), ctx)
+        {
+            // FIXME: this creates the scopes in the correct place, however we never move the bindings contained
+            // within `new_stmts` to the new scope.
+            let block_stmt_scope_id =
+                ctx.insert_scope_below_statements(&new_stmts, ScopeFlags::empty());
+
+            let current_scope_id = ctx.current_scope_id();
+
+            body.statements = ctx.ast.vec1(Self::create_try_stmt(
+                ctx.ast.block_statement_with_scope_id(SPAN, new_stmts, block_stmt_scope_id),
+                Self::create_catch_clause(&using_ctx, current_scope_id, ctx),
+                Self::create_finally_block(&using_ctx, current_scope_id, needs_await, ctx),
+                ctx,
+            ));
         }
     }
 
@@ -461,15 +485,17 @@ impl<'a> ExplicitResourceManagement<'a, '_> {
     fn transform_block_statement(&mut self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
         let Statement::BlockStatement(block_stmt) = stmt else { unreachable!() };
 
-        let scope_id = block_stmt.scope_id();
-        if let Some(replacement) = self.transform_statements(
-            &mut block_stmt.body,
-            Some(scope_id),
-            ctx.current_scope_id(),
-            ctx.current_hoist_scope_id(),
-            ctx,
-        ) {
-            *stmt = replacement;
+        if let Some((new_stmts, needs_await, using_ctx)) =
+            self.transform_statements(&mut block_stmt.body, ctx.current_hoist_scope_id(), ctx)
+        {
+            let current_scope_id = ctx.current_scope_id();
+
+            *stmt = Self::create_try_stmt(
+                ctx.ast.block_statement_with_scope_id(SPAN, new_stmts, block_stmt.scope_id()),
+                Self::create_catch_clause(&using_ctx, current_scope_id, ctx),
+                Self::create_finally_block(&using_ctx, current_scope_id, needs_await, ctx),
+                ctx,
+            );
         }
     }
 
@@ -596,8 +622,6 @@ impl<'a> ExplicitResourceManagement<'a, '_> {
 
     /// Transforms:
     ///  - `node` - the statements to transform
-    ///  - `scope_id` - if provided, it will be used as the scope_id for the new block.
-    ///  - `parent_scope_id` - the parent scope
     ///  - `hoist_scope_id` - the hoist scope, used for generating new var bindings
     ///  - `ctx` - the traverse context
     ///
@@ -624,11 +648,9 @@ impl<'a> ExplicitResourceManagement<'a, '_> {
     fn transform_statements(
         &mut self,
         stmts: &mut ArenaVec<'a, Statement<'a>>,
-        scope_id: Option<ScopeId>,
-        parent_scope_id: ScopeId,
         hoist_scope_id: ScopeId,
         ctx: &mut TraverseCtx<'a>,
-    ) -> Option<Statement<'a>> {
+    ) -> Option<(ArenaVec<'a, Statement<'a>>, bool, BoundIdentifier<'a>)> {
         let mut needs_await = false;
 
         let mut using_ctx = None;
@@ -700,21 +722,16 @@ impl<'a> ExplicitResourceManagement<'a, '_> {
         );
         stmts.insert(0, Statement::from(helper));
 
-        let scope_id_children_to_move = scope_id.unwrap_or(parent_scope_id);
-
-        let scope_id = scope_id
-            .unwrap_or_else(|| ctx.create_child_scope(parent_scope_id, ScopeFlags::empty()));
-        let block = ctx.ast.block_statement_with_scope_id(SPAN, stmts, scope_id);
-
-        let child_ids = ctx.scoping_mut().get_scope_child_ids(scope_id_children_to_move).to_vec();
-        for id in child_ids.iter().filter(|id| *id != &scope_id) {
-            ctx.scoping_mut().change_scope_parent_id(*id, Some(scope_id));
-        }
-
-        let catch = Self::create_catch_clause(&using_ctx, parent_scope_id, ctx);
-        let finally = Self::create_finally_block(&using_ctx, parent_scope_id, needs_await, ctx);
+        Some((stmts, needs_await, using_ctx))
+    }
 
-        Some(ctx.ast.statement_try(SPAN, block, Some(catch), Some(finally)))
+    fn create_try_stmt(
+        body: BlockStatement<'a>,
+        catch: ArenaBox<'a, CatchClause<'a>>,
+        finally: ArenaBox<'a, BlockStatement<'a>>,
+        ctx: &TraverseCtx<'a>,
+    ) -> Statement<'a> {
+        ctx.ast.statement_try(SPAN, body, Some(catch), Some(finally))
     }
 
     /// `catch (_) { _usingCtx.e = _; }`

crates/oxc_traverse/src/context/mod.rs

@@ -315,6 +315,35 @@ impl<'a> TraverseCtx<'a> {
         self.scoping.insert_scope_below_statements(stmts, flags)
     }
 
+    /// Insert a scope between a parent and a child scope.
+    ///
+    /// For example, given the following scopes
+    /// ```ts
+    /// parentScope1: {
+    ///     childScope: { }
+    ///     childScope2: { }
+    /// }
+    /// ```
+    /// and calling this function with `parentScope1` and `childScope`,
+    /// the resulting scopes will be:
+    /// ```ts
+    /// parentScope1: {
+    ///     newScope: {   
+    ///         childScope: { }
+    ///     }
+    ///     childScope2: { }
+    /// }
+    /// ```
+    /// This is a shortcut for `ctx.scoping.insert_scope_between`.
+    pub fn insert_scope_between(
+        &mut self,
+        parent_id: ScopeId,
+        child_id: ScopeId,
+        flags: ScopeFlags,
+    ) -> ScopeId {
+        self.scoping.insert_scope_between(parent_id, child_id, flags)
+    }
+
     /// Remove scope for an expression from the scope chain.
     ///
     /// Delete the scope and set parent of its child scopes to its parent scope.

crates/oxc_traverse/src/context/scoping.rs

@@ -176,6 +176,46 @@ impl TraverseScoping {
         new_scope_id
     }
 
+    /// Insert a scope between a parent and a child scope.
+    ///
+    /// For example, given the following scopes
+    /// ```ts
+    /// parentScope1: {
+    ///     childScope: { }
+    ///     childScope2: { }
+    /// }
+    /// ```
+    /// and calling this function with `parentScope1` and `childScope`,
+    /// the resulting scopes will be:
+    /// ```ts
+    /// parentScope1: {
+    ///     newScope: {   
+    ///         childScope: { }
+    ///     }
+    ///     childScope2: { }
+    /// }
+    /// ```
+    pub fn insert_scope_between(
+        &mut self,
+        parent_id: ScopeId,
+        child_id: ScopeId,
+        flags: ScopeFlags,
+    ) -> ScopeId {
+        let scope_id = self.create_child_scope(parent_id, flags);
+
+        debug_assert_eq!(
+            self.scoping.scope_parent_id(child_id),
+            Some(parent_id),
+            "Child scope must be a child of parent scope"
+        );
+
+        if self.scoping.has_scope_child_ids() {
+            self.scoping.remove_child_scope(parent_id, child_id);
+        }
+        self.scoping.set_scope_parent_id(child_id, Some(scope_id));
+        scope_id
+    }
+
     /// Remove scope for an expression from the scope chain.
     ///
     /// Delete the scope and set parent of its child scopes to its parent scope.

tasks/transform_conformance/snapshots/babel.snap.md

@@ -1,6 +1,6 @@
 commit: 578ac4df
 
-Passed: 713/1191
+Passed: 714/1191
 
 # All Passed:
 * babel-plugin-transform-logical-assignment-operators
@@ -2746,7 +2746,7 @@ transform-react-jsx: unknown field `autoImport`, expected one of `runtime`, `dev
 x Output mismatch
 
 
-# babel-plugin-proposal-explicit-resource-management (20/24)
+# babel-plugin-proposal-explicit-resource-management (21/24)
 * transform-sync/function-body/input.js
 Bindings mismatch:
 after transform: ScopeId(1): ["_usingCtx", "x"]
@@ -2769,17 +2769,6 @@ Symbol scope ID mismatch for "z":
 after transform: SymbolId(2): ScopeId(3)
 rebuilt        : SymbolId(5): ScopeId(4)
 
-* transform-sync/static-block/input.js
-Bindings mismatch:
-after transform: ScopeId(2): ["_usingCtx", "x"]
-rebuilt        : ScopeId(2): ["_usingCtx"]
-Bindings mismatch:
-after transform: ScopeId(3): []
-rebuilt        : ScopeId(3): ["x"]
-Symbol scope ID mismatch for "x":
-after transform: SymbolId(1): ScopeId(2)
-rebuilt        : SymbolId(2): ScopeId(3)
-
 * transform-top-level/hoisting-default-class/input.mjs
 Symbol span mismatch for "C":
 after transform: SymbolId(1): Span { start: 37, end: 38 }

tasks/transform_conformance/snapshots/oxc.snap.md

@@ -323,21 +323,9 @@ x Output mismatch
 Bindings mismatch:
 after transform: ScopeId(1): ["_usingCtx", "a", "b", "x", "y"]
 rebuilt        : ScopeId(1): ["_usingCtx", "a", "b"]
-Scope children mismatch:
-after transform: ScopeId(1): [ScopeId(5), ScopeId(6), ScopeId(8)]
-rebuilt        : ScopeId(1): [ScopeId(2), ScopeId(3), ScopeId(4), ScopeId(6), ScopeId(8)]
-Scope parent mismatch:
-after transform: ScopeId(2): Some(ScopeId(5))
-rebuilt        : ScopeId(2): Some(ScopeId(1))
-Scope parent mismatch:
-after transform: ScopeId(3): Some(ScopeId(5))
-rebuilt        : ScopeId(3): Some(ScopeId(1))
 Bindings mismatch:
 after transform: ScopeId(5): []
 rebuilt        : ScopeId(4): ["x", "y"]
-Scope children mismatch:
-after transform: ScopeId(5): [ScopeId(2), ScopeId(3), ScopeId(4)]
-rebuilt        : ScopeId(4): [ScopeId(5)]
 Symbol scope ID mismatch for "x":
 after transform: SymbolId(3): ScopeId(1)
 rebuilt        : SymbolId(4): ScopeId(4)