diff --git a/src/transformation/visitors/binary-expression/compound.ts b/src/transformation/visitors/binary-expression/compound.ts index c82fe6862..3b92ade58 100644 --- a/src/transformation/visitors/binary-expression/compound.ts +++ b/src/transformation/visitors/binary-expression/compound.ts @@ -1,6 +1,6 @@ import * as ts from "typescript"; import * as lua from "../../../LuaAST"; -import { cast } from "../../../utils"; +import { cast, assertNever } from "../../../utils"; import { TransformationContext } from "../../context"; import { createImmediatelyInvokedFunctionExpression } from "../../utils/lua-ast"; import { isArrayType, isExpressionWithEvaluationEffect } from "../../utils/typescript"; @@ -46,9 +46,12 @@ type CompoundAssignmentToken = | ts.SyntaxKind.AsteriskAsteriskToken | ts.SyntaxKind.LessThanLessThanToken | ts.SyntaxKind.GreaterThanGreaterThanToken - | ts.SyntaxKind.GreaterThanGreaterThanGreaterThanToken; + | ts.SyntaxKind.GreaterThanGreaterThanGreaterThanToken + | ts.SyntaxKind.BarBarToken + | ts.SyntaxKind.AmpersandAmpersandToken + | ts.SyntaxKind.QuestionQuestionToken; -const compoundToAssignmentTokens: Partial> = { +const compoundToAssignmentTokens: Record = { [ts.SyntaxKind.BarEqualsToken]: ts.SyntaxKind.BarToken, [ts.SyntaxKind.PlusEqualsToken]: ts.SyntaxKind.PlusToken, [ts.SyntaxKind.CaretEqualsToken]: ts.SyntaxKind.CaretToken, @@ -61,14 +64,16 @@ const compoundToAssignmentTokens: Partial token in compoundToAssignmentTokens; -export const unwrapCompoundAssignmentToken = ( - token: ts.CompoundAssignmentOperator -): CompoundAssignmentToken | undefined => compoundToAssignmentTokens[token]; +export const unwrapCompoundAssignmentToken = (token: ts.CompoundAssignmentOperator): CompoundAssignmentToken => + compoundToAssignmentTokens[token]; export function transformCompoundAssignmentExpression( context: TransformationContext, @@ -139,6 +144,15 @@ export function transformCompoundAssignmentExpression( const operatorExpression = transformBinaryOperation(context, left, right, operator, expression); const tmpDeclaration = lua.createVariableDeclarationStatement(tmpIdentifier, operatorExpression); const assignStatements = transformAssignment(context, lhs, tmpIdentifier); + + if (isSetterSkippingCompoundAssignmentOperator(operator)) { + return createImmediatelyInvokedFunctionExpression( + [tmpDeclaration, ...transformSetterSkippingCompoundAssignment(context, tmpIdentifier, operator, rhs)], + tmpIdentifier, + expression + ); + } + return createImmediatelyInvokedFunctionExpression( [tmpDeclaration, ...assignStatements], tmpIdentifier, @@ -175,13 +189,74 @@ export function transformCompoundAssignmentStatement( [context.transformExpression(objExpression), context.transformExpression(indexExpression)] ); const accessExpression = lua.createTableIndexExpression(obj, index); + + if (isSetterSkippingCompoundAssignmentOperator(operator)) { + return [ + objAndIndexDeclaration, + ...transformSetterSkippingCompoundAssignment(context, accessExpression, operator, rhs, node), + ]; + } + const operatorExpression = transformBinaryOperation(context, accessExpression, right, operator, node); const assignStatement = lua.createAssignmentStatement(accessExpression, operatorExpression); return [objAndIndexDeclaration, assignStatement]; } else { + if (isSetterSkippingCompoundAssignmentOperator(operator)) { + const luaLhs = context.transformExpression(lhs) as lua.AssignmentLeftHandSideExpression; + return transformSetterSkippingCompoundAssignment(context, luaLhs, operator, rhs, node); + } + // Simple statements // ${left} = ${left} ${replacementOperator} ${right} const operatorExpression = transformBinaryOperation(context, left, right, operator, node); return transformAssignment(context, lhs, operatorExpression); } } + +/* These setter-skipping operators will not execute the setter if result does not change. + * x.y ||= z does NOT call the x.y setter if x.y is already true. + * x.y &&= z does NOT call the x.y setter if x.y is already false. + * x.y ??= z does NOT call the x.y setter if x.y is already not nullish. + */ +type SetterSkippingCompoundAssignmentOperator = ts.LogicalOperator | ts.SyntaxKind.QuestionQuestionToken; + +function isSetterSkippingCompoundAssignmentOperator( + operator: ts.BinaryOperator +): operator is SetterSkippingCompoundAssignmentOperator { + return ( + operator === ts.SyntaxKind.AmpersandAmpersandToken || + operator === ts.SyntaxKind.BarBarToken || + operator === ts.SyntaxKind.QuestionQuestionToken + ); +} + +function transformSetterSkippingCompoundAssignment( + context: TransformationContext, + lhs: lua.AssignmentLeftHandSideExpression, + operator: SetterSkippingCompoundAssignmentOperator, + rhs: ts.Expression, + node?: ts.Node +): lua.Statement[] { + // These assignments have the form 'if x then y = z', figure out what condition x is first. + let condition: lua.Expression; + + if (operator === ts.SyntaxKind.AmpersandAmpersandToken) { + condition = lhs; + } else if (operator === ts.SyntaxKind.BarBarToken) { + condition = lua.createUnaryExpression(lhs, lua.SyntaxKind.NotOperator); + } else if (operator === ts.SyntaxKind.QuestionQuestionToken) { + condition = lua.createBinaryExpression(lhs, lua.createNilLiteral(), lua.SyntaxKind.EqualityOperator); + } else { + assertNever(operator); + } + + // if condition then lhs = rhs end + return [ + lua.createIfStatement( + condition, + lua.createBlock([lua.createAssignmentStatement(lhs, context.transformExpression(rhs))]), + undefined, + node + ), + ]; +} diff --git a/src/transformation/visitors/binary-expression/index.ts b/src/transformation/visitors/binary-expression/index.ts index f150d848e..001591c01 100644 --- a/src/transformation/visitors/binary-expression/index.ts +++ b/src/transformation/visitors/binary-expression/index.ts @@ -2,7 +2,7 @@ import * as ts from "typescript"; import * as lua from "../../../LuaAST"; import { FunctionVisitor, TransformationContext } from "../../context"; import { AnnotationKind, getTypeAnnotations } from "../../utils/annotations"; -import { extensionInvalidInstanceOf, luaTableInvalidInstanceOf, unsupportedNodeKind } from "../../utils/diagnostics"; +import { extensionInvalidInstanceOf, luaTableInvalidInstanceOf } from "../../utils/diagnostics"; import { createImmediatelyInvokedFunctionExpression, wrapInToStringForConcat } from "../../utils/lua-ast"; import { LuaLibFeature, transformLuaLibFunction } from "../../utils/lualib"; import { isStandardLibraryType, isStringType, typeCanSatisfy } from "../../utils/typescript"; @@ -15,6 +15,7 @@ import { transformCompoundAssignmentStatement, unwrapCompoundAssignmentToken, } from "./compound"; +import { assert } from "../../../utils"; type SimpleOperator = | ts.AdditiveOperatorOrHigher @@ -45,13 +46,18 @@ export function transformBinaryOperation( context: TransformationContext, left: lua.Expression, right: lua.Expression, - operator: BitOperator | SimpleOperator, + operator: BitOperator | SimpleOperator | ts.SyntaxKind.QuestionQuestionToken, node: ts.Node ): lua.Expression { if (isBitOperator(operator)) { return transformBinaryBitOperation(context, node, left, right, operator); } + if (operator === ts.SyntaxKind.QuestionQuestionToken) { + assert(ts.isBinaryExpression(node)); + return transformNullishCoalescingExpression(context, node); + } + let luaOperator = simpleOperatorsToLua[operator]; // Check if we need to use string concat operator @@ -78,11 +84,6 @@ export const transformBinaryExpression: FunctionVisitor = ( if (isCompoundAssignmentToken(operator)) { const token = unwrapCompoundAssignmentToken(operator); - if (!token) { - context.diagnostics.push(unsupportedNodeKind(node, operator)); - return lua.createNilLiteral(node); - } - return transformCompoundAssignmentExpression(context, node, node.left, node.right, token, false); } @@ -131,10 +132,6 @@ export const transformBinaryExpression: FunctionVisitor = ( ); } - case ts.SyntaxKind.QuestionQuestionToken: { - return transformNullishCoalescingExpression(context, node); - } - default: return transformBinaryOperation( context, @@ -157,11 +154,6 @@ export function transformBinaryExpressionStatement( if (isCompoundAssignmentToken(operator)) { // +=, -=, etc... const token = unwrapCompoundAssignmentToken(operator); - if (!token) { - context.diagnostics.push(unsupportedNodeKind(node, operator)); - return; - } - return transformCompoundAssignmentStatement(context, expression, expression.left, expression.right, token); } else if (operator === ts.SyntaxKind.EqualsToken) { return transformAssignmentStatement(context, expression as ts.AssignmentExpression); diff --git a/test/unit/assignments.spec.ts b/test/unit/assignments.spec.ts index 7ae3e1bb0..d2d80f08b 100644 --- a/test/unit/assignments.spec.ts +++ b/test/unit/assignments.spec.ts @@ -121,6 +121,9 @@ test.each([ "x ^= y", "x <<= y", "x >>>= y", + "x &&= y", + "x ||= y", + "x ??= y", ])("Operator assignment statements (%p)", statement => { util.testFunction` let x = 3; @@ -359,3 +362,134 @@ test("local multiple variable declaration referencing self indirectly", () => { return bar; `.expectToMatchJsResult(); }); + +describe.each(["x &&= y", "x ||= y"])("boolean compound assignment (%p)", assignment => { + const booleanCases = [ + [false, false], + [false, true], + [true, false], + [true, true], + ]; + test.each(booleanCases)("matches JS", (x, y) => { + util.testFunction` + let x = ${x}; + let y = ${y}; + ${assignment}; + return x; + `.expectToMatchJsResult(); + }); +}); + +test.each([undefined, 3])("nullish coalescing compound assignment", initialValue => { + util.testFunction` + let x: number = ${util.formatCode(initialValue)}; + x ??= 5; + return x; + `.expectToMatchJsResult(); +}); + +test("nullish coalescing compound assignment lhs false", () => { + util.testFunction` + let x = false; + x ??= true; + return x; + `.expectToMatchJsResult(); +}); + +test("nullish coalescing compound assignment side effect not evaluated", () => { + util.testFunction` + let x = 3; + let y = 10; + x ??= (y += 5); + return [x, y]; + `.expectToMatchJsResult(); +}); + +test.each([ + { operator: "||=", initialValue: true }, + { operator: "&&=", initialValue: false }, + { operator: "??=", initialValue: false }, +])("compound assignment short-circuits and does not call setter", ({ operator, initialValue }) => { + /* + In JS if the rhs does not affect the resulting value, the setter is NOT called: + * x.y ||= z is translated to x.y || (x.y = z). + * x.y &&= z is translated to x.y && (x.y = z). + * x.y ||= z is translated to x.y !== undefined && (x.y = z). + + Test if setter in Lua is called same nr of times as in JS. + */ + util.testModule` + export let setterCalled = 0; + + class MyClass { + + get prop(): any { + return ${initialValue}; + } + + set prop(value: any) { + setterCalled++; + } + } + + const inst = new MyClass(); + inst.prop ${operator} 8; + `.expectToMatchJsResult(); +}); + +test.each([ + { operator: "||=", initialValue: true }, + { operator: "&&=", initialValue: false }, + { operator: "??=", initialValue: false }, +])("compound assignment short-circuits and does not call setter as expression", ({ operator, initialValue }) => { + /* + In JS if the rhs does not affect the resulting value, the setter is NOT called: + * x.y ||= z is translated to x.y || (x.y = z). + * x.y &&= z is translated to x.y && (x.y = z). + * x.y ||= z is translated to x.y !== undefined && (x.y = z). + + Test if setter in Lua is called same nr of times as in JS. + */ + util.testModule` + export let setterCalled = 0; + + class MyClass { + + get prop(): any { + return ${initialValue}; + } + + set prop(value: any) { + setterCalled++; + } + } + + const inst = new MyClass(); + export const result = (inst.prop ${operator} 8); + `.expectToMatchJsResult(); +}); + +test.each([ + { operator: "+=", initialValue: 3 }, + { operator: "-=", initialValue: 10 }, + { operator: "*=", initialValue: 4 }, + { operator: "/=", initialValue: 20 }, + { operator: "||=", initialValue: false }, + { operator: "&&=", initialValue: true }, + { operator: "??=", initialValue: undefined }, +])("compound assignment side effects", ({ operator, initialValue }) => { + // Test if when assigning to something with potential side effects, they are only evaluated once. + util.testFunction` + const obj: { prop: any} = { prop: ${initialValue} }; + + let objGot = 0; + function getObj() { + objGot++; + return obj; + } + + getObj().prop ${operator} 4; + + return [obj, objGot]; + `.expectToMatchJsResult(); +});