Skip to content

Commit

Permalink
Fix: make require-atomic-updates relax (fixes #11723)
Browse files Browse the repository at this point in the history
  • Loading branch information
mysticatea committed May 24, 2019
1 parent ec105b2 commit 42764b4
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 4 deletions.
40 changes: 36 additions & 4 deletions lib/rules/require-atomic-updates.js
Expand Up @@ -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.
Expand All @@ -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 {
Expand Down Expand Up @@ -235,6 +266,7 @@ module.exports = {
return;
}
const variable = getVariable(reference);
const referenceName = getReferenceName(reference);
const writeExpr = getWriteExpr(reference);

// Add a fresh read variable.
Expand All @@ -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);

Expand Down
17 changes: 17 additions & 0 deletions tests/lib/rules/require-atomic-updates.js
Expand Up @@ -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();
}
}
`
],

Expand Down

0 comments on commit 42764b4

Please sign in to comment.