feat(cfg): control-flow graph engine, formal-verification stack, and CFG-backed rules

.changeset/cfg-beefup-and-rule-fixes.md

@@ -0,0 +1,11 @@
+---
+"oxlint-plugin-react-doctor": patch
+---
+
+Beef up the control-flow graph and fix the false positives it exposed.
+
+The internal CFG now exposes reachability, dominance, post-dominance, loop-membership, and unreachable-code primitives, models loop back-edges and infinite loops, and gives `try`/`catch`/`finally` proper finalize/join semantics (a `finally` body stays reachable even when the `try` returns; code after the try is unreachable when no path completes normally). Several rules adopt it:
+
+- `nextjs-no-redirect-in-try-catch` no longer mis-flags `redirect()` / `notFound()` in a `catch` block, in a `finally` block, or in a `try` that has only a `finally` (no `catch`) — none of those swallow the navigation control-flow error.
+- `no-mutating-reducer-state` no longer reports a loop that mutates and then `return`s a fresh object (`for (…) { state.items.push(x); return { ...state } }`) when a trailing `return state` only runs on the no-match path.
+- `js-hoist-regexp`, `js-index-maps`, and `js-set-map-lookups` no longer mis-flag work inside a callback that merely escapes a loop (the loop-aware check now uses real CFG loop membership instead of lexical nesting depth).

packages/oxlint-plugin-react-doctor/src/plugin/constants/js.ts

@@ -1,11 +1,3 @@
-export const LOOP_TYPES = [
-  "ForStatement",
-  "ForInStatement",
-  "ForOfStatement",
-  "WhileStatement",
-  "DoWhileStatement",
-];
-
 // ESTree node type names for the three "function-like" syntactic
 // forms — declaration, expression, arrow. Used by the scope analyzer
 // (to bound function scopes) and by `rules-of-hooks` (to skip into

packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-hoist-regexp.test.ts

@@ -0,0 +1,34 @@
+import { describe, expect, it } from "vite-plus/test";
+import { runRule } from "../../../test-utils/run-rule.js";
+import { jsHoistRegexp } from "./js-hoist-regexp.js";
+
+describe("js-hoist-regexp", () => {
+  it("flags `new RegExp(...)` built inside a loop body", () => {
+    const result = runRule(
+      jsHoistRegexp,
+      `function fn(rows) { for (const row of rows) { const re = new RegExp(row.pattern); test(re); } }`,
+    );
+    expect(result.parseErrors).toEqual([]);
+    expect(result.diagnostics).toHaveLength(1);
+  });
+
+  it("does not flag a `new RegExp(...)` outside any loop", () => {
+    const result = runRule(jsHoistRegexp, `const re = new RegExp(pattern);`);
+    expect(result.diagnostics).toHaveLength(0);
+  });
+
+  it("does not flag a `new RegExp(...)` inside a callback that merely escapes the loop", () => {
+    // Regression: the regexp is built per-click, not per-iteration —
+    // the click handler is a separate function with its own acyclic
+    // CFG, so cycle-based loop membership must report nothing.
+    const result = runRule(
+      jsHoistRegexp,
+      `function fn(rows) {
+        for (const row of rows) {
+          row.element.onclick = () => { const re = new RegExp(row.pattern); test(re); };
+        }
+      }`,
+    );
+    expect(result.diagnostics).toHaveLength(0);
+  });
+});

packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-hoist-regexp.ts

