Fix false positive in require-lastindex-reset-before-global-exec-loop for naturally-exhausting nested exec loops (#53602)

Author: Copilot

eslint-factory/src/rules/require-lastindex-reset-before-global-exec-loop.test.ts

@@ -54,6 +54,24 @@ describe("require-lastindex-reset-before-global-exec-loop", () => {
          }`,
         // Unrelated while loop.
         `while (x < 10) { x++; }`,
+        // Mirrors extractTemporaryIdReferences: module-scope 'g' regex reused across an
+        // outer for-loop's iterations, but the inner while loop's body has no
+        // break/return/throw, so it always drains to natural exhaustion and resets
+        // lastIndex to 0 on its own before the next field is scanned.
+        `const TEMPORARY_ID_PATTERN = /#(aw_[A-Za-z0-9_]{3,12})\\b/gi;
+         function extractTemporaryIdReferences(message) {
+           const tempIds = new Set();
+           const textFields = ["body", "title", "description"];
+           for (const field of textFields) {
+             if (typeof message[field] === "string") {
+               let match;
+               while ((match = TEMPORARY_ID_PATTERN.exec(message[field])) !== null) {
+                 tempIds.add(match[1]);
+               }
+             }
+           }
+           return tempIds;
+         }`,
       ],
       invalid: [
         {
@@ -86,6 +104,44 @@ describe("require-lastindex-reset-before-global-exec-loop", () => {
                  }`,
           errors: [{ messageId: "requireLastIndexReset" }],
         },
+        {
+          // Reused across an outer for-loop's iterations like the valid case above, but
+          // the loop body can return early, so it is not guaranteed to drain the regex
+          // to natural exhaustion each time - the reset is still required.
+          code: `const TEMPORARY_ID_PATTERN = /#(aw_[A-Za-z0-9_]{3,12})\\b/gi;
+                 function extractTemporaryIdReferences(message, stopField) {
+                   const tempIds = new Set();
+                   const textFields = ["body", "title", "description"];
+                   for (const field of textFields) {
+                     if (typeof message[field] === "string") {
+                       let match;
+                       while ((match = TEMPORARY_ID_PATTERN.exec(message[field])) !== null) {
+                         if (field === stopField) return tempIds;
+                         tempIds.add(match[1]);
+                       }
+                     }
+                   }
+                   return tempIds;
+                 }`,
+          errors: [{ messageId: "requireLastIndexReset" }],
+        },
+        {
+          // A `break` targeting the exec loop's own label still stops the loop before
+          // `.exec()` naturally returns null (unlike a `continue` to the same label,
+          // which just moves on to the next iteration), so it can leave `lastIndex`
+          // dirty and must not be exempted just because it's nested in an outer loop.
+          code: `const RE = /foo/g;
+                 function scan(items) {
+                   for (const item of items) {
+                     let m;
+                     inner: while ((m = RE.exec(item)) !== null) {
+                       if (skip(m)) break inner;
+                       use(m);
+                     }
+                   }
+                 }`,
+          errors: [{ messageId: "requireLastIndexReset" }],
+        },
       ],
     });
   });

eslint-factory/src/rules/require-lastindex-reset-before-global-exec-loop.ts

@@ -34,6 +34,95 @@ function getExecLoopRegexName(test: TSESTree.Expression): string | null {
   return callee.object.name;
 }
 
