From 42764b43f791b81998f935ecf25d009fecf6164b Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Fri, 24 May 2019 20:40:01 +0900 Subject: [PATCH] Fix: make require-atomic-updates relax (fixes #11723) --- lib/rules/require-atomic-updates.js | 40 ++++++++++++++++++++--- tests/lib/rules/require-atomic-updates.js | 17 ++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/lib/rules/require-atomic-updates.js b/lib/rules/require-atomic-updates.js index 09d7670d819..2f5cd942744 100644 --- a/lib/rules/require-atomic-updates.js +++ b/lib/rules/require-atomic-updates.js @@ -5,6 +5,8 @@ */ "use strict"; +const { getPropertyName } = require("eslint-utils"); + /** * Make the map from identifiers to each reference. * @param {escope.Scope} scope The scope to get references. @@ -24,16 +26,45 @@ 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"` + * @param {escope.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) { + names.push(getPropertyName(node.parent, reference.from) || "*"); + node = node.parent; + } else { + break; + } + } + + return names.join("."); +} + /** * 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 {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) { const functionScope = variable.scope.variableScope; - return variable.references.every(reference => - reference.from.variableScope === functionScope); + return variable.references + .filter(reference => getReferenceName(reference) === referenceName) + .every(reference => reference.from.variableScope === functionScope); } class SegmentInfo { @@ -235,6 +266,7 @@ module.exports = { return; } const variable = getVariable(reference); + const referenceName = getReferenceName(reference); const writeExpr = getWriteExpr(reference); // Add a fresh read variable. @@ -248,7 +280,7 @@ module.exports = { */ if (writeExpr && writeExpr.parent.right === writeExpr && // ← exclude variable declarations. - !isLocalVariableWithoutEscape(variable) + !isLocalVariableWithoutEscape(variable, referenceName) ) { let refs = assignmentReferences.get(writeExpr); diff --git a/tests/lib/rules/require-atomic-updates.js b/tests/lib/rules/require-atomic-updates.js index 1287c356fb2..9eb89f95168 100644 --- a/tests/lib/rules/require-atomic-updates.js +++ b/tests/lib/rules/require-atomic-updates.js @@ -129,6 +129,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(); + } + } ` ],