fix(transformer): use UIDs in TS namespace transforms (#3395)

Use the `TraverseCtx::generate_uid` method introduced in #3395 to fix
some of the TS namespace test cases.

But... I honestly have no idea what I'm doing here!

I am running up against a combination of 3 different areas where I know
very little:

1. I am unfamiliar with `Semantic`.
2. I am unfamiliar with Oxc's conventions for writing transforms.
3. I don't "speak" Typescript!

This PR should not be merged as is. I'm pretty sure it's wrong. I've
done it mostly just to test out `generate_uid`.

In particular:

1. Is `SymbolFlags::FunctionScopedVariable` the right flags to use in
all cases here?
2. `generate_uid` creates a symbol, and registers a binding for it in
the scope tree. So if there are any branches in this logic where `name`
doesn't actually get used after `generate_uid` is called, then it's not
right.
3. Identifiers which are added to AST should also have corresponding
`ReferenceId`s created for them (but I imagine this is also missing in
many other places in the transformer).

On point 2: We could split up `generate_uid` into 2 functions. One
function would find a valid UID name *but not register a binding for
it*. If you want to actually use that name, you'd call a 2nd function to
generate the binding. Would that be a better API?

Could someone help me out to progress this please?

(Sorry my lack of knowledge is a bit useless here. I will learn all
these things in time, but just trying to be honest about where I'm at
right now. I'm sure I could figure it out myself, but it would take me
hours, whereas others will probably look at it and know what to do in
about 5 mins.)

crates/oxc_semantic/src/scope.rs

@@ -151,6 +151,10 @@ impl ScopeTree {
         self.bindings[scope_id].insert(name, symbol_id);
     }
 
+    pub fn remove_binding(&mut self, scope_id: ScopeId, name: &CompactStr) {
+        self.bindings[scope_id].shift_remove(name);
+    }
+
     pub(crate) fn add_unresolved_reference(
         &mut self,
         scope_id: ScopeId,

crates/oxc_transformer/src/lib.rs

@@ -92,8 +92,8 @@ impl<'a> Transformer<'a> {
 }
 
 impl<'a> Traverse<'a> for Transformer<'a> {
-    fn enter_program(&mut self, program: &mut Program<'a>, _ctx: &mut TraverseCtx<'a>) {
+    fn enter_program(&mut self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) {
-        self.x0_typescript.transform_program(program);
+        self.x0_typescript.transform_program(program, ctx);
     }
 
     fn exit_program(&mut self, program: &mut Program<'a>, _ctx: &mut TraverseCtx<'a>) {

crates/oxc_transformer/src/typescript/mod.rs

@@ -68,8 +68,8 @@ impl<'a> TypeScript<'a> {
 
 // Transforms
 impl<'a> TypeScript<'a> {
-    pub fn transform_program(&self, program: &mut Program<'a>) {
+    pub fn transform_program(&self, program: &mut Program<'a>, ctx: &mut TraverseCtx) {
-        self.transform_program_for_namespace(program);
+        self.transform_program_for_namespace(program, ctx);
     }
 
     pub fn transform_program_on_exit(&self, program: &mut Program<'a>) {

crates/oxc_transformer/src/typescript/namespace.rs

@@ -4,14 +4,22 @@ use super::{diagnostics::ambient_module_nested, TypeScript};
 
 use oxc_allocator::{Box, Vec};
 use oxc_ast::{ast::*, syntax_directed_operations::BoundNames};
-use oxc_span::{Atom, SPAN};
+use oxc_span::{Atom, CompactStr, SPAN};
-use oxc_syntax::operator::{AssignmentOperator, LogicalOperator};
+use oxc_syntax::{
+    operator::{AssignmentOperator, LogicalOperator},
+    symbol::SymbolFlags,
+};
+use oxc_traverse::TraverseCtx;
 
 // TODO:
 // 1. register scope for the newly created function: <https://github.com/babel/babel/blob/08b0472069cd207f043dd40a4d157addfdd36011/packages/babel-plugin-transform-typescript/src/namespace.ts#L38>
 impl<'a> TypeScript<'a> {
     // `namespace Foo { }` -> `let Foo; (function (_Foo) { })(Foo || (Foo = {}));`
-    pub(super) fn transform_program_for_namespace(&self, program: &mut Program<'a>) {
+    pub(super) fn transform_program_for_namespace(
+        &self,
+        program: &mut Program<'a>,
+        ctx: &mut TraverseCtx,
+    ) {
         // namespace declaration is only allowed at the top level
 
         if !has_namespace(program.body.as_slice()) {
@@ -31,7 +39,7 @@ impl<'a> TypeScript<'a> {
                 Statement::TSModuleDeclaration(decl) => {
                     if !decl.modifiers.is_contains_declare() {
                         if let Some(transformed_stmt) =
-                            self.handle_nested(self.ctx.ast.copy(&decl).unbox(), None)
+                            self.handle_nested(self.ctx.ast.copy(&decl).unbox(), None, ctx)
                         {
                             let name = decl.id.name();
                             if names.insert(name.clone()) {
@@ -51,7 +59,7 @@ impl<'a> TypeScript<'a> {
                         {
                             if !decl.modifiers.is_contains_declare() {
                                 if let Some(transformed_stmt) =
-                                    self.handle_nested(self.ctx.ast.copy(decl), None)
+                                    self.handle_nested(self.ctx.ast.copy(decl), None, ctx)
                                 {
                                     let name = decl.id.name();
                                     if names.insert(name.clone()) {
@@ -119,12 +127,17 @@ impl<'a> TypeScript<'a> {
         &self,
         decl: TSModuleDeclaration<'a>,
         parent_export: Option<Expression<'a>>,
+        ctx: &mut TraverseCtx,
     ) -> Option<Statement<'a>> {
         let mut names: FxHashSet<Atom<'a>> = FxHashSet::default();
 
         let real_name = decl.id.name();
 
-        let name = self.ctx.ast.new_atom(&format!("_{}", real_name.clone())); // path.scope.generateUid(realName.name);
+        // TODO: This binding is created in wrong scope.
+        // Needs to be created in scope of function which `transform_namespace` creates below.
+        let name = self.ctx.ast.new_atom(
+            &ctx.generate_uid_in_current_scope(real_name, SymbolFlags::FunctionScopedVariable),
+        );
 
         let namespace_top_level = if let Some(body) = decl.body {
             match body {
@@ -161,7 +174,7 @@ impl<'a> TypeScript<'a> {
                     }
 
                     let module_name = decl.id.name().clone();
-                    if let Some(transformed) = self.handle_nested(decl.unbox(), None) {
+                    if let Some(transformed) = self.handle_nested(decl.unbox(), None, ctx) {
                         is_empty = false;
                         if names.insert(module_name.clone()) {
                             new_stmts.push(Statement::from(
@@ -233,6 +246,7 @@ impl<'a> TypeScript<'a> {
                                     Some(self.ctx.ast.identifier_reference_expression(
                                         IdentifierReference::new(SPAN, name.clone()),
                                     )),
+                                    ctx,
                                 ) {
                                     is_empty = false;
                                     if names.insert(module_name.clone()) {
@@ -267,6 +281,11 @@ impl<'a> TypeScript<'a> {
         }
 
         if is_empty {
+            // Delete the scope binding that `ctx.generate_uid_in_current_scope` created above,
+            // as no binding is actually being created
+            let current_scope_id = ctx.current_scope_id();
+            ctx.scopes_mut().remove_binding(current_scope_id, &CompactStr::from(name.as_str()));
+
             return None;
         }
 

tasks/transform_conformance/babel.snap.md

@@ -1,6 +1,6 @@
 commit: 4bd1b2c2
 
-Passed: 309/351
+Passed: 313/351
 
 # All Passed:
 * babel-preset-react
@@ -21,18 +21,14 @@ Passed: 309/351
 * opts/optimizeConstEnums/input.ts
 * opts/rewriteImportExtensions/input.ts
 
-# babel-plugin-transform-typescript (123/154)
+# babel-plugin-transform-typescript (127/154)
 * enum/mix-references/input.ts
 * enum/scoped/input.ts
 * enum/ts5.0-const-foldable/input.ts
 * exports/declared-types/input.ts
 * imports/enum-value/input.ts
 * imports/type-only-export-specifier-2/input.ts
-* namespace/contentious-names/input.ts
 * namespace/empty-removed/input.ts
-* namespace/module-nested/input.ts
-* namespace/module-nested-export/input.ts
-* namespace/multiple/input.ts
 * namespace/mutable-fail/input.ts
 * namespace/namespace-flag/input.ts
 * namespace/namespace-nested-module/input.ts