Skip to content

Commit

Permalink
Refactor logical assignment
Browse files Browse the repository at this point in the history
  • Loading branch information
Kingwl committed May 16, 2020
1 parent cb2e71c commit 81ce254
Show file tree
Hide file tree
Showing 13 changed files with 47 additions and 117 deletions.
78 changes: 23 additions & 55 deletions src/compiler/binder.ts
Expand Up @@ -1047,24 +1047,9 @@ namespace ts {
}
}

function isLogicalAssignmentExpressioin(node: Node) {
while (true) {
if (isParenthesizedExpression(node)) {
node = node.expression;
}
else {
return isBinaryExpression(node) && isLogicalAssignmentOperator(node.operatorToken.kind);
}
}
}

function isTopLevelLogicalAssignmentExpression(node: Node): boolean {
while (isParenthesizedExpression(node.parent)) {
node = node.parent;
}
return !isStatementCondition(node) &&
!isLogicalAssignmentExpressioin(node.parent) &&
!(isOptionalChain(node.parent) && node.parent.expression === node);
function isLogicalAssignmentExpression(node: Node) {
node = skipParentheses(node);
return isBinaryExpression(node) && isLogicalOrCoalescingAssignmentOperator(node.operatorToken.kind);
}

function isTopLevelLogicalExpression(node: Node): boolean {
Expand All @@ -1073,6 +1058,7 @@ namespace ts {
node = node.parent;
}
return !isStatementCondition(node) &&
!isLogicalAssignmentExpression(node.parent) &&
!isLogicalExpression(node.parent) &&
!(isOptionalChain(node.parent) && node.parent.expression === node);
}
Expand All @@ -1089,7 +1075,7 @@ namespace ts {

function bindCondition(node: Expression | undefined, trueTarget: FlowLabel, falseTarget: FlowLabel) {
doWithConditionalBranches(bind, node, trueTarget, falseTarget);
if (!node || !isLogicalAssignmentExpressioin(node) && !isLogicalExpression(node) && !(isOptionalChain(node) && isOutermostOptionalChain(node))) {
if (!node || !isLogicalAssignmentExpression(node) && !isLogicalExpression(node) && !(isOptionalChain(node) && isOutermostOptionalChain(node))) {
addAntecedent(trueTarget, createFlowCondition(FlowFlags.TrueCondition, currentFlow, node));
addAntecedent(falseTarget, createFlowCondition(FlowFlags.FalseCondition, currentFlow, node));
}
Expand Down Expand Up @@ -1189,24 +1175,6 @@ namespace ts {
currentFlow = finishFlowLabel(postIfLabel);
}

function bindLogicalAssignmentExpression(node: BinaryExpression, trueTarget: FlowLabel, falseTarget: FlowLabel) {
const preRightLabel = createBranchLabel();
if (node.operatorToken.kind === SyntaxKind.AmpersandAmpersandEqualsToken) {
bindCondition(node.left, preRightLabel, falseTarget);
}
else {
bindCondition(node.left, trueTarget, preRightLabel);
}
currentFlow = finishFlowLabel(preRightLabel);
bind(node.operatorToken);

doWithConditionalBranches(bind, node.right, trueTarget, falseTarget);
bindAssignmentTargetFlow(node.left);

addAntecedent(trueTarget, createFlowCondition(FlowFlags.TrueCondition, currentFlow, node));
addAntecedent(falseTarget, createFlowCondition(FlowFlags.FalseCondition, currentFlow, node));
}

function bindReturnOrThrow(node: ReturnStatement | ThrowStatement): void {
bind(node.expression);
if (node.kind === SyntaxKind.ReturnStatement) {
Expand Down Expand Up @@ -1448,17 +1416,27 @@ namespace ts {
}
}

function bindLogicalExpression(node: BinaryExpression, trueTarget: FlowLabel, falseTarget: FlowLabel) {
function bindLogicalLikeExpression(node: BinaryExpression, trueTarget: FlowLabel, falseTarget: FlowLabel) {
const preRightLabel = createBranchLabel();
if (node.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken) {
if (node.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken || node.operatorToken.kind === SyntaxKind.AmpersandAmpersandEqualsToken) {
bindCondition(node.left, preRightLabel, falseTarget);
}
else {
bindCondition(node.left, trueTarget, preRightLabel);
}
currentFlow = finishFlowLabel(preRightLabel);
bind(node.operatorToken);
bindCondition(node.right, trueTarget, falseTarget);

if (isLogicalOrCoalescingAssignmentOperator(node.operatorToken.kind)) {
doWithConditionalBranches(bind, node.right, trueTarget, falseTarget);
bindAssignmentTargetFlow(node.left);

addAntecedent(trueTarget, createFlowCondition(FlowFlags.TrueCondition, currentFlow, node));
addAntecedent(falseTarget, createFlowCondition(FlowFlags.FalseCondition, currentFlow, node));
}
else {
bindCondition(node.right, trueTarget, falseTarget);
}
}

function bindPrefixUnaryExpressionFlow(node: PrefixUnaryExpression) {
Expand Down Expand Up @@ -1544,25 +1522,15 @@ namespace ts {
// TODO: bindLogicalExpression is recursive - if we want to handle deeply nested `&&` expressions
// we'll need to handle the `bindLogicalExpression` scenarios in this state machine, too
// For now, though, since the common cases are chained `+`, leaving it recursive is fine
if (operator === SyntaxKind.AmpersandAmpersandToken || operator === SyntaxKind.BarBarToken || operator === SyntaxKind.QuestionQuestionToken) {
if (operator === SyntaxKind.AmpersandAmpersandToken || operator === SyntaxKind.BarBarToken || operator === SyntaxKind.QuestionQuestionToken ||
isLogicalOrCoalescingAssignmentOperator(operator)) {
if (isTopLevelLogicalExpression(node)) {
const postExpressionLabel = createBranchLabel();
bindLogicalExpression(node, postExpressionLabel, postExpressionLabel);
currentFlow = finishFlowLabel(postExpressionLabel);
}
else {
bindLogicalExpression(node, currentTrueTarget!, currentFalseTarget!);
}
completeNode();
}
else if(isLogicalAssignmentOperator(operator)) {
if (isTopLevelLogicalAssignmentExpression(node)) {
const postExpressionLabel = createBranchLabel();
bindLogicalAssignmentExpression(node, postExpressionLabel, postExpressionLabel);
bindLogicalLikeExpression(node, postExpressionLabel, postExpressionLabel);
currentFlow = finishFlowLabel(postExpressionLabel);
}
else {
bindLogicalAssignmentExpression(node, currentTrueTarget!, currentFalseTarget!);
bindLogicalLikeExpression(node, currentTrueTarget!, currentFalseTarget!);
}
completeNode();
}
Expand Down Expand Up @@ -3656,7 +3624,7 @@ namespace ts {
if (operatorTokenKind === SyntaxKind.QuestionQuestionToken) {
transformFlags |= TransformFlags.AssertES2020;
}
else if (isLogicalAssignmentOperator(operatorTokenKind)) {
else if (isLogicalOrCoalescingAssignmentOperator(operatorTokenKind)) {
transformFlags |= TransformFlags.AssertESNext;
}
else if (operatorTokenKind === SyntaxKind.EqualsToken && leftKind === SyntaxKind.ObjectLiteralExpression) {
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/checker.ts
Expand Up @@ -28596,7 +28596,7 @@ namespace ts {
getUnionType([removeDefinitelyFalsyTypes(leftType), rightType], UnionReduction.Subtype) :
leftType;
if (operator === SyntaxKind.BarBarEqualsToken) {
checkAssignmentOperator(resultType);
checkAssignmentOperator(rightType);
}
return resultType;
}
Expand All @@ -28606,7 +28606,7 @@ namespace ts {
getUnionType([getNonNullableType(leftType), rightType], UnionReduction.Subtype) :
leftType;
if (operator === SyntaxKind.QuestionQuestionEqualsToken) {
checkAssignmentOperator(resultType);
checkAssignmentOperator(rightType);
}
return resultType;
}
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/transformers/esnext.ts
Expand Up @@ -18,7 +18,7 @@ namespace ts {
switch (node.kind) {
case SyntaxKind.BinaryExpression:
const binaryExpression = <BinaryExpression>node;
if (isLogicalAssignmentOperator(binaryExpression.operatorToken.kind)) {
if (isLogicalOrCoalescingAssignmentOperator(binaryExpression.operatorToken.kind)) {
return transformLogicalAssignmentOperators(binaryExpression);
}
// falls through
Expand All @@ -29,7 +29,7 @@ namespace ts {

function transformLogicalAssignmentOperators(binaryExpression: BinaryExpression): VisitResult<Node> {
const operator = binaryExpression.operatorToken;
if (isCompoundAssignment(operator.kind) && isLogicalAssignmentOperator(operator.kind)) {
if (isCompoundAssignment(operator.kind) && isLogicalOrCoalescingAssignmentOperator(operator.kind)) {
const nonAssignmentOperator = getNonAssignmentOperatorForCompoundAssignment(operator.kind);
const left = visitNode(binaryExpression.left, visitor, isExpression);
const right = visitNode(binaryExpression.right, visitor, isExpression);
Expand Down
8 changes: 2 additions & 6 deletions src/compiler/utilities.ts
Expand Up @@ -2594,7 +2594,7 @@ namespace ts {
switch (parent.kind) {
case SyntaxKind.BinaryExpression:
const binaryOperator = (<BinaryExpression>parent).operatorToken.kind;
return isAssignmentOperator(binaryOperator) && !isLogicalAssignmentOperator(binaryOperator) && (<BinaryExpression>parent).left === node ?
return isAssignmentOperator(binaryOperator) && !isLogicalOrCoalescingAssignmentOperator(binaryOperator) && (<BinaryExpression>parent).left === node ?
binaryOperator === SyntaxKind.EqualsToken ? AssignmentKind.Definite : AssignmentKind.Compound :
AssignmentKind.None;
case SyntaxKind.PrefixUnaryExpression:
Expand Down Expand Up @@ -3378,10 +3378,6 @@ namespace ts {
return 14;
case SyntaxKind.AsteriskAsteriskToken:
return 15;
case SyntaxKind.BarBarEqualsToken:
case SyntaxKind.AmpersandAmpersandEqualsToken:
case SyntaxKind.QuestionQuestionEqualsToken:
return 16;
}

// -1 is lower than all other precedences. Returning it will cause binary expression
Expand Down Expand Up @@ -4454,7 +4450,7 @@ namespace ts {
|| token === SyntaxKind.ExclamationToken;
}

export function isLogicalAssignmentOperator(token: SyntaxKind): boolean {
export function isLogicalOrCoalescingAssignmentOperator(token: SyntaxKind): boolean {
return token === SyntaxKind.BarBarEqualsToken
|| token === SyntaxKind.AmpersandAmpersandEqualsToken
|| token === SyntaxKind.QuestionQuestionEqualsToken;
Expand Down
6 changes: 0 additions & 6 deletions src/services/codefixes/inferFromUsage.ts
Expand Up @@ -785,12 +785,6 @@ namespace ts.codefix {
}
break;

case SyntaxKind.BarBarEqualsToken:
case SyntaxKind.QuestionQuestionEqualsToken:
case SyntaxKind.AmpersandAmpersandEqualsToken:
// TODO: infer here
break;

case SyntaxKind.AmpersandAmpersandToken:
case SyntaxKind.CommaToken:
case SyntaxKind.InstanceOfKeyword:
Expand Down
@@ -1,29 +1,20 @@
tests/cases/conformance/esnext/logicalAssignment/logicalAssignment7.ts(2,6): error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
tests/cases/conformance/esnext/logicalAssignment/logicalAssignment7.ts(6,6): error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
tests/cases/conformance/esnext/logicalAssignment/logicalAssignment7.ts(10,5): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/esnext/logicalAssignment/logicalAssignment7.ts(10,6): error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
tests/cases/conformance/esnext/logicalAssignment/logicalAssignment7.ts(10,40): error TS2345: Argument of type '100' is not assignable to parameter of type 'never'.


==== tests/cases/conformance/esnext/logicalAssignment/logicalAssignment7.ts (5 errors) ====
==== tests/cases/conformance/esnext/logicalAssignment/logicalAssignment7.ts (2 errors) ====
function foo1(results: number[] | undefined, results1: number[] | undefined) {
(results ||= results1 ||= []).push(100);
~~~~~~~~~~~~~~~~~~~~
!!! error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
}

function foo2(results: number[] | undefined, results1: number[] | undefined) {
(results ??= results1 ??= []).push(100);
~~~~~~~~~~~~~~~~~~~~
!!! error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
}

function foo3(results: number[] | undefined, results1: number[] | undefined) {
(results &&= results1 &&= []).push(100);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2532: Object is possibly 'undefined'.
~~~~~~~~~~~~~~~~~~~~
!!! error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
~~~
!!! error TS2345: Argument of type '100' is not assignable to parameter of type 'never'.
}
Expand Up @@ -14,12 +14,11 @@ function foo3(results: number[] | undefined, results1: number[] | undefined) {
//// [logicalAssignment7.js]
"use strict";
function foo1(results, results1) {
(results || (results = results1) || (results || (results = results1) = [])).push(100);
(results || (results = results1 || (results1 = []))).push(100);
}
function foo2(results, results1) {
var _a;
((_a = results !== null && results !== void 0 ? results : (results = results1)) !== null && _a !== void 0 ? _a : (results !== null && results !== void 0 ? results : (results = results1) = [])).push(100);
(results !== null && results !== void 0 ? results : (results = results1 !== null && results1 !== void 0 ? results1 : (results1 = []))).push(100);
}
function foo3(results, results1) {
(results && (results = results1) && (results && (results = results1) = [])).push(100);
(results && (results = results1 && (results1 = []))).push(100);
}
Expand Up @@ -9,8 +9,8 @@ function foo1(results: number[] | undefined, results1: number[] | undefined) {
>(results ||= results1 ||= []).push : (...items: number[]) => number
>(results ||= results1 ||= []) : number[]
>results ||= results1 ||= [] : number[]
>results ||= results1 : number[] | undefined
>results : number[] | undefined
>results1 ||= [] : number[]
>results1 : number[] | undefined
>[] : never[]
>push : (...items: number[]) => number
Expand All @@ -27,8 +27,8 @@ function foo2(results: number[] | undefined, results1: number[] | undefined) {
>(results ??= results1 ??= []).push : (...items: number[]) => number
>(results ??= results1 ??= []) : number[]
>results ??= results1 ??= [] : number[]
>results ??= results1 : number[] | undefined
>results : number[] | undefined
>results1 ??= [] : number[]
>results1 : number[] | undefined
>[] : never[]
>push : (...items: number[]) => number
Expand All @@ -45,8 +45,8 @@ function foo3(results: number[] | undefined, results1: number[] | undefined) {
>(results &&= results1 &&= []).push : (...items: never[]) => number
>(results &&= results1 &&= []) : never[] | undefined
>results &&= results1 &&= [] : never[] | undefined
>results &&= results1 : number[] | undefined
>results : number[] | undefined
>results1 &&= [] : never[] | undefined
>results1 : number[] | undefined
>[] : never[]
>push : (...items: never[]) => number
Expand Down
@@ -1,29 +1,20 @@
tests/cases/conformance/esnext/logicalAssignment/logicalAssignment7.ts(2,6): error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
tests/cases/conformance/esnext/logicalAssignment/logicalAssignment7.ts(6,6): error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
tests/cases/conformance/esnext/logicalAssignment/logicalAssignment7.ts(10,5): error TS2532: Object is possibly 'undefined'.
tests/cases/conformance/esnext/logicalAssignment/logicalAssignment7.ts(10,6): error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
tests/cases/conformance/esnext/logicalAssignment/logicalAssignment7.ts(10,40): error TS2345: Argument of type '100' is not assignable to parameter of type 'never'.


==== tests/cases/conformance/esnext/logicalAssignment/logicalAssignment7.ts (5 errors) ====
==== tests/cases/conformance/esnext/logicalAssignment/logicalAssignment7.ts (2 errors) ====
function foo1(results: number[] | undefined, results1: number[] | undefined) {
(results ||= results1 ||= []).push(100);
~~~~~~~~~~~~~~~~~~~~
!!! error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
}

function foo2(results: number[] | undefined, results1: number[] | undefined) {
(results ??= results1 ??= []).push(100);
~~~~~~~~~~~~~~~~~~~~
!!! error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
}

function foo3(results: number[] | undefined, results1: number[] | undefined) {
(results &&= results1 &&= []).push(100);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2532: Object is possibly 'undefined'.
~~~~~~~~~~~~~~~~~~~~
!!! error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
~~~
!!! error TS2345: Argument of type '100' is not assignable to parameter of type 'never'.
}
Expand Up @@ -14,11 +14,11 @@ function foo3(results: number[] | undefined, results1: number[] | undefined) {
//// [logicalAssignment7.js]
"use strict";
function foo1(results, results1) {
(results || (results = results1) || (results || (results = results1) = [])).push(100);
(results || (results = results1 || (results1 = []))).push(100);
}
function foo2(results, results1) {
(results ?? (results = results1) ?? (results ?? (results = results1) = [])).push(100);
(results ?? (results = results1 ?? (results1 = []))).push(100);
}
function foo3(results, results1) {
(results && (results = results1) && (results && (results = results1) = [])).push(100);
(results && (results = results1 && (results1 = []))).push(100);
}

0 comments on commit 81ce254

Please sign in to comment.