Implement block scoping approach for using declaration shadowing in for-of loops

Copilot · weswigham · Copilot · commit 8f3df0254408 · 2025-06-25T19:31:16.000Z
Co-authored-by: weswigham &lt;2932786+weswigham@users.noreply.github.com&gt;
diff --git a/src/compiler/transformers/esnext.ts b/src/compiler/transformers/esnext.ts
@@ -15,7 +15,6 @@ import {
     ExportSpecifier,
     Expression,
     firstOrUndefined,
-    forEachChild,
     ForOfStatement,
     ForStatement,
     GeneratedIdentifierFlags,
@@ -37,7 +36,6 @@ import {
     isPrologueDirective,
     isSourceFile,
     isStatement,
-    isVariableDeclaration,
     isVariableDeclarationList,
     isVariableStatement,
     ModifierFlags,
@@ -297,55 +295,7 @@ export function transformESNext(context: TransformationContext): (x: SourceFile
     /**
      * Collects all variable declarations that shadow a given identifier name in a statement.
      */
-    function collectShadowingVariables(statement: Statement, shadowedName: string): VariableDeclaration[] {
-        const shadowingVars: VariableDeclaration[] = [];
-        
-        function visit(node: Node): void {
-            if (isVariableStatement(node)) {
-                for (const declaration of node.declarationList.declarations) {
-                    if (isIdentifier(declaration.name) && declaration.name.escapedText === shadowedName) {
-                        shadowingVars.push(declaration);
-                    }
-                }
-            }
-            forEachChild(node, visit);
-        }
-        
-        visit(statement);
-        return shadowingVars;
-    }
 
-    /**
-     * Creates a visitor that renames shadowing variables to avoid conflicts.
-     */
-    function createShadowingVariableRenamer(shadowedName: string): (node: Node) => VisitResult<Node> {
-        const renamingMap = new Map<string, Identifier>();
-        
-        return function renameShadowingVariables(node: Node): VisitResult<Node> {
-            if (isVariableDeclaration(node) && isIdentifier(node.name) && node.name.escapedText === shadowedName) {
-                // Create a unique name for this shadowing variable
-                const uniqueName = factory.createUniqueName(shadowedName as string, GeneratedIdentifierFlags.Optimistic);
-                renamingMap.set(node.name.escapedText as string, uniqueName);
-                
-                return factory.updateVariableDeclaration(
-                    node,
-                    uniqueName,
-                    node.exclamationToken,
-                    node.type,
-                    visitNode(node.initializer, renameShadowingVariables, isExpression)
-                );
-            }
-            
-            if (isIdentifier(node)) {
-                const renamed = renamingMap.get(node.escapedText as string);
-                if (renamed) {
-                    return renamed;
-                }
-            }
-            
-            return visitEachChild(node, renameShadowingVariables, context);
-        };
-    }
 
     function visitForOfStatement(node: ForOfStatement) {
         if (isUsingVariableDeclarationList(node.initializer)) {
@@ -356,11 +306,12 @@ export function transformESNext(context: TransformationContext): (x: SourceFile
             // produces a shallow transformation to:
             //
             //  for (const x_1 of y) {
-            //    using x = x;
-            //    ...
+            //    using x = x_1;
+            //    { ... }
             //  }
             //
-            // before handing the shallow transformation back to the visitor for an in-depth transformation.
+            // where the original loop body is wrapped in an additional block scope
+            // to handle shadowing variables naturally through block scoping.
             const forInitializer = node.initializer;
             const forDecl = firstOrUndefined(forInitializer.declarations) || factory.createVariableDeclaration(factory.createTempVariable(/*recordTempVariable*/ undefined));
 
@@ -370,18 +321,10 @@ export function transformESNext(context: TransformationContext): (x: SourceFile
             const usingVarList = factory.createVariableDeclarationList([usingVar], isAwaitUsing ? NodeFlags.AwaitUsing : NodeFlags.Using);
             const usingVarStatement = factory.createVariableStatement(/*modifiers*/ undefined, usingVarList);
             
-            // Check if the loop body contains shadowing variables and rename them if necessary
-            const shadowedName = isIdentifier(forDecl.name) ? forDecl.name.escapedText as string : undefined;
-            let transformedStatement = node.statement;
-            
-            if (shadowedName) {
-                const shadowingVars = collectShadowingVariables(node.statement, shadowedName);
-                if (shadowingVars.length > 0) {
-                    // Apply the renaming visitor to the loop body
-                    const renamer = createShadowingVariableRenamer(shadowedName);
-                    transformedStatement = visitNode(node.statement, renamer, isStatement);
-                }
-            }
+            // Wrap the original loop body in an additional block scope to handle shadowing
+            const wrappedStatement = isBlock(node.statement) ? 
+                node.statement : 
+                factory.createBlock([node.statement], /*multiLine*/ true);
             
             return visitNode(
                 factory.updateForOfStatement(
@@ -391,15 +334,10 @@ export function transformESNext(context: TransformationContext): (x: SourceFile
                         factory.createVariableDeclaration(temp),
                     ], NodeFlags.Const),
                     node.expression,
-                    isBlock(transformedStatement) ?
-                        factory.updateBlock(transformedStatement, [
-                            usingVarStatement,
-                            ...transformedStatement.statements,
-                        ]) :
-                        factory.createBlock([
-                            usingVarStatement,
-                            transformedStatement,
-                        ], /*multiLine*/ true),
+                    factory.createBlock([
+                        usingVarStatement,
+                        wrappedStatement,
+                    ], /*multiLine*/ true),
                 ),
                 visitor,
                 isStatement,
diff --git a/tests/baselines/reference/awaitUsingDeclarationsInForAwaitOf(target=es2015).js b/tests/baselines/reference/awaitUsingDeclarationsInForAwaitOf(target=es2015).js
@@ -88,6 +88,8 @@ function main() {
                 const env_1 = { stack: [], error: void 0, hasError: false };
                 try {
                     const d1 = __addDisposableResource(env_1, d1_1, true);
+                    {
+                    }
                 }
                 catch (e_2) {
                     env_1.error = e_2;
diff --git a/tests/baselines/reference/awaitUsingDeclarationsInForAwaitOf(target=es2017).js b/tests/baselines/reference/awaitUsingDeclarationsInForAwaitOf(target=es2017).js
@@ -76,6 +76,8 @@ async function main() {
             const env_1 = { stack: [], error: void 0, hasError: false };
             try {
                 const d1 = __addDisposableResource(env_1, d1_1, true);
+                {
+                }
             }
             catch (e_2) {
                 env_1.error = e_2;
diff --git a/tests/baselines/reference/awaitUsingDeclarationsInForAwaitOf(target=es2022).js b/tests/baselines/reference/awaitUsingDeclarationsInForAwaitOf(target=es2022).js
@@ -64,6 +64,8 @@ async function main() {
         const env_1 = { stack: [], error: void 0, hasError: false };
         try {
             const d1 = __addDisposableResource(env_1, d1_1, true);
+            {
+            }
         }
         catch (e_1) {
             env_1.error = e_1;
diff --git a/tests/baselines/reference/awaitUsingDeclarationsInForAwaitOf(target=es5).js b/tests/baselines/reference/awaitUsingDeclarationsInForAwaitOf(target=es5).js
@@ -128,6 +128,8 @@ function main() {
                 case 3:
                     _j.trys.push([3, 4, 5, 8]);
                     d1 = __addDisposableResource(env_1, d1_1, true);
+                    {
+                    }
                     return [3 /*break*/, 8];
                 case 4:
                     e_1 = _j.sent();
diff --git a/tests/baselines/reference/awaitUsingDeclarationsInForAwaitOf.3(target=es5).js b/tests/baselines/reference/awaitUsingDeclarationsInForAwaitOf.3(target=es5).js
@@ -118,7 +118,9 @@ try {
         var env_1 = { stack: [], error: void 0, hasError: false };
         try {
             var _e = __addDisposableResource(env_1, _e_1, true);
-            ;
+            {
+                ;
+            }
         }
         catch (e_2) {
             env_1.error = e_2;
@@ -159,7 +161,9 @@ export function test() {
                 case 3:
                     _f.trys.push([3, 4, 5, 8]);
                     _b = __addDisposableResource(env_2, _b_1, true);
-                    ;
+                    {
+                        ;
+                    }
                     return [3 /*break*/, 8];
                 case 4:
                     e_3 = _f.sent();
diff --git a/tests/baselines/reference/awaitUsingDeclarationsInForOf.1(target=es2015).js b/tests/baselines/reference/awaitUsingDeclarationsInForOf.1(target=es2015).js
@@ -77,6 +77,8 @@ function main() {
             const env_1 = { stack: [], error: void 0, hasError: false };
             try {
                 const d1 = __addDisposableResource(env_1, d1_1, true);
+                {
+                }
             }
             catch (e_1) {
                 env_1.error = e_1;
diff --git a/tests/baselines/reference/awaitUsingDeclarationsInForOf.1(target=es2017).js b/tests/baselines/reference/awaitUsingDeclarationsInForOf.1(target=es2017).js
@@ -65,6 +65,8 @@ async function main() {
         const env_1 = { stack: [], error: void 0, hasError: false };
         try {
             const d1 = __addDisposableResource(env_1, d1_1, true);
+            {
+            }
         }
         catch (e_1) {
             env_1.error = e_1;
diff --git a/tests/baselines/reference/awaitUsingDeclarationsInForOf.1(target=es2022).js b/tests/baselines/reference/awaitUsingDeclarationsInForOf.1(target=es2022).js
@@ -65,6 +65,8 @@ async function main() {
         const env_1 = { stack: [], error: void 0, hasError: false };
         try {
             const d1 = __addDisposableResource(env_1, d1_1, true);
+            {
+            }
         }
         catch (e_1) {
             env_1.error = e_1;
diff --git a/tests/baselines/reference/awaitUsingDeclarationsInForOf.1(target=es5).js b/tests/baselines/reference/awaitUsingDeclarationsInForOf.1(target=es5).js
@@ -117,6 +117,8 @@ function main() {
                 case 2:
                     _d.trys.push([2, 3, 4, 7]);
                     d1 = __addDisposableResource(env_1, d1_1, true);
+                    {
+                    }
                     return [3 /*break*/, 7];
                 case 3:
                     e_1 = _d.sent();
diff --git a/tests/baselines/reference/awaitUsingDeclarationsInForOf.5(target=es5).js b/tests/baselines/reference/awaitUsingDeclarationsInForOf.5(target=es5).js
@@ -107,7 +107,9 @@ for (var _i = 0, x_1 = x; _i < x_1.length; _i++) {
     var env_1 = { stack: [], error: void 0, hasError: false };
     try {
         var _a = __addDisposableResource(env_1, _a_1, true);
-        ;
+        {
+            ;
+        }
     }
     catch (e_1) {
         env_1.error = e_1;
@@ -135,7 +137,9 @@ export function test() {
                 case 2:
                     _b.trys.push([2, 3, 4, 7]);
                     _a = __addDisposableResource(env_2, _a_2, true);
-                    ;
+                    {
+                        ;
+                    }
                     return [3 /*break*/, 7];
                 case 3:
                     e_2 = _b.sent();
diff --git a/tests/baselines/reference/usingDeclarationsInForAwaitOf(target=es2015).js b/tests/baselines/reference/usingDeclarationsInForAwaitOf(target=es2015).js
@@ -87,6 +87,8 @@ function main() {
                 const env_1 = { stack: [], error: void 0, hasError: false };
                 try {
                     const d1 = __addDisposableResource(env_1, d1_1, false);
+                    {
+                    }
                 }
                 catch (e_2) {
                     env_1.error = e_2;
diff --git a/tests/baselines/reference/usingDeclarationsInForAwaitOf(target=es2017).js b/tests/baselines/reference/usingDeclarationsInForAwaitOf(target=es2017).js
@@ -77,6 +77,8 @@ async function main() {
             const env_1 = { stack: [], error: void 0, hasError: false };
             try {
                 const d1 = __addDisposableResource(env_1, d1_1, false);
+                {
+                }
             }
             catch (e_2) {
                 env_1.error = e_2;
diff --git a/tests/baselines/reference/usingDeclarationsInForAwaitOf(target=es2022).js b/tests/baselines/reference/usingDeclarationsInForAwaitOf(target=es2022).js
@@ -65,6 +65,8 @@ async function main() {
         const env_1 = { stack: [], error: void 0, hasError: false };
         try {
             const d1 = __addDisposableResource(env_1, d1_1, false);
+            {
+            }
         }
         catch (e_1) {
             env_1.error = e_1;
diff --git a/tests/baselines/reference/usingDeclarationsInForAwaitOf(target=es5).js b/tests/baselines/reference/usingDeclarationsInForAwaitOf(target=es5).js
@@ -123,6 +123,8 @@ function main() {
                     env_1 = { stack: [], error: void 0, hasError: false };
                     try {
                         d1 = __addDisposableResource(env_1, d1_1, false);
+                        {
+                        }
                     }
                     catch (e_2) {
                         env_1.error = e_2;
diff --git a/tests/baselines/reference/usingDeclarationsInForOf.1(target=es2015).js b/tests/baselines/reference/usingDeclarationsInForOf.1(target=es2015).js
@@ -62,6 +62,8 @@ for (const d1_1 of [{ [Symbol.dispose]() { } }, null, undefined]) {
     const env_1 = { stack: [], error: void 0, hasError: false };
     try {
         const d1 = __addDisposableResource(env_1, d1_1, false);
+        {
+        }
     }
     catch (e_1) {
         env_1.error = e_1;
diff --git a/tests/baselines/reference/usingDeclarationsInForOf.1(target=es2017).js b/tests/baselines/reference/usingDeclarationsInForOf.1(target=es2017).js
@@ -62,6 +62,8 @@ for (const d1_1 of [{ [Symbol.dispose]() { } }, null, undefined]) {
     const env_1 = { stack: [], error: void 0, hasError: false };
     try {
         const d1 = __addDisposableResource(env_1, d1_1, false);
+        {
+        }
     }
     catch (e_1) {
         env_1.error = e_1;
diff --git a/tests/baselines/reference/usingDeclarationsInForOf.1(target=es2022).js b/tests/baselines/reference/usingDeclarationsInForOf.1(target=es2022).js
@@ -62,6 +62,8 @@ for (const d1_1 of [{ [Symbol.dispose]() { } }, null, undefined]) {
     const env_1 = { stack: [], error: void 0, hasError: false };
     try {
         const d1 = __addDisposableResource(env_1, d1_1, false);
+        {
+        }
     }
     catch (e_1) {
         env_1.error = e_1;
diff --git a/tests/baselines/reference/usingDeclarationsInForOf.1(target=es5).js b/tests/baselines/reference/usingDeclarationsInForOf.1(target=es5).js
@@ -64,6 +64,8 @@ for (var _i = 0, _b = [(_a = {}, _a[Symbol.dispose] = function () { }, _a), null
     var env_1 = { stack: [], error: void 0, hasError: false };
     try {
         var d1 = __addDisposableResource(env_1, d1_1, false);
+        {
+        }
     }
     catch (e_1) {
         env_1.error = e_1;
diff --git a/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=es2015).errors.txt b/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=es2015).errors.txt
@@ -20,4 +20,5 @@ main.ts(12,7): error TS2481: Cannot initialize outer scoped variable 'baz' in th
     
 ==== tslib.d.ts (0 errors) ====
     export declare function __addDisposableResource<T>(env: any, value: T, async: boolean): T;
-    export declare function __disposeResources(env: any): void;
+    export declare function __disposeResources(env: any): void;
+    
diff --git a/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=es2015).js b/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=es2015).js
@@ -17,7 +17,8 @@ for (using baz of []) {
 
 //// [tslib.d.ts]
 export declare function __addDisposableResource<T>(env: any, value: T, async: boolean): T;
-export declare function __disposeResources(env: any): void;
+export declare function __disposeResources(env: any): void;
+
 
 //// [main.js]
 var __addDisposableResource = (this && this.__addDisposableResource) || function (env, value, async) {
@@ -78,7 +79,9 @@ for (const foo_1 of []) {
     const env_1 = { stack: [], error: void 0, hasError: false };
     try {
         const foo = __addDisposableResource(env_1, foo_1, false);
-        const foo_2 = new Foo();
+        {
+            const foo = new Foo();
+        }
     }
     catch (e_1) {
         env_1.error = e_1;
@@ -92,7 +95,9 @@ for (const bar_1 of []) {
     const env_2 = { stack: [], error: void 0, hasError: false };
     try {
         const bar = __addDisposableResource(env_2, bar_1, false);
-        let bar_2 = "test";
+        {
+            let bar = "test";
+        }
     }
     catch (e_2) {
         env_2.error = e_2;
@@ -106,7 +111,9 @@ for (const baz_1 of []) {
     const env_3 = { stack: [], error: void 0, hasError: false };
     try {
         const baz = __addDisposableResource(env_3, baz_1, false);
-        var baz_2 = 42;
+        {
+            var baz = 42;
+        }
     }
     catch (e_3) {
         env_3.error = e_3;
diff --git a/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=es2017).errors.txt b/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=es2017).errors.txt
@@ -20,4 +20,5 @@ main.ts(12,7): error TS2481: Cannot initialize outer scoped variable 'baz' in th
     
 ==== tslib.d.ts (0 errors) ====
     export declare function __addDisposableResource<T>(env: any, value: T, async: boolean): T;
-    export declare function __disposeResources(env: any): void;
+    export declare function __disposeResources(env: any): void;
+    
diff --git a/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=es2017).js b/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=es2017).js
diff --git a/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=es2022).errors.txt b/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=es2022).errors.txt
diff --git a/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=es2022).js b/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=es2022).js
diff --git a/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=es5).errors.txt b/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=es5).errors.txt
diff --git a/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=es5).js b/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=es5).js
diff --git a/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=esnext).errors.txt b/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=esnext).errors.txt
diff --git a/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=esnext).js b/tests/baselines/reference/usingDeclarationsInForOfShadowing(target=esnext).js

Original file line number	Diff line number	Diff line change
`@@ -88,6 +88,8 @@ function main() {`
`88`	`88`	`const env_1 = { stack: [], error: void 0, hasError: false };`
`89`	`89`	`try {`
`90`	`90`	`const d1 = __addDisposableResource(env_1, d1_1, true);`
	`91`	`+ {`
	`92`	`+ }`
`91`	`93`	`}`
`92`	`94`	`catch (e_2) {`
`93`	`95`	`env_1.error = e_2;`
Original file line number	Diff line number	Diff line change
`@@ -76,6 +76,8 @@ async function main() {`
`76`	`76`	`const env_1 = { stack: [], error: void 0, hasError: false };`
`77`	`77`	`try {`
`78`	`78`	`const d1 = __addDisposableResource(env_1, d1_1, true);`
	`79`	`+ {`
	`80`	`+ }`
`79`	`81`	`}`
`80`	`82`	`catch (e_2) {`
`81`	`83`	`env_1.error = e_2;`
Original file line number	Diff line number	Diff line change
`@@ -64,6 +64,8 @@ async function main() {`
`64`	`64`	`const env_1 = { stack: [], error: void 0, hasError: false };`
`65`	`65`	`try {`
`66`	`66`	`const d1 = __addDisposableResource(env_1, d1_1, true);`
	`67`	`+ {`
	`68`	`+ }`
`67`	`69`	`}`
`68`	`70`	`catch (e_1) {`
`69`	`71`	`env_1.error = e_1;`
Original file line number	Diff line number	Diff line change
`@@ -77,6 +77,8 @@ function main() {`
`77`	`77`	`const env_1 = { stack: [], error: void 0, hasError: false };`
`78`	`78`	`try {`
`79`	`79`	`const d1 = __addDisposableResource(env_1, d1_1, true);`
	`80`	`+ {`
	`81`	`+ }`
`80`	`82`	`}`
`81`	`83`	`catch (e_1) {`
`82`	`84`	`env_1.error = e_1;`
Original file line number	Diff line number	Diff line change
`@@ -65,6 +65,8 @@ async function main() {`
`65`	`65`	`const env_1 = { stack: [], error: void 0, hasError: false };`
`66`	`66`	`try {`
`67`	`67`	`const d1 = __addDisposableResource(env_1, d1_1, true);`
	`68`	`+ {`
	`69`	`+ }`
`68`	`70`	`}`
`69`	`71`	`catch (e_1) {`
`70`	`72`	`env_1.error = e_1;`