Add in missing 4.0 compound assignments#919
Conversation
| context.diagnostics.push(unsupportedNodeKind(node, operator)); | ||
| return []; |
There was a problem hiding this comment.
Since operator is a union, and all it's values are handled above, you can use assertNever to statically verify that this branch would never be executed
| context.diagnostics.push(unsupportedNodeKind(node, operator)); | |
| return []; | |
| assertNever(operator); |
| } else if (operator === ts.SyntaxKind.QuestionQuestionToken) { | ||
| return [ | ||
| lua.createIfStatement( | ||
| lua.createBinaryExpression(lhs, lua.createNilLiteral(), lua.SyntaxKind.EqualityOperator), |
There was a problem hiding this comment.
All these 3 branches appear to be different only in the first parameter here. Thoughts on just extracting it into a condition variable?
| { operator: "||=", initialValue: false }, | ||
| { operator: "&&=", initialValue: true }, | ||
| { operator: "??=", initialValue: undefined }, | ||
| ])("compound assignment side effects", ({ operator, initialValue }) => { |
There was a problem hiding this comment.
Shouldn't it be the case with other compound assignment operators too?
test/unit/assignments.spec.ts
Outdated
| export let setterCalled = 0; | ||
|
|
||
| class MyClass { | ||
|
|
| if (operator === ts.SyntaxKind.QuestionQuestionToken) { | ||
| assert(ts.isBinaryExpression(node)); | ||
| return transformNullishCoalescingExpression(context, node); | ||
| } |
There was a problem hiding this comment.
Do we need this branch? If it's required, we can remove case ts.SyntaxKind.QuestionQuestionToken from transformBinaryExpression
There was a problem hiding this comment.
Yep you're right, it is needed here, I removed the case from transformBinaryExpression.
| * 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 = |
There was a problem hiding this comment.
After checking out the proposal it seems to be intrinsic behavior for logical assignment concept, so I'd call it LogicalAssignmentOperator
There was a problem hiding this comment.
Actually TS already has a type LogicalOperator = AmpersandAmpersandToken | BarBarToken, but it does not include ??, I propose we keep this as it is to avoid confusing naming, I did update the type to use ts.LogicalOperator instead of the individual tokens.
2 participants
Closes #916