Skip to content

Fix broken for-in declaration #711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Perryvw merged 3 commits into from
Sep 1, 2019
Merged

Fix broken for-in declaration #711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac391cd
Fix busted for-in declaration
Perryvw Aug 28, 2019
be68c94
Fixed scoping issue once the for-in loop is finished
Perryvw Aug 28, 2019
6dc4568
replaced splice with unshift
Perryvw Aug 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions src/LuaTransformer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2628,10 +2628,6 @@ export class LuaTransformer {
}

public transformForInStatement(statement: ts.ForInStatement): StatementVisitResult {
// Get variable identifier
const variable = (statement.initializer as ts.VariableDeclarationList).declarations[0];
const identifier = variable.name as ts.Identifier;

// Transpile expression
const pairsIdentifier = tstl.createIdentifier("pairs");
const expression = this.transformExpression(statement.expression);
Expand All @@ -2643,7 +2639,30 @@ export class LuaTransformer {

const body = tstl.createBlock(this.transformLoopBody(statement));

return tstl.createForInStatement(body, [this.transformIdentifier(identifier)], [pairsCall], statement);
// Transform iteration variable
// TODO: After the transformation pipeline refactor we should look at refactoring this together with the
// for-of initializer transformation.
let iterationVariable: tstl.Identifier;
if (
ts.isVariableDeclarationList(statement.initializer) &&
ts.isIdentifier(statement.initializer.declarations[0].name)
) {
iterationVariable = this.transformIdentifier(statement.initializer.declarations[0].name);
} else if (ts.isIdentifier(statement.initializer)) {
// Iteration variable becomes ____key
iterationVariable = tstl.createIdentifier("____key");
// Push variable = ____key to the start of the loop body to match TS scoping
const initializer = tstl.createAssignmentStatement(
this.transformIdentifier(statement.initializer),
iterationVariable
);
body.statements.unshift(initializer);
} else {
// This should never occur
throw TSTLErrors.UnsupportedForInVariable(statement.initializer);
}

return tstl.createForInStatement(body, [iterationVariable], [pairsCall], statement);
}

public transformSwitchStatement(statement: ts.SwitchStatement): StatementVisitResult {
Expand Down
3 changes: 3 additions & 0 deletions src/TSTLErrors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ export const NonFlattenableDestructure = (node: ts.Node) =>
export const UndefinedFunctionDefinition = (functionSymbolId: number) =>
new Error(`Function definition for function symbol ${functionSymbolId} is undefined.`);

export const UnsupportedForInVariable = (initializer: ts.Node) =>
new TranspileError(`Unsuppored for-in variable kind.`, initializer);

export const UndefinedScope = () => new Error("Expected to pop a scope, but found undefined.");

export const UndefinedTypeNode = (node: ts.Node) => new TranspileError("Failed to resolve required type node.", node);
Expand Down
24 changes: 24 additions & 0 deletions test/unit/loops.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,3 +630,27 @@ test("do...while double-negation", () => {
expect(util.transpileString(code)).not.toMatch("not");
expect(util.transpileAndExecute(code)).toBe(2);
});

test("for...in with pre-defined variable", () => {
util.testFunction`
const obj = { x: "y", foo: "bar" };

let x = "";
let result = [];
for (x in obj) {
result.push(x);
}
return result;

@ark120202 ark120202 Aug 31, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since objects are compared deeply these 2 tests may be merged to return { x, result }

`.expectToMatchJsResult();
});

test("for...in with pre-defined variable keeps last value", () => {
util.testFunction`
const obj = { x: "y", foo: "bar" };

let x = "";
for (x in obj) {
}
return x;
`.expectToMatchJsResult();
});