diff --git a/lib/rules/require-atomic-updates.js b/lib/rules/require-atomic-updates.js index 09d7670d8198..c48ef793251a 100644 --- a/lib/rules/require-atomic-updates.js +++ b/lib/rules/require-atomic-updates.js @@ -5,11 +5,19 @@ */ "use strict"; +const { getPropertyName } = require("eslint-utils"); + +/** @typedef {import("eslint-scope").Reference} Reference */ +/** @typedef {import("eslint-scope").Scope} Scope */ +/** @typedef {import("eslint-scope").Variable} Variable */ +/** @typedef {import("estree").Identifier} Identifier */ +/** @typedef {import("../linter/code-path-analysis/code-path-segment")} PathSegment */ + /** * Make the map from identifiers to each reference. - * @param {escope.Scope} scope The scope to get references. - * @param {Map} [outReferenceMap] The map from identifier nodes to each reference object. - * @returns {Map} `referenceMap`. + * @param {Scope} scope The scope to get references. + * @param {Map} [outReferenceMap] The map from identifier nodes to each reference object. + * @returns {Map} `referenceMap`. */ function createReferenceMap(scope, outReferenceMap = new Map()) { for (const reference of scope.references) { @@ -24,20 +32,81 @@ function createReferenceMap(scope, outReferenceMap = new Map()) { return outReferenceMap; } +/** + * Get the name of a given reference. + * The name contains property names if the reference is to access to properties. For example: + * - `foo` → `["foo"]` + * - `foo.bar` → `["foo", "bar"]` + * - `foo.bar.baz` → `["foo", "bar", "baz"]` + * - `foo.bar[baz].qiz` → `["foo", "bar"]` // until a dynamic access. + * @param {Reference} reference The reference to get. + * @returns {string[]} The reference name. + */ +function getReferenceName(reference) { + const names = [reference.identifier.name]; + let node = reference.identifier; + + while (node) { + const t = node.parent.type; + + if (t === "MemberExpression" && node.parent.object === node) { + const name = getPropertyName(node.parent, reference.from); + + // If the property name was dynamic, decide the reference name there. + if (!name) { + break; + } + + names.push(name); + node = node.parent; + } else { + break; + } + } + + return names; +} + +/** + * Check if given two names are matched (left-hand match). + * @param {string[]} xs A reference name to check. + * @param {string[]} ys Another reference name to check. + * @returns {boolean} `true` if the two names are matched. + */ +function isReferenceNameMatched(xs, ys) { + const len = Math.min(xs.length, ys.length); + + for (let i = len - 1; i >= 0; --i) { + if (xs[i] !== ys[i]) { + return false; + } + } + + return true; +} + /** * Checks if an expression is a variable that can only be observed within the given function. - * @param {escope.Variable} variable The variable to check + * @param {Variable|null} variable The variable to check + * @param {string[]} referenceName The reference name. * @returns {boolean} `true` if the variable is local to the given function, and is never referenced in a closure. */ -function isLocalVariableWithoutEscape(variable) { +function isLocalVariableWithoutEscape(variable, referenceName) { + if (!variable) { + return false; // A global variable which was not defined. + } + const functionScope = variable.scope.variableScope; - return variable.references.every(reference => - reference.from.variableScope === functionScope); + return variable.references + .filter(reference => isReferenceNameMatched(getReferenceName(reference), referenceName)) + .every(reference => reference.from.variableScope === functionScope); } class SegmentInfo { constructor() { + + /** @type {WeakMap, freshReadVariableNames: Set }>} */ this.info = new WeakMap(); } @@ -47,52 +116,52 @@ class SegmentInfo { * @returns {void} */ initialize(segment) { - const outdatedReadVariables = new Set(); - const freshReadVariables = new Set(); + const outdatedReadVariableNames = new Set(); + const freshReadVariableNames = new Set(); for (const prevSegment of segment.prevSegments) { const info = this.info.get(prevSegment); if (info) { - info.outdatedReadVariables.forEach(Set.prototype.add, outdatedReadVariables); - info.freshReadVariables.forEach(Set.prototype.add, freshReadVariables); + info.outdatedReadVariableNames.forEach(Set.prototype.add, outdatedReadVariableNames); + info.freshReadVariableNames.forEach(Set.prototype.add, freshReadVariableNames); } } - this.info.set(segment, { outdatedReadVariables, freshReadVariables }); + this.info.set(segment, { outdatedReadVariableNames, freshReadVariableNames }); } /** * Mark a given variable as read on given segments. + * If the variable name is `foo.x.y`, this method marks `foo`, `foo.x`, and `foo.x.y`. * @param {PathSegment[]} segments The segments that it read the variable on. - * @param {escope.Variable} variable The variable to be read. + * @param {string[]} variableName The variable name to be read. * @returns {void} */ - markAsRead(segments, variable) { + markAsRead(segments, variableName) { + const keys = variableName.map((_, i, array) => JSON.stringify(array.slice(0, i + 1))); + for (const segment of segments) { const info = this.info.get(segment); if (info) { - info.freshReadVariables.add(variable); + keys.forEach(Set.prototype.add, info.freshReadVariableNames); } } } /** - * Move `freshReadVariables` to `outdatedReadVariables`. + * Move `freshReadVariableNames` to `outdatedReadVariableNames`. * @param {PathSegment[]} segments The segments to process. * @returns {void} */ makeOutdated(segments) { - const vars = new Set(); - for (const segment of segments) { const info = this.info.get(segment); if (info) { - info.freshReadVariables.forEach(Set.prototype.add, info.outdatedReadVariables); - info.freshReadVariables.forEach(Set.prototype.add, vars); - info.freshReadVariables.clear(); + info.freshReadVariableNames.forEach(Set.prototype.add, info.outdatedReadVariableNames); + info.freshReadVariableNames.clear(); } } } @@ -100,14 +169,16 @@ class SegmentInfo { /** * Check if a given variable is outdated on the current segments. * @param {PathSegment[]} segments The current segments. - * @param {escope.Variable} variable The variable to check. + * @param {string[]} variableName The variable name to check. * @returns {boolean} `true` if the variable is outdated on the segments. */ - isOutdated(segments, variable) { + isOutdated(segments, variableName) { + const key = JSON.stringify(variableName); + for (const segment of segments) { const info = this.info.get(segment); - if (info && info.outdatedReadVariables.has(variable)) { + if (info && info.outdatedReadVariableNames.has(key)) { return true; } } @@ -140,44 +211,14 @@ module.exports = { create(context) { const sourceCode = context.getSourceCode(); - const globalScope = context.getScope(); - const dummyVariables = new Map(); const assignmentReferences = new Map(); const segmentInfo = new SegmentInfo(); let stack = null; - /** - * Get the variable of a given reference. - * If it's not defined, returns a dummy object. - * @param {escope.Reference} reference The reference to get. - * @returns {escope.Variable} The variable of the reference. - */ - function getVariable(reference) { - if (reference.resolved) { - return reference.resolved; - } - - // Get or create a dummy. - const name = reference.identifier.name; - let variable = dummyVariables.get(name); - - if (!variable) { - variable = { - name, - scope: globalScope, - references: [] - }; - dummyVariables.set(name, variable); - } - variable.references.push(reference); - - return variable; - } - /** * Get `reference.writeExpr` of a given reference. * If it's the read reference of MemberExpression in LHS, returns RHS in order to address `a.b = await a` - * @param {escope.Reference} reference The reference to get. + * @param {Reference} reference The reference to get. * @returns {Expression|null} The `reference.writeExpr`. */ function getWriteExpr(reference) { @@ -234,12 +275,13 @@ module.exports = { if (!reference) { return; } - const variable = getVariable(reference); + const variable = reference.resolved; + const referenceName = getReferenceName(reference); const writeExpr = getWriteExpr(reference); // Add a fresh read variable. if (reference.isRead() && !(writeExpr && writeExpr.parent.operator === "=")) { - segmentInfo.markAsRead(codePath.currentSegments, variable); + segmentInfo.markAsRead(codePath.currentSegments, referenceName); } /* @@ -248,7 +290,7 @@ module.exports = { */ if (writeExpr && writeExpr.parent.right === writeExpr && // ← exclude variable declarations. - !isLocalVariableWithoutEscape(variable) + !isLocalVariableWithoutEscape(variable, referenceName) ) { let refs = assignmentReferences.get(writeExpr); @@ -263,7 +305,7 @@ module.exports = { /* * Verify assignments. - * If the reference exists in `outdatedReadVariables` list, report it. + * If the reference exists in `outdatedReadVariableNames` list, report it. */ ":expression:exit"(node) { const { codePath, referenceMap } = stack; @@ -285,9 +327,9 @@ module.exports = { assignmentReferences.delete(node); for (const reference of references) { - const variable = getVariable(reference); + const referenceName = getReferenceName(reference); - if (segmentInfo.isOutdated(codePath.currentSegments, variable)) { + if (segmentInfo.isOutdated(codePath.currentSegments, referenceName)) { context.report({ node: node.parent, messageId: "nonAtomicUpdate", diff --git a/tests/lib/rules/require-atomic-updates.js b/tests/lib/rules/require-atomic-updates.js index 1287c356fb22..3cb02de9e619 100644 --- a/tests/lib/rules/require-atomic-updates.js +++ b/tests/lib/rules/require-atomic-updates.js @@ -53,6 +53,7 @@ ruleTester.run("require-atomic-updates", rule, { "let foo; async function x() { foo = condition ? foo : await bar; }", "async function x() { let foo; bar(() => { let foo; blah(foo); }); foo += await result; }", "let foo; async function x() { foo = foo + 1; await bar; }", + "let foo; async function x() { foo.x = foo.y + await bar; }", /* @@ -129,6 +130,23 @@ ruleTester.run("require-atomic-updates", rule, { await doElse(); } } + `, + + // https://github.com/eslint/eslint/issues/11723 + ` + function get() { return {}; } + function update() {} + async function foo(bar) { + let oldBar = await get(bar.id); + let doIt = () => { + bar.visible = true; + }; + if (oldBar) { + bar.installDate = oldBar.installDate; + } else { + bar.installDate = new Date(); + } + } ` ], @@ -228,6 +246,14 @@ ruleTester.run("require-atomic-updates", rule, { { code: "let foo = 0; async function x() { foo = (a ? b ? c ? d ? foo : e : f : g : h) + await bar; if (baz); }", errors: [VARIABLE_ERROR] + }, + { + code: "let foo; async function x() { foo.x = foo.x + await amount; }", + errors: [{ messageId: "nonAtomicUpdate", data: { value: "foo.x" } }] + }, + { + code: "let foo; async function x() { foo.x = foo.x.id + await amount; }", + errors: [{ messageId: "nonAtomicUpdate", data: { value: "foo.x" } }] } ] });