Skip to content

Commit 849f630

Browse files
PerryvwPerryvw
authored
Add in missing 4.0 compound assignments (#919)
* Add in missing 4.0 compound assignments * Compound assignments 4.0 take 2 * Addressed PR comments * Removed obsolete exception case in transformBinaryExpression
1 parent 1093e8f commit 849f630

File tree

3 files changed

+223
-22
lines changed

3 files changed

+223
-22
lines changed

‎src/transformation/visitors/binary-expression/compound.ts‎

Lines changed: 81 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as ts from "typescript";
22
import * as lua from "../../../LuaAST";
3-
import { cast } from "../../../utils";
3+
import { cast, assertNever } from "../../../utils";
44
import { TransformationContext } from "../../context";
55
import { createImmediatelyInvokedFunctionExpression } from "../../utils/lua-ast";
66
import { isArrayType, isExpressionWithEvaluationEffect } from "../../utils/typescript";
@@ -46,9 +46,12 @@ type CompoundAssignmentToken =
4646
| ts.SyntaxKind.AsteriskAsteriskToken
4747
| ts.SyntaxKind.LessThanLessThanToken
4848
| ts.SyntaxKind.GreaterThanGreaterThanToken
49-
| ts.SyntaxKind.GreaterThanGreaterThanGreaterThanToken;
49+
| ts.SyntaxKind.GreaterThanGreaterThanGreaterThanToken
50+
| ts.SyntaxKind.BarBarToken
51+
| ts.SyntaxKind.AmpersandAmpersandToken
52+
| ts.SyntaxKind.QuestionQuestionToken;
5053

51-
const compoundToAssignmentTokens: Partial<Record<ts.CompoundAssignmentOperator, CompoundAssignmentToken>> = {
54+
const compoundToAssignmentTokens: Record<ts.CompoundAssignmentOperator, CompoundAssignmentToken> = {
5255
[ts.SyntaxKind.BarEqualsToken]: ts.SyntaxKind.BarToken,
5356
[ts.SyntaxKind.PlusEqualsToken]: ts.SyntaxKind.PlusToken,
5457
[ts.SyntaxKind.CaretEqualsToken]: ts.SyntaxKind.CaretToken,
@@ -61,14 +64,16 @@ const compoundToAssignmentTokens: Partial<Record<ts.CompoundAssignmentOperator,
6164
[ts.SyntaxKind.LessThanLessThanEqualsToken]: ts.SyntaxKind.LessThanLessThanToken,
6265
[ts.SyntaxKind.GreaterThanGreaterThanEqualsToken]: ts.SyntaxKind.GreaterThanGreaterThanToken,
6366
[ts.SyntaxKind.GreaterThanGreaterThanGreaterThanEqualsToken]: ts.SyntaxKind.GreaterThanGreaterThanGreaterThanToken,
67+
[ts.SyntaxKind.BarBarEqualsToken]: ts.SyntaxKind.BarBarToken,
68+
[ts.SyntaxKind.AmpersandAmpersandEqualsToken]: ts.SyntaxKind.AmpersandAmpersandToken,
69+
[ts.SyntaxKind.QuestionQuestionEqualsToken]: ts.SyntaxKind.QuestionQuestionToken,
6470
};
6571

6672
export const isCompoundAssignmentToken = (token: ts.BinaryOperator): token is ts.CompoundAssignmentOperator =>
6773
token in compoundToAssignmentTokens;
6874

69-
export const unwrapCompoundAssignmentToken = (
70-
token: ts.CompoundAssignmentOperator
71-
): CompoundAssignmentToken | undefined => compoundToAssignmentTokens[token];
75+
export const unwrapCompoundAssignmentToken = (token: ts.CompoundAssignmentOperator): CompoundAssignmentToken =>
76+
compoundToAssignmentTokens[token];
7277

7378
export function transformCompoundAssignmentExpression(
7479
context: TransformationContext,
@@ -139,6 +144,15 @@ export function transformCompoundAssignmentExpression(
139144
const operatorExpression = transformBinaryOperation(context, left, right, operator, expression);
140145
const tmpDeclaration = lua.createVariableDeclarationStatement(tmpIdentifier, operatorExpression);
141146
const assignStatements = transformAssignment(context, lhs, tmpIdentifier);
147+
148+
if (isSetterSkippingCompoundAssignmentOperator(operator)) {
149+
return createImmediatelyInvokedFunctionExpression(
150+
[tmpDeclaration, ...transformSetterSkippingCompoundAssignment(context, tmpIdentifier, operator, rhs)],
151+
tmpIdentifier,
152+
expression
153+
);
154+
}
155+
142156
return createImmediatelyInvokedFunctionExpression(
143157
[tmpDeclaration, ...assignStatements],
144158
tmpIdentifier,
@@ -175,13 +189,74 @@ export function transformCompoundAssignmentStatement(
175189
[context.transformExpression(objExpression), context.transformExpression(indexExpression)]
176190
);
177191
const accessExpression = lua.createTableIndexExpression(obj, index);
192+
193+
if (isSetterSkippingCompoundAssignmentOperator(operator)) {
194+
return [
195+
objAndIndexDeclaration,
196+
...transformSetterSkippingCompoundAssignment(context, accessExpression, operator, rhs, node),
197+
];
198+
}
199+
178200
const operatorExpression = transformBinaryOperation(context, accessExpression, right, operator, node);
179201
const assignStatement = lua.createAssignmentStatement(accessExpression, operatorExpression);
180202
return [objAndIndexDeclaration, assignStatement];
181203
} else {
204+
if (isSetterSkippingCompoundAssignmentOperator(operator)) {
205+
const luaLhs = context.transformExpression(lhs) as lua.AssignmentLeftHandSideExpression;
206+
return transformSetterSkippingCompoundAssignment(context, luaLhs, operator, rhs, node);
207+
}
208+
182209
// Simple statements
183210
// ${left} = ${left} ${replacementOperator} ${right}
184211
const operatorExpression = transformBinaryOperation(context, left, right, operator, node);
185212
return transformAssignment(context, lhs, operatorExpression);
186213
}
187214
}
215+
216+
/* These setter-skipping operators will not execute the setter if result does not change.
217+
* x.y ||= z does NOT call the x.y setter if x.y is already true.
218+
* x.y &&= z does NOT call the x.y setter if x.y is already false.
219+
* x.y ??= z does NOT call the x.y setter if x.y is already not nullish.
220+
*/
221+
type SetterSkippingCompoundAssignmentOperator = ts.LogicalOperator | ts.SyntaxKind.QuestionQuestionToken;
222+
223+
function isSetterSkippingCompoundAssignmentOperator(
224+
operator: ts.BinaryOperator
225+
): operator is SetterSkippingCompoundAssignmentOperator {
226+
return (
227+
operator === ts.SyntaxKind.AmpersandAmpersandToken ||
228+
operator === ts.SyntaxKind.BarBarToken ||
229+
operator === ts.SyntaxKind.QuestionQuestionToken
230+
);
231+
}
232+
233+
function transformSetterSkippingCompoundAssignment(
234+
context: TransformationContext,
235+
lhs: lua.AssignmentLeftHandSideExpression,
236+
operator: SetterSkippingCompoundAssignmentOperator,
237+
rhs: ts.Expression,
238+
node?: ts.Node
239+
): lua.Statement[] {
240+
// These assignments have the form 'if x then y = z', figure out what condition x is first.
241+
let condition: lua.Expression;
242+
243+
if (operator === ts.SyntaxKind.AmpersandAmpersandToken) {
244+
condition = lhs;
245+
} else if (operator === ts.SyntaxKind.BarBarToken) {
246+
condition = lua.createUnaryExpression(lhs, lua.SyntaxKind.NotOperator);
247+
} else if (operator === ts.SyntaxKind.QuestionQuestionToken) {
248+
condition = lua.createBinaryExpression(lhs, lua.createNilLiteral(), lua.SyntaxKind.EqualityOperator);
249+
} else {
250+
assertNever(operator);
251+
}
252+
253+
// if condition then lhs = rhs end
254+
return [
255+
lua.createIfStatement(
256+
condition,
257+
lua.createBlock([lua.createAssignmentStatement(lhs, context.transformExpression(rhs))]),
258+
undefined,
259+
node
260+
),
261+
];
262+
}

‎src/transformation/visitors/binary-expression/index.ts‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import * as ts from "typescript";
22
import * as lua from "../../../LuaAST";
33
import { FunctionVisitor, TransformationContext } from "../../context";
44
import { AnnotationKind, getTypeAnnotations } from "../../utils/annotations";
5-
import { extensionInvalidInstanceOf, luaTableInvalidInstanceOf, unsupportedNodeKind } from "../../utils/diagnostics";
5+
import { extensionInvalidInstanceOf, luaTableInvalidInstanceOf } from "../../utils/diagnostics";
66
import { createImmediatelyInvokedFunctionExpression, wrapInToStringForConcat } from "../../utils/lua-ast";
77
import { LuaLibFeature, transformLuaLibFunction } from "../../utils/lualib";
88
import { isStandardLibraryType, isStringType, typeCanSatisfy } from "../../utils/typescript";
@@ -15,6 +15,7 @@ import {
1515
transformCompoundAssignmentStatement,
1616
unwrapCompoundAssignmentToken,
1717
} from "./compound";
18+
import { assert } from "../../../utils";
1819

1920
type SimpleOperator =
2021
| ts.AdditiveOperatorOrHigher
@@ -45,13 +46,18 @@ export function transformBinaryOperation(
4546
context: TransformationContext,
4647
left: lua.Expression,
4748
right: lua.Expression,
48-
operator: BitOperator | SimpleOperator,
49+
operator: BitOperator | SimpleOperator | ts.SyntaxKind.QuestionQuestionToken,
4950
node: ts.Node
5051
): lua.Expression {
5152
if (isBitOperator(operator)) {
5253
return transformBinaryBitOperation(context, node, left, right, operator);
5354
}
5455

56+
if (operator === ts.SyntaxKind.QuestionQuestionToken) {
57+
assert(ts.isBinaryExpression(node));
58+
return transformNullishCoalescingExpression(context, node);
59+
}
60+
5561
let luaOperator = simpleOperatorsToLua[operator];
5662

5763
// Check if we need to use string concat operator
@@ -78,11 +84,6 @@ export const transformBinaryExpression: FunctionVisitor<ts.BinaryExpression> = (
7884

7985
if (isCompoundAssignmentToken(operator)) {
8086
const token = unwrapCompoundAssignmentToken(operator);
81-
if (!token) {
82-
context.diagnostics.push(unsupportedNodeKind(node, operator));
83-
return lua.createNilLiteral(node);
84-
}
85-
8687
return transformCompoundAssignmentExpression(context, node, node.left, node.right, token, false);
8788
}
8889

@@ -131,10 +132,6 @@ export const transformBinaryExpression: FunctionVisitor<ts.BinaryExpression> = (
131132
);
132133
}
133134

134-
case ts.SyntaxKind.QuestionQuestionToken: {
135-
return transformNullishCoalescingExpression(context, node);
136-
}
137-
138135
default:
139136
return transformBinaryOperation(
140137
context,
@@ -157,11 +154,6 @@ export function transformBinaryExpressionStatement(
157154
if (isCompoundAssignmentToken(operator)) {
158155
// +=, -=, etc...
159156
const token = unwrapCompoundAssignmentToken(operator);
160-
if (!token) {
161-
context.diagnostics.push(unsupportedNodeKind(node, operator));
162-
return;
163-
}
164-
165157
return transformCompoundAssignmentStatement(context, expression, expression.left, expression.right, token);
166158
} else if (operator === ts.SyntaxKind.EqualsToken) {
167159
return transformAssignmentStatement(context, expression as ts.AssignmentExpression<ts.EqualsToken>);

‎test/unit/assignments.spec.ts‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,9 @@ test.each([
121121
"x ^= y",
122122
"x <<= y",
123123
"x >>>= y",
124+
"x &&= y",
125+
"x ||= y",
126+
"x ??= y",
124127
])("Operator assignment statements (%p)", statement => {
125128
util.testFunction`
126129
let x = 3;
@@ -359,3 +362,134 @@ test("local multiple variable declaration referencing self indirectly", () => {
359362
return bar;
360363
`.expectToMatchJsResult();
361364
});
365+
366+
describe.each(["x &&= y", "x ||= y"])("boolean compound assignment (%p)", assignment => {
367+
const booleanCases = [
368+
[false, false],
369+
[false, true],
370+
[true, false],
371+
[true, true],
372+
];
373+
test.each(booleanCases)("matches JS", (x, y) => {
374+
util.testFunction`
375+
let x = ${x};
376+
let y = ${y};
377+
${assignment};
378+
return x;
379+
`.expectToMatchJsResult();
380+
});
381+
});
382+
383+
test.each([undefined, 3])("nullish coalescing compound assignment", initialValue => {
384+
util.testFunction`
385+
let x: number = ${util.formatCode(initialValue)};
386+
x ??= 5;
387+
return x;
388+
`.expectToMatchJsResult();
389+
});
390+
391+
test("nullish coalescing compound assignment lhs false", () => {
392+
util.testFunction`
393+
let x = false;
394+
x ??= true;
395+
return x;
396+
`.expectToMatchJsResult();
397+
});
398+
399+
test("nullish coalescing compound assignment side effect not evaluated", () => {
400+
util.testFunction`
401+
let x = 3;
402+
let y = 10;
403+
x ??= (y += 5);
404+
return [x, y];
405+
`.expectToMatchJsResult();
406+
});
407+
408+
test.each([
409+
{ operator: "||=", initialValue: true },
410+
{ operator: "&&=", initialValue: false },
411+
{ operator: "??=", initialValue: false },
412+
])("compound assignment short-circuits and does not call setter", ({ operator, initialValue }) => {
413+
/*
414+
In JS if the rhs does not affect the resulting value, the setter is NOT called:
415+
* x.y ||= z is translated to x.y || (x.y = z).
416+
* x.y &&= z is translated to x.y && (x.y = z).
417+
* x.y ||= z is translated to x.y !== undefined && (x.y = z).
418+
419+
Test if setter in Lua is called same nr of times as in JS.
420+
*/
421+
util.testModule`
422+
export let setterCalled = 0;
423+
424+
class MyClass {
425+
426+
get prop(): any {
427+
return ${initialValue};
428+
}
429+
430+
set prop(value: any) {
431+
setterCalled++;
432+
}
433+
}
434+
435+
const inst = new MyClass();
436+
inst.prop ${operator} 8;
437+
`.expectToMatchJsResult();
438+
});
439+
440+
test.each([
441+
{ operator: "||=", initialValue: true },
442+
{ operator: "&&=", initialValue: false },
443+
{ operator: "??=", initialValue: false },
444+
])("compound assignment short-circuits and does not call setter as expression", ({ operator, initialValue }) => {
445+
/*
446+
In JS if the rhs does not affect the resulting value, the setter is NOT called:
447+
* x.y ||= z is translated to x.y || (x.y = z).
448+
* x.y &&= z is translated to x.y && (x.y = z).
449+
* x.y ||= z is translated to x.y !== undefined && (x.y = z).
450+
451+
Test if setter in Lua is called same nr of times as in JS.
452+
*/
453+
util.testModule`
454+
export let setterCalled = 0;
455+
456+
class MyClass {
457+
458+
get prop(): any {
459+
return ${initialValue};
460+
}
461+
462+
set prop(value: any) {
463+
setterCalled++;
464+
}
465+
}
466+
467+
const inst = new MyClass();
468+
export const result = (inst.prop ${operator} 8);
469+
`.expectToMatchJsResult();
470+
});
471+
472+
test.each([
473+
{ operator: "+=", initialValue: 3 },
474+
{ operator: "-=", initialValue: 10 },
475+
{ operator: "*=", initialValue: 4 },
476+
{ operator: "/=", initialValue: 20 },
477+
{ operator: "||=", initialValue: false },
478+
{ operator: "&&=", initialValue: true },
479+
{ operator: "??=", initialValue: undefined },
480+
])("compound assignment side effects", ({ operator, initialValue }) => {
481+
// Test if when assigning to something with potential side effects, they are only evaluated once.
482+
util.testFunction`
483+
const obj: { prop: any} = { prop: ${initialValue} };
484+
485+
let objGot = 0;
486+
function getObj() {
487+
objGot++;
488+
return obj;
489+
}
490+
491+
getObj().prop ${operator} 4;
492+
493+
return [obj, objGot];
494+
`.expectToMatchJsResult();
495+
});

0 commit comments

Comments
 (0)