Skip to content
Permalink
Browse files
Fix: track variables, not names in require-atomic-updates (fixes #14208
…) (#14282)

* Fix: require-atomic-updates false positives in loops (fixes #14208)

* Rework & store variables in segmentInfo instead of variable names

* Skip unresolved references in createReferenceMap
  • Loading branch information
patriscus committed May 8, 2021
1 parent 6a86e50 commit ae6dbd148aaca83e4bd04b9351b54029c50fac8a
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 21 deletions.
@@ -13,6 +13,10 @@
*/
function createReferenceMap(scope, outReferenceMap = new Map()) {
for (const reference of scope.references) {
if (reference.resolved === null) {
continue;
}

outReferenceMap.set(reference.identifier, reference);
}
for (const childScope of scope.childScopes) {
@@ -86,42 +90,42 @@ class SegmentInfo {
* @returns {void}
*/
initialize(segment) {
const outdatedReadVariableNames = new Set();
const freshReadVariableNames = new Set();
const outdatedReadVariables = new Set();
const freshReadVariables = new Set();

for (const prevSegment of segment.prevSegments) {
const info = this.info.get(prevSegment);

if (info) {
info.outdatedReadVariableNames.forEach(Set.prototype.add, outdatedReadVariableNames);
info.freshReadVariableNames.forEach(Set.prototype.add, freshReadVariableNames);
info.outdatedReadVariables.forEach(Set.prototype.add, outdatedReadVariables);
info.freshReadVariables.forEach(Set.prototype.add, freshReadVariables);
}
}

this.info.set(segment, { outdatedReadVariableNames, freshReadVariableNames });
this.info.set(segment, { outdatedReadVariables, freshReadVariables });
}

/**
* Mark a given variable as read on given segments.
* @param {PathSegment[]} segments The segments that it read the variable on.
* @param {string} variableName The variable name to be read.
* @param {Variable} variable The variable to be read.
* @returns {void}
*/
markAsRead(segments, variableName) {
markAsRead(segments, variable) {
for (const segment of segments) {
const info = this.info.get(segment);

if (info) {
info.freshReadVariableNames.add(variableName);
info.freshReadVariables.add(variable);

// If a variable is freshly read again, then it's no more out-dated.
info.outdatedReadVariableNames.delete(variableName);
info.outdatedReadVariables.delete(variable);
}
}
}

/**
* Move `freshReadVariableNames` to `outdatedReadVariableNames`.
* Move `freshReadVariables` to `outdatedReadVariables`.
* @param {PathSegment[]} segments The segments to process.
* @returns {void}
*/
@@ -130,23 +134,23 @@ class SegmentInfo {
const info = this.info.get(segment);

if (info) {
info.freshReadVariableNames.forEach(Set.prototype.add, info.outdatedReadVariableNames);
info.freshReadVariableNames.clear();
info.freshReadVariables.forEach(Set.prototype.add, info.outdatedReadVariables);
info.freshReadVariables.clear();
}
}
}

/**
* Check if a given variable is outdated on the current segments.
* @param {PathSegment[]} segments The current segments.
* @param {string} variableName The variable name to check.
* @param {Variable} variable The variable to check.
* @returns {boolean} `true` if the variable is outdated on the segments.
*/
isOutdated(segments, variableName) {
isOutdated(segments, variable) {
for (const segment of segments) {
const info = this.info.get(segment);

if (info && info.outdatedReadVariableNames.has(variableName)) {
if (info && info.outdatedReadVariables.has(variable)) {
return true;
}
}
@@ -214,14 +218,13 @@ module.exports = {
if (!reference) {
return;
}
const name = reference.identifier.name;
const variable = reference.resolved;
const writeExpr = getWriteExpr(reference);
const isMemberAccess = reference.identifier.parent.type === "MemberExpression";

// Add a fresh read variable.
if (reference.isRead() && !(writeExpr && writeExpr.parent.operator === "=")) {
segmentInfo.markAsRead(codePath.currentSegments, name);
segmentInfo.markAsRead(codePath.currentSegments, variable);
}

/*
@@ -245,7 +248,7 @@ module.exports = {

/*
* Verify assignments.
* If the reference exists in `outdatedReadVariableNames` list, report it.
* If the reference exists in `outdatedReadVariables` list, report it.
*/
":expression:exit"(node) {
const { codePath, referenceMap } = stack;
@@ -267,9 +270,9 @@ module.exports = {
assignmentReferences.delete(node);

for (const reference of references) {
const name = reference.identifier.name;
const variable = reference.resolved;

if (segmentInfo.isOutdated(codePath.currentSegments, name)) {
if (segmentInfo.isOutdated(codePath.currentSegments, variable)) {
context.report({
node: node.parent,
messageId: "nonAtomicUpdate",
@@ -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; }",
"async function x() { foo += await bar; }",


/*
@@ -162,6 +163,52 @@ ruleTester.run("require-atomic-updates", rule, {
count -= 1
return
}
`,

// https://github.com/eslint/eslint/issues/14208
`
async function foo(e) {
}
async function run() {
const input = [];
const props = [];
for(const entry of input) {
const prop = props.find(a => a.id === entry.id) || null;
await foo(entry);
}
for(const entry of input) {
const prop = props.find(a => a.id === entry.id) || null;
}
for(const entry2 of input) {
const prop = props.find(a => a.id === entry2.id) || null;
}
}
`,

`
async function run() {
{
let entry;
await entry;
}
{
let entry;
() => entry;
entry = 1;
}
}
`,

`
async function run() {
await a;
b = 1;
}
`
],

@@ -251,7 +298,7 @@ ruleTester.run("require-atomic-updates", rule, {
errors: [COMPUTED_PROPERTY_ERROR, STATIC_PROPERTY_ERROR]
},
{
code: "async function x() { foo += await bar; }",
code: "let foo = ''; async function x() { foo += await bar; }",
errors: [VARIABLE_ERROR]
},
{

0 comments on commit ae6dbd1

Please sign in to comment.