fixes to binding destructuriing in loops and functions (#1182)

tomblind · web-flow · commit 1f3a505488c3 · 2021-12-02T22:19:53.000+01:00
Co-authored-by: Tom &lt;tomblind@users.noreply.github.com&gt;
diff --git a/src/transformation/utils/scope.ts b/src/transformation/utils/scope.ts
@@ -14,6 +14,7 @@ export enum ScopeType {
     Block = 1 << 5,
     Try = 1 << 6,
     Catch = 1 << 7,
+    LoopInitializer = 1 << 8,
 }
 
 interface FunctionDefinitionInfo {
diff --git a/src/transformation/visitors/function.ts b/src/transformation/visitors/function.ts
@@ -85,7 +85,11 @@ export function transformFunctionBodyHeader(
             }
 
             // Binding pattern
-            bindingPatternDeclarations.push(...transformBindingPattern(context, declaration.name, identifier));
+            const name = declaration.name;
+            const [precedingStatements, bindings] = transformInPrecedingStatementScope(context, () =>
+                transformBindingPattern(context, name, identifier)
+            );
+            bindingPatternDeclarations.push(...precedingStatements, ...bindings);
         } else if (declaration.initializer !== undefined) {
             // Default parameter
             headerStatements.push(
diff --git a/src/transformation/visitors/loops/utils.ts b/src/transformation/visitors/loops/utils.ts
@@ -1,6 +1,7 @@
 import * as ts from "typescript";
 import * as lua from "../../../LuaAST";
 import { TransformationContext } from "../../context";
+import { transformInPrecedingStatementScope } from "../../utils/preceding-statements";
 import { performHoisting, popScope, pushScope, ScopeType } from "../../utils/scope";
 import { isAssignmentPattern } from "../../utils/typescript";
 import { transformAssignment } from "../binary-expression/assignments";
@@ -49,14 +50,20 @@ export function transformForInitializer(
 ): lua.Identifier {
     const valueVariable = lua.createIdentifier("____value");
 
+    pushScope(context, ScopeType.LoopInitializer);
+
     if (ts.isVariableDeclarationList(initializer)) {
         // Declaration of new variable
 
         const binding = getVariableDeclarationBinding(context, initializer);
         if (ts.isArrayBindingPattern(binding) || ts.isObjectBindingPattern(binding)) {
-            block.statements.unshift(...transformBindingPattern(context, binding, valueVariable));
+            const [precedingStatements, bindings] = transformInPrecedingStatementScope(context, () =>
+                transformBindingPattern(context, binding, valueVariable)
+            );
+            block.statements.unshift(...precedingStatements, ...bindings);
         } else {
             // Single variable declared in for loop
+            popScope(context);
             return transformIdentifier(context, binding);
         }
     } else {
@@ -69,6 +76,7 @@ export function transformForInitializer(
         );
     }
 
+    popScope(context);
     return valueVariable;
 }
 
diff --git a/test/unit/destructuring.spec.ts b/test/unit/destructuring.spec.ts
@@ -53,6 +53,17 @@ test("in function parameter creates local variables", () => {
     expect(code).toContain("local b =");
 });
 
+test("in function parameter creates local variables in correct scope", () => {
+    util.testFunction`
+        let x = 7;
+        function foo([x]: [number]) {
+            x *= 2;
+        }
+        foo([1]);
+        return x;
+    `.expectToMatchJsResult();
+});
+
 test.each(testCases)("in variable declaration (%p)", ({ binding, value }) => {
     util.testFunction`
         let ${allBindings};
diff --git a/test/unit/loops.spec.ts b/test/unit/loops.spec.ts
@@ -279,6 +279,27 @@ test("forof destructing", () => {
     `.expectToMatchJsResult();
 });
 
+test("forof destructing scope", () => {
+    util.testFunction`
+        let x = 7;
+        for (let [x] of [[1], [2], [3]]) {
+            x *= 2;
+        }
+        return x;
+    `.expectToMatchJsResult();
+});
+
+// This catches the case where x is falsely seen as globally scoped and the 'local' is stripped out
+test("forof destructing scope (global)", () => {
+    util.testModule`
+        let x = 7;
+        for (let [x] of [[1], [2], [3]]) {
+            x *= 2;
+        }
+        if (x !== 7) throw x;
+    `.expectNoExecutionError();
+});
+
 test("forof nested destructing", () => {
     util.testFunction`
         const obj = { a: [3], b: [5] };

Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,7 @@ export enum ScopeType {`
`14`	`14`	`Block = 1 << 5,`
`15`	`15`	`Try = 1 << 6,`
`16`	`16`	`Catch = 1 << 7,`
	`17`	`+ LoopInitializer = 1 << 8,`
`17`	`18`	`}`
`18`	`19`
`19`	`20`	`interface FunctionDefinitionInfo {`