diff --git a/lib/rules/require-atomic-updates.js b/lib/rules/require-atomic-updates.js index 5d7d7151455..c6cf0d74774 100644 --- a/lib/rules/require-atomic-updates.js +++ b/lib/rules/require-atomic-updates.js @@ -119,6 +119,66 @@ module.exports = { }); } + const alreadyReportedAssignments = new WeakSet(); + + class AssignmentTrackerState { + constructor({ openAssignmentsWithoutReads = new Set(), openAssignmentsWithReads = new Set() } = {}) { + this.openAssignmentsWithoutReads = openAssignmentsWithoutReads; + this.openAssignmentsWithReads = openAssignmentsWithReads; + } + + copy() { + return new AssignmentTrackerState({ + openAssignmentsWithoutReads: new Set(this.openAssignmentsWithoutReads), + openAssignmentsWithReads: new Set(this.openAssignmentsWithReads) + }); + } + + merge(other) { + const initialAssignmentsWithoutReadsCount = this.openAssignmentsWithoutReads.size; + const initialAssignmentsWithReadsCount = this.openAssignmentsWithReads.size; + + other.openAssignmentsWithoutReads.forEach(assignment => this.openAssignmentsWithoutReads.add(assignment)); + other.openAssignmentsWithReads.forEach(assignment => this.openAssignmentsWithReads.add(assignment)); + + return this.openAssignmentsWithoutReads.size > initialAssignmentsWithoutReadsCount || + this.openAssignmentsWithReads.size > initialAssignmentsWithReadsCount; + } + + enterAssignment(assignmentExpression) { + (assignmentExpression.operator === "=" ? this.openAssignmentsWithoutReads : this.openAssignmentsWithReads).add(assignmentExpression); + } + + exitAssignment(assignmentExpression) { + this.openAssignmentsWithoutReads.delete(assignmentExpression); + this.openAssignmentsWithReads.delete(assignmentExpression); + } + + exitAwaitOrYield(node, surroundingFunction) { + return [...this.openAssignmentsWithReads] + .filter(assignment => !isLocalVariableWithoutEscape(assignment.left, surroundingFunction)) + .forEach(assignment => { + if (!alreadyReportedAssignments.has(assignment)) { + reportAssignment(assignment); + alreadyReportedAssignments.add(assignment); + } + }); + } + + exitIdentifierOrMemberExpression(node) { + [...this.openAssignmentsWithoutReads] + .filter(assignment => ( + assignment.left !== node && + assignment.left.type === node.type && + astUtils.equalTokens(assignment.left, node, sourceCode) + )) + .forEach(assignment => { + this.openAssignmentsWithoutReads.delete(assignment); + this.openAssignmentsWithReads.add(assignment); + }); + } + } + /** * If the control flow graph of a function enters an assignment expression, then does the * both of the following steps in order (possibly with other steps in between) before exiting the @@ -135,54 +195,51 @@ module.exports = { codePathSegment, surroundingFunction, { - seenSegments = new Set(), - openAssignmentsWithoutReads = new Set(), - openAssignmentsWithReads = new Set() + stateBySegmentStart = new WeakMap(), + stateBySegmentEnd = new WeakMap() } = {} ) { - if (seenSegments.has(codePathSegment)) { - - // An AssignmentExpression can't contain loops, so it's not necessary to reenter them with new state. - return; + if (!stateBySegmentStart.has(codePathSegment)) { + stateBySegmentStart.set(codePathSegment, new AssignmentTrackerState()); } + const currentState = stateBySegmentStart.get(codePathSegment).copy(); + expressionsByCodePathSegment.get(codePathSegment).forEach(({ entering, node }) => { if (node.type === "AssignmentExpression") { if (entering) { - (node.operator === "=" ? openAssignmentsWithoutReads : openAssignmentsWithReads).add(node); + currentState.enterAssignment(node); } else { - openAssignmentsWithoutReads.delete(node); - openAssignmentsWithReads.delete(node); + currentState.exitAssignment(node); } } else if (!entering && (node.type === "AwaitExpression" || node.type === "YieldExpression")) { - [...openAssignmentsWithReads] - .filter(assignment => !isLocalVariableWithoutEscape(assignment.left, surroundingFunction)) - .forEach(reportAssignment); - - openAssignmentsWithReads.clear(); + currentState.exitAwaitOrYield(node, surroundingFunction); } else if (!entering && (node.type === "Identifier" || node.type === "MemberExpression")) { - [...openAssignmentsWithoutReads] - .filter(assignment => ( - assignment.left !== node && - assignment.left.type === node.type && - astUtils.equalTokens(assignment.left, node, sourceCode) - )) - .forEach(assignment => { - openAssignmentsWithoutReads.delete(assignment); - openAssignmentsWithReads.add(assignment); - }); + currentState.exitIdentifierOrMemberExpression(node); } }); + stateBySegmentEnd.set(codePathSegment, currentState); + codePathSegment.nextSegments.forEach(nextSegment => { + if (stateBySegmentStart.has(nextSegment)) { + if (!stateBySegmentStart.get(nextSegment).merge(currentState)) { + + /* + * This segment has already been processed with the given set of inputs; + * no need to do it again. After no new state is available to process + * for any control flow segment in the graph, the analysis reaches a fixpoint and + * traversal stops. + */ + return; + } + } else { + stateBySegmentStart.set(nextSegment, currentState.copy()); + } findOutdatedReads( nextSegment, surroundingFunction, - { - seenSegments: new Set(seenSegments).add(codePathSegment), - openAssignmentsWithoutReads: new Set(openAssignmentsWithoutReads), - openAssignmentsWithReads: new Set(openAssignmentsWithReads) - } + { stateBySegmentStart, stateBySegmentEnd } ); }); } diff --git a/tests/lib/rules/require-atomic-updates.js b/tests/lib/rules/require-atomic-updates.js index eabd521499c..4128357773b 100644 --- a/tests/lib/rules/require-atomic-updates.js +++ b/tests/lib/rules/require-atomic-updates.js @@ -51,7 +51,64 @@ ruleTester.run("require-atomic-updates", rule, { "let foo; function* x() { foo = bar + foo; }", "async function x() { let foo; bar(() => baz += 1); foo += await amount; }", "let foo; async function x() { foo = condition ? foo : await bar; }", - "async function x() { let foo; bar(() => { let foo; blah(foo); }); foo += await result; }" + "async function x() { let foo; bar(() => { let foo; blah(foo); }); foo += await result; }", + "let foo; async function x() { foo = foo + 1; await bar; }", + + + /* + * Ensure rule doesn't take exponential time in the number of branches + * (see https://github.com/eslint/eslint/issues/10893) + */ + ` + async function foo() { + if (1); + if (2); + if (3); + if (4); + if (5); + if (6); + if (7); + if (8); + if (9); + if (10); + if (11); + if (12); + if (13); + if (14); + if (15); + if (16); + if (17); + if (18); + if (19); + if (20); + } + `, + ` + async function foo() { + return [ + 1 ? a : b, + 2 ? a : b, + 3 ? a : b, + 4 ? a : b, + 5 ? a : b, + 6 ? a : b, + 7 ? a : b, + 8 ? a : b, + 9 ? a : b, + 10 ? a : b, + 11 ? a : b, + 12 ? a : b, + 13 ? a : b, + 14 ? a : b, + 15 ? a : b, + 16 ? a : b, + 17 ? a : b, + 18 ? a : b, + 19 ? a : b, + 20 ? a : b + ]; + } + ` ], invalid: [ @@ -59,6 +116,10 @@ ruleTester.run("require-atomic-updates", rule, { code: "let foo; async function x() { foo += await amount; }", errors: [{ messageId: "nonAtomicUpdate", data: { value: "foo" } }] }, + { + code: "if (1); let foo; async function x() { foo += await amount; }", + errors: [{ messageId: "nonAtomicUpdate", data: { value: "foo" } }] + }, { code: "let foo; async function x() { while (condition) { foo += await amount; } }", errors: [VARIABLE_ERROR] @@ -138,6 +199,14 @@ ruleTester.run("require-atomic-updates", rule, { { code: "async function x() { foo += await bar; }", errors: [VARIABLE_ERROR] + }, + { + code: "let foo = 0; async function x() { foo = (a ? b : foo) + await bar; if (baz); }", + errors: [VARIABLE_ERROR] + }, + { + code: "let foo = 0; async function x() { foo = (a ? b ? c ? d ? foo : e : f : g : h) + await bar; if (baz); }", + errors: [VARIABLE_ERROR] } ] });