Skip to content

Commit a834c82

Browse files
ark120202ark120202
committed
Error on vars in loop variable declarations
1 parent fcb88da commit a834c82

File tree

7 files changed

+67
-34
lines changed

7 files changed

+67
-34
lines changed

‎src/transformation/utils/lua-ast.ts‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import { LuaTarget } from "../../CompilerOptions";
33
import * as lua from "../../LuaAST";
44
import { TransformationContext } from "../context";
55
import { getCurrentNamespace } from "../visitors/namespace";
6-
import { UnsupportedVarDeclaration } from "./errors";
76
import { createExportedIdentifier, getIdentifierExportScope } from "./export";
87
import { findScope, peekScope, ScopeType } from "./scope";
98
import { isFunctionType } from "./typescript";
@@ -120,21 +119,14 @@ export function createLocalOrExportedOrGlobalDeclaration(
120119
let declaration: lua.VariableDeclarationStatement | undefined;
121120
let assignment: lua.AssignmentStatement | undefined;
122121

122+
const isVariableDeclaration = tsOriginal !== undefined && ts.isVariableDeclaration(tsOriginal);
123123
const isFunctionDeclaration = tsOriginal !== undefined && ts.isFunctionDeclaration(tsOriginal);
124124

125125
const identifiers = Array.isArray(lhs) ? lhs : [lhs];
126126
if (identifiers.length === 0) {
127127
return [];
128128
}
129129

130-
let isVariableDeclaration = false;
131-
if (tsOriginal && ts.isVariableDeclaration(tsOriginal)) {
132-
isVariableDeclaration = true;
133-
if (tsOriginal.parent && (tsOriginal.parent.flags & (ts.NodeFlags.Let | ts.NodeFlags.Const)) === 0) {
134-
throw UnsupportedVarDeclaration(tsOriginal.parent);
135-
}
136-
}
137-
138130
const exportScope = overrideExportScope ?? getIdentifierExportScope(context, identifiers[0]);
139131
if (exportScope) {
140132
// exported

‎src/transformation/visitors/loops/for-in.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { ForbiddenForIn, UnsupportedForInVariable } from "../../utils/errors";
55
import { isArrayType } from "../../utils/typescript";
66
import { transformIdentifier } from "../identifier";
77
import { transformLoopBody } from "./body";
8+
import { getVariableDeclarationBinding } from "./utils";
89

910
export const transformForInStatement: FunctionVisitor<ts.ForInStatement> = (statement, context) => {
1011
if (isArrayType(context, context.checker.getTypeAtLocation(statement.expression))) {
@@ -22,11 +23,13 @@ export const transformForInStatement: FunctionVisitor<ts.ForInStatement> = (stat
2223
// TODO: After the transformation pipeline refactor we should look at refactoring this together with the
2324
// for-of initializer transformation.
2425
let iterationVariable: lua.Identifier;
25-
if (
26-
ts.isVariableDeclarationList(statement.initializer) &&
27-
ts.isIdentifier(statement.initializer.declarations[0].name)
28-
) {
29-
iterationVariable = transformIdentifier(context, statement.initializer.declarations[0].name);
26+
if (ts.isVariableDeclarationList(statement.initializer)) {
27+
const binding = getVariableDeclarationBinding(statement.initializer);
28+
if (!ts.isIdentifier(binding)) {
29+
throw UnsupportedForInVariable(statement.initializer);
30+
}
31+
32+
iterationVariable = transformIdentifier(context, binding);
3033
} else if (ts.isIdentifier(statement.initializer)) {
3134
// Iteration variable becomes ____key
3235
iterationVariable = lua.createIdentifier("____key");

‎src/transformation/visitors/loops/for-of.ts‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,24 @@ import { transformArguments } from "../call";
1616
import { transformIdentifier } from "../identifier";
1717
import { transformArrayBindingElement, transformVariableDeclaration } from "../variable-declaration";
1818
import { transformLoopBody } from "./body";
19+
import { getVariableDeclarationBinding } from "./utils";
1920

2021
function transformForOfInitializer(
2122
context: TransformationContext,
2223
initializer: ts.ForInitializer,
2324
expression: lua.Expression
2425
): lua.Statement | undefined {
2526
if (ts.isVariableDeclarationList(initializer)) {
27+
const binding = getVariableDeclarationBinding(initializer);
2628
// Declaration of new variable
27-
if (ts.isArrayBindingPattern(initializer.declarations[0].name)) {
28-
if (initializer.declarations[0].name.elements.length === 0) {
29+
if (ts.isArrayBindingPattern(binding)) {
30+
if (binding.elements.length === 0) {
2931
// Ignore empty destructuring assignment
3032
return undefined;
3133
}
3234

3335
expression = createUnpackCall(context, expression, initializer);
34-
} else if (ts.isObjectBindingPattern(initializer.declarations[0].name)) {
36+
} else if (ts.isObjectBindingPattern(binding)) {
3537
throw UnsupportedObjectDestructuringInForOf(initializer);
3638
}
3739

@@ -90,19 +92,19 @@ function transformForRangeStatement(
9092
throw InvalidForRangeCall(statement.initializer, "@forRange loop must declare its own control variable.");
9193
}
9294

93-
const controlDeclaration = statement.initializer.declarations[0];
94-
if (!ts.isIdentifier(controlDeclaration.name)) {
95+
const binding = getVariableDeclarationBinding(statement.initializer);
96+
if (!ts.isIdentifier(binding)) {
9597
throw InvalidForRangeCall(statement.initializer, "@forRange loop cannot use destructuring.");
9698
}
9799

98-
if (!isNumberType(context, context.checker.getTypeAtLocation(controlDeclaration))) {
100+
if (!isNumberType(context, context.checker.getTypeAtLocation(binding))) {
99101
throw InvalidForRangeCall(
100102
statement.expression,
101103
"@forRange function must return Iterable<number> or Array<number>."
102104
);
103105
}
104106

105-
const control = transformIdentifier(context, controlDeclaration.name);
107+
const control = transformIdentifier(context, binding);
106108
const signature = context.checker.getResolvedSignature(statement.expression);
107109
const [start, limit, step] = transformArguments(context, statement.expression.arguments, signature);
108110
return lua.createForStatement(block, control, start, limit, step, statement);
@@ -121,9 +123,9 @@ function transformForOfLuaIteratorStatement(
121123
if (ts.isVariableDeclarationList(statement.initializer)) {
122124
// Variables declared in for loop
123125
// for ${initializer} in ${iterable} do
124-
const initializerVariable = statement.initializer.declarations[0].name;
125-
if (ts.isArrayBindingPattern(initializerVariable)) {
126-
const identifiers = initializerVariable.elements.map(e => transformArrayBindingElement(context, e));
126+
const binding = getVariableDeclarationBinding(statement.initializer);
127+
if (ts.isArrayBindingPattern(binding)) {
128+
const identifiers = binding.elements.map(e => transformArrayBindingElement(context, e));
127129
if (identifiers.length === 0) {
128130
identifiers.push(lua.createAnonymousIdentifier());
129131
}
@@ -160,6 +162,7 @@ function transformForOfLuaIteratorStatement(
160162
// LuaIterator (no TupleReturn)
161163
if (
162164
ts.isVariableDeclarationList(statement.initializer) &&
165+
statement.initializer.declarations.length > 0 &&
163166
ts.isIdentifier(statement.initializer.declarations[0].name)
164167
) {
165168
// Single variable declared in for loop
@@ -191,15 +194,15 @@ function transformForOfArrayStatement(
191194
let valueVariable: lua.Identifier;
192195
if (ts.isVariableDeclarationList(statement.initializer)) {
193196
// Declaration of new variable
194-
const variables = statement.initializer.declarations[0].name;
195-
if (ts.isArrayBindingPattern(variables) || ts.isObjectBindingPattern(variables)) {
197+
const binding = getVariableDeclarationBinding(statement.initializer);
198+
if (ts.isArrayBindingPattern(binding) || ts.isObjectBindingPattern(binding)) {
196199
valueVariable = lua.createIdentifier("____values");
197200
const initializer = transformForOfInitializer(context, statement.initializer, valueVariable);
198201
if (initializer) {
199202
block.statements.unshift(initializer);
200203
}
201204
} else {
202-
valueVariable = transformIdentifier(context, variables);
205+
valueVariable = transformIdentifier(context, binding);
203206
}
204207
} else {
205208
// Assignment to existing variable
@@ -225,6 +228,7 @@ function transformForOfIteratorStatement(
225228
const iterable = context.transformExpression(statement.expression);
226229
if (
227230
ts.isVariableDeclarationList(statement.initializer) &&
231+
statement.initializer.declarations.length > 0 &&
228232
ts.isIdentifier(statement.initializer.declarations[0].name)
229233
) {
230234
// Single variable declared in for loop

‎src/transformation/visitors/loops/for.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@ import * as ts from "typescript";
22
import * as lua from "../../../LuaAST";
33
import { flatMap } from "../../../utils";
44
import { FunctionVisitor } from "../../context";
5-
import { transformVariableDeclaration } from "../variable-declaration";
5+
import { checkVariableDeclarationList, transformVariableDeclaration } from "../variable-declaration";
66
import { transformLoopBody } from "./body";
77

88
export const transformForStatement: FunctionVisitor<ts.ForStatement> = (statement, context) => {
99
const result: lua.Statement[] = [];
1010

1111
if (statement.initializer) {
1212
if (ts.isVariableDeclarationList(statement.initializer)) {
13+
checkVariableDeclarationList(statement.initializer);
1314
// local initializer = value
1415
result.push(...flatMap(statement.initializer.declarations, d => transformVariableDeclaration(context, d)));
1516
} else {

‎src/transformation/visitors/loops/utils.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import * as ts from "typescript";
2+
import { checkVariableDeclarationList } from "../variable-declaration";
3+
4+
export function getVariableDeclarationBinding(node: ts.VariableDeclarationList): ts.BindingName {
5+
checkVariableDeclarationList(node);
6+
7+
if (node.declarations.length === 0) {
8+
return ts.createIdentifier("____");
9+
}
10+
11+
return node.declarations[0].name;
12+
}

‎src/transformation/visitors/variable-declaration.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { assertNever, flatMap } from "../../utils";
44
import { FunctionVisitor, TransformationContext } from "../context";
55
import { isTupleReturnCall } from "../utils/annotations";
66
import { validateAssignment } from "../utils/assignment-validation";
7-
import { UnsupportedKind } from "../utils/errors";
7+
import { UnsupportedKind, UnsupportedVarDeclaration } from "../utils/errors";
88
import { addExportToIdentifier } from "../utils/export";
99
import { createLocalOrExportedOrGlobalDeclaration, createUnpackCall } from "../utils/lua-ast";
1010
import { LuaLibFeature, transformLuaLibFunction } from "../utils/lualib";
@@ -222,5 +222,15 @@ export function transformVariableDeclaration(
222222
}
223223
}
224224

225-
export const transformVariableStatement: FunctionVisitor<ts.VariableStatement> = (node, context) =>
226-
flatMap(node.declarationList.declarations, declaration => transformVariableDeclaration(context, declaration));
225+
export function checkVariableDeclarationList(node: ts.VariableDeclarationList): void {
226+
if ((node.flags & (ts.NodeFlags.Let | ts.NodeFlags.Const)) === 0) {
227+
throw UnsupportedVarDeclaration(node);
228+
}
229+
}
230+
231+
export const transformVariableStatement: FunctionVisitor<ts.VariableStatement> = (node, context) => {
232+
checkVariableDeclarationList(node.declarationList);
233+
return flatMap(node.declarationList.declarations, declaration =>
234+
transformVariableDeclaration(context, declaration)
235+
);
236+
};

‎test/unit/assignments.spec.ts‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,25 @@ test("let declaration", () => {
1010
expect(lua).toBe(`local foo = true`);
1111
});
1212

13-
test("`var` declaration is disallowed", () => {
13+
test("var declaration is disallowed", () => {
1414
util.testFunction`
1515
var foo = true;
1616
`.expectToHaveDiagnostics();
1717
});
1818

19-
// TODO:
20-
test.skip("`var` declaration in for...of loop is disallowed", () => {
19+
test("var declaration in for loop is disallowed", () => {
20+
util.testFunction`
21+
for (var foo = 0;;) {}
22+
`.expectToHaveDiagnostics();
23+
});
24+
25+
test("var declaration in for...in loop is disallowed", () => {
26+
util.testFunction`
27+
for (var foo in {}) {}
28+
`.expectToHaveDiagnostics();
29+
});
30+
31+
test("var declaration in for...of loop is disallowed", () => {
2132
util.testFunction`
2233
for (var foo of []) {}
2334
`.expectToHaveDiagnostics();

0 commit comments

Comments
 (0)