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

lib/rules/require-atomic-updates.js

@@ -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",

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; }",
+        "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]
         },
         {