@@ -12,7 +12,7 @@ export const jsHoistRegexp = defineRule({
   recommendation:
     "Move `new RegExp(...)` (or large regex literals) to a constant outside the loop so it isn't rebuilt on every pass",
   create: (context: RuleContext) =>
-    createLoopAwareVisitors({
+    createLoopAwareVisitors(context, {
       NewExpression(node: EsTreeNodeOfType<"NewExpression">) {
         if (isNodeOfType(node.callee, "Identifier") && node.callee.name === "RegExp") {
           context.report({

packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-index-maps.ts

@@ -32,7 +32,7 @@ export const jsIndexMaps = defineRule({
   recommendation:
     "Build a `Map` once before the loop instead of calling `array.find(...)` inside it",
   create: (context: RuleContext) =>
-    createLoopAwareVisitors({
+    createLoopAwareVisitors(context, {
       CallExpression(node: EsTreeNodeOfType<"CallExpression">) {
         if (
           !isNodeOfType(node.callee, "MemberExpression") ||

packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-set-map-lookups.ts

@@ -280,7 +280,7 @@ export const jsSetMapLookups = defineRule({
   recommendation:
     "Use a `Set` or `Map` when you check for the same items over and over. `Array.includes`/`find` scans the whole list each time",
   create: (context: RuleContext) =>
-    createLoopAwareVisitors({
+    createLoopAwareVisitors(context, {
       CallExpression(node: EsTreeNodeOfType<"CallExpression">) {
         if (
           !isNodeOfType(node.callee, "MemberExpression") ||

packages/oxlint-plugin-react-doctor/src/plugin/rules/nextjs/nextjs-no-redirect-in-try-catch.test.ts

@@ -0,0 +1,75 @@
+import { describe, expect, it } from "vite-plus/test";
+import { runRule } from "../../../test-utils/run-rule.js";
+import { nextjsNoRedirectInTryCatch } from "./nextjs-no-redirect-in-try-catch.js";
+
+const CASES: ReadonlyArray<{ name: string; code: string; expectedDiagnosticCount: number }> = [
+  {
+    name: "flags redirect() in a try body that has a catch handler",
+    code: `import { redirect } from "next/navigation";
+      export const action = () => { try { redirect("/x"); } catch (e) {} };`,
+    expectedDiagnosticCount: 1,
+  },
+  {
+    name: "ignores redirect() in the catch handler",
+    code: `import { redirect } from "next/navigation";
+      export const action = () => { try {} catch { redirect("/x"); } };`,
+    expectedDiagnosticCount: 0,
+  },
+  {
+    name: "ignores redirect() in the finally block",
+    code: `import { redirect } from "next/navigation";
+      export const action = () => { try {} finally { redirect("/x"); } };`,
+    expectedDiagnosticCount: 0,
+  },
+  {
+    name: "ignores redirect() in a try with only a finally (no catch)",
+    code: `import { redirect } from "next/navigation";
+      export const action = () => { try { redirect("/x"); } finally {} };`,
+    expectedDiagnosticCount: 0,
+  },
+  {
+    name: "flags at the inner try when the nested try is the one that catches",
+    code: `import { redirect } from "next/navigation";
+      export const action = () => { try { try { redirect("/x"); } catch {} } catch {} };`,
+    expectedDiagnosticCount: 1,
+  },
+  {
+    name: "flags an inner-catch redirect that the outer try body swallows",
+    code: `import { redirect } from "next/navigation";
+      export const action = () => { try { try {} catch { redirect("/x"); } } catch {} };`,
+    expectedDiagnosticCount: 1,
+  },
+  {
+    name: "ignores redirect() outside any try",
+    code: `import { redirect } from "next/navigation";
+      export const action = () => { redirect("/x"); };`,
+    expectedDiagnosticCount: 0,
+  },
+  {
+    name: "still flags redirect() nested in an if inside the try body",
+    code: `import { redirect } from "next/navigation";
+      export const action = (x) => { try { if (x) redirect("/x"); } catch (e) {} };`,
+    expectedDiagnosticCount: 1,
+  },
+  {
+    name: "flags notFound() in a try body that has a catch handler",
+    code: `import { notFound } from "next/navigation";
+      export const action = () => { try { notFound(); } catch (e) {} };`,
+    expectedDiagnosticCount: 1,
+  },
+  {
+    name: "flags permanentRedirect() in a try body that has a catch handler",
+    code: `import { permanentRedirect } from "next/navigation";
+      export const action = () => { try { permanentRedirect("/x"); } catch (e) {} };`,
+    expectedDiagnosticCount: 1,
+  },
+];
+
+describe("nextjs-no-redirect-in-try-catch", () => {
+  for (const testCase of CASES) {
+    it(testCase.name, () => {
+      const result = runRule(nextjsNoRedirectInTryCatch, testCase.code);
+      expect(result.diagnostics).toHaveLength(testCase.expectedDiagnosticCount);
+    });
+  }
+});

packages/oxlint-plugin-react-doctor/src/plugin/rules/nextjs/nextjs-no-redirect-in-try-catch.ts

@@ -2,8 +2,27 @@ import { NEXTJS_NAVIGATION_FUNCTIONS } from "../../constants/nextjs.js";
 import { defineRule } from "../../utils/define-rule.js";
 import type { RuleContext } from "../../utils/rule-context.js";
 import { isNodeOfType } from "../../utils/is-node-of-type.js";
+import type { EsTreeNode } from "../../utils/es-tree-node.js";
 import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js";
 
+// A navigation call's control-flow error is only swallowed when the call sits
+// in the `try` BLOCK of a statement that has a `catch` handler — a call in the
+// `catch`/`finally`, or in a `try` whose only companion is `finally`, escapes
+// untouched. So we walk the ancestry for the nearest such enclosing try rather
+// than counting raw try-nesting depth (which over-reports all three cases).
+const isInsideSwallowingTry = (callExpression: EsTreeNode): boolean => {
+  let child: EsTreeNode = callExpression;
+  let ancestor = callExpression.parent ?? null;
+  while (ancestor) {
+    if (isNodeOfType(ancestor, "TryStatement") && ancestor.handler && ancestor.block === child) {
+      return true;
+    }
+    child = ancestor;
+    ancestor = ancestor.parent ?? null;
+  }
+  return false;
+};
+
 export const nextjsNoRedirectInTryCatch = defineRule({
   id: "nextjs-no-redirect-in-try-catch",
   title: "redirect() inside try-catch",
@@ -12,26 +31,16 @@ export const nextjsNoRedirectInTryCatch = defineRule({
   severity: "warn",
   recommendation:
     "Move `redirect()` or `notFound()` outside the try block, or rethrow in `catch`, because these APIs throw control-flow errors that catch blocks swallow.",
-  create: (context: RuleContext) => {
-    let tryCatchDepth = 0;
-
-    return {
-      TryStatement() {
-        tryCatchDepth++;
-      },
-      "TryStatement:exit"() {
-        tryCatchDepth--;
-      },
-      CallExpression(node: EsTreeNodeOfType<"CallExpression">) {
-        if (tryCatchDepth === 0) return;
-        if (!isNodeOfType(node.callee, "Identifier")) return;
-        if (!NEXTJS_NAVIGATION_FUNCTIONS.has(node.callee.name)) return;
+  create: (context: RuleContext) => ({
+    CallExpression(node: EsTreeNodeOfType<"CallExpression">) {
+      if (!isNodeOfType(node.callee, "Identifier")) return;
+      if (!NEXTJS_NAVIGATION_FUNCTIONS.has(node.callee.name)) return;
+      if (!isInsideSwallowingTry(node)) return;
 
-        context.report({
-          node,
-          message: `${node.callee.name}() inside try-catch gets swallowed, so the redirect silently fails.`,
-        });
-      },
-    };
-  },
+      context.report({
+        node,
+        message: `${node.callee.name}() inside try-catch gets swallowed, so the redirect silently fails.`,
+      });
+    },
+  }),
 });

packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-mutating-reducer-state.test.ts

@@ -984,4 +984,54 @@ describe("no-mutating-reducer-state", () => {
     expect(result.parseErrors).toEqual([]);
     expect(Array.isArray(result.diagnostics)).toBe(true);
   });
+
+  it("does not flag a loop mutation that only precedes a fresh-object return", () => {
+    // The 2^N path walker treats the whole loop as one straight-line
+    // statement, so without CFG reachability it wrongly attributes the
+    // `push` to the trailing `return state`. But whenever the `push`
+    // runs, control hits `return { ...state }` (a NEW top-level object —
+    // React re-renders); `return state` is reached only when nothing
+    // matched, i.e. when `push` never ran. Mirrors the if-branch
+    // invariant that mutate-then-return-fresh is out of scope.
+    const result = runRule(
+      noMutatingReducerState,
+      `
+      import { useReducer } from "react";
+
+      function reducer(state, action) {
+        for (const item of action.items) {
+          if (item.id !== action.targetId) continue;
+          state.items.push(item);
+          return { ...state, changed: true };
+        }
+        return state;
+      }
+
+      useReducer(reducer, { items: [] });
+    `,
+    );
+
+    expect(result.parseErrors).toEqual([]);
+    expect(result.diagnostics).toEqual([]);
+  });
+
+  it("still flags a loop mutation that reaches a same-reference return", () => {
+    const result = runRule(
+      noMutatingReducerState,
+      `
+      import { useReducer } from "react";
+
+      function reducer(state, action) {
+        for (const item of action.items) {
+          state.items.push(item);
+        }
+        return state;
+      }
+
+      useReducer(reducer, { items: [] });
+    `,
+    );
+
+    expect(result.diagnostics).toHaveLength(1);
+  });
 });

packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-mutating-reducer-state.ts

@@ -683,9 +683,20 @@ const analyzeReactUseReducerFunctionForStateMutation = (
             statement,
             activeState,
           );
-          const mutationsAtReturn = [...activeState.mutations, ...returnMutations];
           if (canExpressionReturnOriginalReducerStateReference(statement.argument, activeState)) {
-            reportReducerStateMutations(mutationsAtReturn);
+            // The 2^N path walker treats a loop / try body as one straight-line
+            // statement, so it can attribute a remembered mutation to a later
+            // top-level return even when an inner return/break severs that flow
+            // (mutate-then-`return { ...state }` in a loop, with a fallback
+            // `return state` after it). The CFG knows whether the mutation can
+            // actually reach this return; drop the ones that can't. Cross-file
+            // reducer nodes live in another file's CFG, so skip the filter there.
+            const rememberedMutations = options.crossFileConsumerCallSite
+              ? activeState.mutations
+              : activeState.mutations.filter((mutation) =>
+                  context.cfg.isReachable(mutation.node, statement),
+                );
+            reportReducerStateMutations([...rememberedMutations, ...returnMutations]);
           }
           continue;
         }