fix(transformer/typescript): enum merging when same name declared in …

…outer scope (#8691) ```typescript var x = 10; enum Merge { x = Math.random() } enum Merge { y = x // <-- refers to Merge.x } ``` This case wasn't covered in #8543 and by the [babel test case](https://github.com/babel/babel/blob/e568916ef3e5fd288039a65c904b12a7eba2a73a/packages/babel-plugin-transform-typescript/test/fixtures/enum/non-constant-member-reference/input.ts). To handle it we still have to go through the scope ancestors. --------- Co-authored-by: Dunqing <[email protected]>
oxc-project · Jan 26, 2025 · 3e509e1 · 3e509e1
1 parent 77ef61a
commit 3e509e1
Show file tree

Hide file tree

Showing 4 changed files with 139 additions and 9 deletions.
diff --git a/crates/oxc_transformer/src/typescript/enum.rs b/crates/oxc_transformer/src/typescript/enum.rs
@@ -1,9 +1,11 @@
 use rustc_hash::FxHashMap;
+use std::cell::Cell;
 
 use oxc_allocator::Vec as ArenaVec;
 use oxc_ast::{ast::*, visit::walk_mut, VisitMut, NONE};
+use oxc_data_structures::stack::NonEmptyStack;
 use oxc_ecmascript::ToInt32;
-use oxc_semantic::ScopeId;
+use oxc_semantic::{ScopeFlags, ScopeId};
 use oxc_span::{Atom, Span, SPAN};
 use oxc_syntax::{
     number::{NumberBase, ToJsString},
@@ -521,9 +523,9 @@ impl<'a> TypeScriptEnum<'a> {
 /// ```
 struct IdentifierReferenceRename<'a, 'ctx> {
     enum_name: Atom<'a>,
-    enum_scope_id: ScopeId,
-    ctx: &'ctx TraverseCtx<'a>,
     previous_enum_members: PrevMembers<'a>,
+    scope_stack: NonEmptyStack<ScopeId>,
+    ctx: &'ctx TraverseCtx<'a>,
 }
 
 impl<'a, 'ctx> IdentifierReferenceRename<'a, 'ctx> {
@@ -533,23 +535,70 @@ impl<'a, 'ctx> IdentifierReferenceRename<'a, 'ctx> {
         previous_enum_members: PrevMembers<'a>,
         ctx: &'ctx TraverseCtx<'a>,
     ) -> Self {
-        IdentifierReferenceRename { enum_name, enum_scope_id, ctx, previous_enum_members }
+        IdentifierReferenceRename {
+            enum_name,
+            previous_enum_members,
+            scope_stack: NonEmptyStack::new(enum_scope_id),
+            ctx,
+        }
     }
 }
 
 impl IdentifierReferenceRename<'_, '_> {
     fn should_reference_enum_member(&self, ident: &IdentifierReference<'_>) -> bool {
+        // Don't need to rename the identifier if it's not a member of the enum,
+        if !self.previous_enum_members.contains_key(&ident.name) {
+            return false;
+        };
+
         let symbol_table = self.ctx.scoping.symbols();
         let Some(symbol_id) = symbol_table.get_reference(ident.reference_id()).symbol_id() else {
-            // No symbol found. If the name is found in previous_enum_members,
-            // it must be referencing a member declared in a previous enum block: `enum Foo { A }; enum Foo { B = A }`
-            return self.previous_enum_members.contains_key(&ident.name);
+            // No symbol found, yet the name is found in previous_enum_members.
+            // It must be referencing a member declared in a previous enum block: `enum Foo { A }; enum Foo { B = A }`
+            return true;
         };
-        symbol_table.get_scope_id(symbol_id) == self.enum_scope_id
+
+        let symbol_scope_id = symbol_table.get_scope_id(symbol_id);
+        // Don't need to rename the identifier when it references a nested enum member:
+        //
+        // ```ts
+        // enum OuterEnum {
+        //   A = 0,
+        //   B = () => {
+        //     enum InnerEnum {
+        //       A = 0,
+        //       B = A,
+        //           ^ This references to `InnerEnum.A` should not be renamed
+        //     }
+        //     return InnerEnum.B;
+        //   }
+        // }
+        // ```
+        //
+        // `NonEmptyStack` guarantees that the stack is not empty.
+        *self.scope_stack.first().unwrap() == symbol_scope_id
+            // The resolved symbol is declared outside the enum,
+            // and we have checked that the name exists in previous_enum_members:
+            //
+            // ```ts
+            // const A = 0;
+            // enum Foo { A }
+            // enum Foo { B = A }
+            //                ^ This should be renamed to Foo.A
+            // ```
+        || !self.scope_stack.contains(&symbol_scope_id)
     }
 }
 
 impl<'a> VisitMut<'a> for IdentifierReferenceRename<'a, '_> {
+    fn enter_scope(&mut self, _flags: ScopeFlags, scope_id: &Cell<Option<ScopeId>>) {
+        self.scope_stack.push(scope_id.get().unwrap());
+    }
+
+    fn leave_scope(&mut self) {
+        self.scope_stack.pop();
+    }
+
     fn visit_expression(&mut self, expr: &mut Expression<'a>) {
         match expr {
             Expression::Identifier(ident) if self.should_reference_enum_member(ident) => {

diff --git a/tasks/transform_conformance/snapshots/oxc.snap.md b/tasks/transform_conformance/snapshots/oxc.snap.md
@@ -107,18 +107,65 @@ rebuilt        : ScopeId(0): []
 
 * enum-member-reference/input.ts
 Missing ReferenceId: "Foo"
+Missing ReferenceId: "Merge"
+Missing ReferenceId: "NestInner"
 Bindings mismatch:
 after transform: ScopeId(1): ["Foo", "a", "b", "c"]
 rebuilt        : ScopeId(1): ["Foo"]
 Scope flags mismatch:
 after transform: ScopeId(1): ScopeFlags(0x0)
 rebuilt        : ScopeId(1): ScopeFlags(Function)
+Bindings mismatch:
+after transform: ScopeId(2): ["Merge", "x"]
+rebuilt        : ScopeId(2): ["Merge"]
+Scope flags mismatch:
+after transform: ScopeId(2): ScopeFlags(0x0)
+rebuilt        : ScopeId(2): ScopeFlags(Function)
+Bindings mismatch:
+after transform: ScopeId(3): ["Merge", "y"]
+rebuilt        : ScopeId(3): ["Merge"]
+Scope flags mismatch:
+after transform: ScopeId(3): ScopeFlags(0x0)
+rebuilt        : ScopeId(3): ScopeFlags(Function)
+Bindings mismatch:
+after transform: ScopeId(4): ["NestOuter", "a", "b"]
+rebuilt        : ScopeId(4): ["NestOuter"]
+Scope flags mismatch:
+after transform: ScopeId(4): ScopeFlags(0x0)
+rebuilt        : ScopeId(4): ScopeFlags(Function)
+Bindings mismatch:
+after transform: ScopeId(6): ["NestInner", "a", "b"]
+rebuilt        : ScopeId(6): ["NestInner"]
+Scope flags mismatch:
+after transform: ScopeId(6): ScopeFlags(0x0)
+rebuilt        : ScopeId(6): ScopeFlags(Function)
+Symbol reference IDs mismatch for "x":
+after transform: SymbolId(0): [ReferenceId(2), ReferenceId(4)]
+rebuilt        : SymbolId(0): [ReferenceId(7)]
 Symbol flags mismatch for "Foo":
 after transform: SymbolId(1): SymbolFlags(RegularEnum)
 rebuilt        : SymbolId(1): SymbolFlags(FunctionScopedVariable)
 Symbol reference IDs mismatch for "Foo":
-after transform: SymbolId(5): [ReferenceId(3), ReferenceId(4), ReferenceId(5), ReferenceId(6), ReferenceId(7), ReferenceId(8), ReferenceId(9)]
+after transform: SymbolId(14): [ReferenceId(8), ReferenceId(9), ReferenceId(10), ReferenceId(11), ReferenceId(12), ReferenceId(13), ReferenceId(14)]
 rebuilt        : SymbolId(2): [ReferenceId(0), ReferenceId(1), ReferenceId(2), ReferenceId(3), ReferenceId(4), ReferenceId(5), ReferenceId(6), ReferenceId(8)]
+Symbol flags mismatch for "Merge":
+after transform: SymbolId(5): SymbolFlags(RegularEnum)
+rebuilt        : SymbolId(3): SymbolFlags(FunctionScopedVariable)
+Symbol redeclarations mismatch for "Merge":
+after transform: SymbolId(5): [Span { start: 103, end: 108 }]
+rebuilt        : SymbolId(3): []
+Symbol reference IDs mismatch for "Merge":
+after transform: SymbolId(16): [ReferenceId(20), ReferenceId(21), ReferenceId(22)]
+rebuilt        : SymbolId(5): [ReferenceId(16), ReferenceId(17), ReferenceId(18), ReferenceId(19)]
+Symbol flags mismatch for "NestOuter":
+after transform: SymbolId(8): SymbolFlags(RegularEnum)
+rebuilt        : SymbolId(6): SymbolFlags(FunctionScopedVariable)
+Symbol flags mismatch for "NestInner":
+after transform: SymbolId(11): SymbolFlags(RegularEnum)
+rebuilt        : SymbolId(8): SymbolFlags(BlockScopedVariable)
+Symbol reference IDs mismatch for "NestInner":
+after transform: SymbolId(18): [ReferenceId(31), ReferenceId(32), ReferenceId(33), ReferenceId(34), ReferenceId(35)]
+rebuilt        : SymbolId(9): [ReferenceId(25), ReferenceId(26), ReferenceId(28), ReferenceId(29), ReferenceId(30), ReferenceId(31)]
 
 * export-elimination/input.ts
 Bindings mismatch:

diff --git a/...ance/tests/babel-plugin-transform-typescript/test/fixtures/enum-member-reference/input.ts b/...ance/tests/babel-plugin-transform-typescript/test/fixtures/enum-member-reference/input.ts
@@ -5,3 +5,17 @@ enum Foo {
     b = a,
     c = b + x,
 }
+
+enum Merge { x = Math.random() }
+enum Merge { y = x }
+
+enum NestOuter {
+    a,
+    b = (() => {
+        enum NestInner {
+            a = Math.random(),
+            b = a
+        }
+        return NestInner.b;
+    })()
+}
diff --git a/...nce/tests/babel-plugin-transform-typescript/test/fixtures/enum-member-reference/output.js b/...nce/tests/babel-plugin-transform-typescript/test/fixtures/enum-member-reference/output.js
@@ -5,3 +5,23 @@ var Foo = function(Foo) {
   Foo[Foo['c'] = Foo.b + x] = 'c';
   return Foo;
 }(Foo || {});
+var Merge = function(Merge) {
+  Merge[Merge['x'] = Math.random()] = 'x';
+  return Merge;
+}(Merge || {});
+Merge = function(Merge) {
+  Merge[Merge['y'] = Merge.x] = 'y';
+  return Merge;
+}(Merge || {});
+var NestOuter = function(NestOuter) {
+  NestOuter[NestOuter['a'] = 0] = 'a';
+  NestOuter[NestOuter['b'] = (() => {
+    let NestInner = function(NestInner) {
+      NestInner[NestInner['a'] = Math.random()] = 'a';
+      NestInner[NestInner['b'] = NestInner.a] = 'b';
+      return NestInner;
+    }({});
+    return NestInner.b;
+  })()] = 'b';
+  return NestOuter;
+}(NestOuter || {});