+const LOOP_NODE_TYPES = new Set<string>([AST_NODE_TYPES.ForStatement, AST_NODE_TYPES.ForOfStatement, AST_NODE_TYPES.ForInStatement, AST_NODE_TYPES.WhileStatement, AST_NODE_TYPES.DoWhileStatement]);
+
+/**
+ * Returns true when `node` is nested inside another loop within the same enclosing
+ * function/program (i.e. the exec loop could run again on a later sibling iteration,
+ * reusing the same stateful regex across those iterations).
+ */
+function hasEnclosingLoop(node: TSESTree.Node): boolean {
+  let current: TSESTree.Node | undefined = node.parent;
+  while (
+    current &&
+    current.type !== AST_NODE_TYPES.FunctionDeclaration &&
+    current.type !== AST_NODE_TYPES.FunctionExpression &&
+    current.type !== AST_NODE_TYPES.ArrowFunctionExpression &&
+    current.type !== AST_NODE_TYPES.Program
+  ) {
+    if (LOOP_NODE_TYPES.has(current.type)) return true;
+    current = current.parent;
+  }
+  return false;
+}
+
+/**
+ * Returns the set of labels that directly label `node` (e.g. `outer: while (...) {}`).
+ */
+function getOwnLabels(node: TSESTree.Node): Set<string> {
+  const labels = new Set<string>();
+  let current: TSESTree.Node = node;
+  while (current.parent?.type === AST_NODE_TYPES.LabeledStatement && current.parent.body === current) {
+    labels.add(current.parent.label.name);
+    current = current.parent;
+  }
+  return labels;
+}
+
+/**
+ * Walks `body` (without crossing into nested function scopes) looking for a
+ * `break`/`return`/`throw`, or a labeled `continue` targeting a loop other than the one
+ * labeled by `ownLabels`, any of which would let the loop stop before `.exec()` has a
+ * chance to run out of matches and reset `lastIndex` to 0 naturally.
+ */
+function loopBodyCanExitEarly(body: TSESTree.Node, ownLabels: Set<string>): boolean {
+  let exitsEarly = false;
+
+  function visit(node: unknown, inNestedLoopOrSwitch: boolean): void {
+    if (exitsEarly || !node || typeof node !== "object" || typeof (node as TSESTree.Node).type !== "string") return;
+    const current = node as TSESTree.Node;
+
+    switch (current.type) {
+      case AST_NODE_TYPES.FunctionDeclaration:
+      case AST_NODE_TYPES.FunctionExpression:
+      case AST_NODE_TYPES.ArrowFunctionExpression:
+        return; // Don't cross into nested function scopes; their control flow is independent.
+      case AST_NODE_TYPES.ReturnStatement:
+      case AST_NODE_TYPES.ThrowStatement:
+        exitsEarly = true;
+        return;
+      case AST_NODE_TYPES.BreakStatement:
+        // Unlike continue, break always terminates the loop rather than moving on to
+        // the next iteration, so it can happen before .exec() naturally returns null,
+        // regardless of whether it's labeled with this loop's own label. An unlabeled
+        // break inside a nested loop/switch only exits that nested construct though.
+        if (current.label || !inNestedLoopOrSwitch) exitsEarly = true;
+        return;
+      case AST_NODE_TYPES.ContinueStatement:
+        // A labeled continue whose label isn't this loop's own label targets some
+        // outer loop, ending this loop's iteration early.
+        if (current.label && !ownLabels.has(current.label.name)) exitsEarly = true;
+        return;
+      default:
+        break;
+    }
+
+    const isLoopOrSwitch = LOOP_NODE_TYPES.has(current.type) || current.type === AST_NODE_TYPES.SwitchStatement;
+    for (const key of Object.keys(current)) {
+      if (key === "parent") continue;
+      const value = (current as unknown as Record<string, unknown>)[key];
+      if (Array.isArray(value)) {
+        for (const item of value) visit(item, inNestedLoopOrSwitch || isLoopOrSwitch);
+      } else {
+        visit(value, inNestedLoopOrSwitch || isLoopOrSwitch);
+      }
+    }
+  }
+
+  visit(body, false);
+  return exitsEarly;
+}
+
 export const requireLastIndexResetBeforeGlobalExecLoopRule = createRule({
   name: "require-lastindex-reset-before-global-exec-loop",
   meta: {
@@ -92,6 +181,15 @@ export const requireLastIndexResetBeforeGlobalExecLoopRule = createRule({
         const relevantTextBefore = textBefore.slice(scanStart);
 
         if (!resetPattern.test(relevantTextBefore)) {
+          // If this exec loop is nested inside another loop (reusing the same regex
+          // across sibling iterations, e.g. one `while` per field of an outer `for`)
+          // and its body has no `break`/`return`/`throw`/outer-`continue`, it is
+          // guaranteed to run to natural exhaustion every time, at which point `.exec()`
+          // resets `lastIndex` to 0 on its own. There's nothing to fix in that case.
+          if (hasEnclosingLoop(node) && !loopBodyCanExitEarly(node.body, getOwnLabels(node))) {
+            return;
+          }
+
           context.report({
             node,
             messageId: "requireLastIndexReset",