fix: resolve cache/navigation binding and unsafe mutation parallelization

.changeset/fix-binding-resolution-and-mutations.md

@@ -0,0 +1,11 @@
+---
+"oxlint-plugin-react-doctor": patch
+---
+
+Fix cache/navigation binding resolution and unsafe mutation parallelization (issue #1810)
+
+- **server-cache-with-object-literal**: Properly resolve React.cache imports through aliases and check all argument positions for fresh objects/arrays. Shadowed or non-React cache functions no longer trigger false positives.
+
+- **nextjs-no-redirect-in-try-catch**: Recognize Next.js `unstable_rethrow(error)` as a valid error forwarding pattern, suppressing the diagnostic when the caught error is correctly rethrown.
+
+- **server-sequential-independent-await** and **async-parallel**: Detect mutating HTTP requests (POST, PUT, PATCH, DELETE) and preserve their ordering, preventing incorrect parallelization suggestions for operations that must run sequentially.

packages/fuzz/corpus/regressions/async-parallel--ordered-http-mutations.tsx

@@ -0,0 +1,10 @@
+// rule: async-parallel
+// verdict: pass
+// Source: issue #1810 and PR #1811 review.
+// Weakness: HTTP writes must preserve their sequential order.
+export async function update() {
+  const first = await fetch("/create", { method: "POST" });
+  const second = await fetch("/update", { method: "PATCH" });
+  const third = await fetch("/delete", { method: "DELETE" });
+  return [first, second, third];
+}

packages/fuzz/corpus/regressions/nextjs-no-redirect-in-try-catch--framework-rethrow.tsx

@@ -0,0 +1,12 @@
+// rule: nextjs-no-redirect-in-try-catch
+// verdict: pass
+// Source: issue #1810 and PR #1811 review.
+// Weakness: Framework rethrows must forward the actual caught binding.
+import { redirect, unstable_rethrow } from "next/navigation";
+export function Page() {
+  try {
+    redirect("/done");
+  } catch (error) {
+    unstable_rethrow(error);
+  }
+}

packages/fuzz/corpus/regressions/server-cache-with-object-literal--mutable-alias.tsx

@@ -0,0 +1,9 @@
+// rule: server-cache-with-object-literal
+// verdict: pass
+// Source: issue #1810 and PR #1811 review.
+// Weakness: A reassigned alias no longer identifies a cached function.
+import { cache } from "react";
+const read = cache(load);
+let alias = read;
+alias = other;
+export const result = alias({ id: 1 });

packages/fuzz/corpus/regressions/server-sequential-independent-await--ordered-http-mutations.tsx

@@ -0,0 +1,9 @@
+// rule: server-sequential-independent-await
+// verdict: pass
+// Source: issue #1810 and PR #1811 review.
+// Weakness: HTTP writes must preserve their sequential order.
+export async function update() {
+  const first = await fetch("/create", { method: "POST" });
+  const second = await fetch("/update", { method: "PATCH" });
+  return [first, second];
+}

packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/async-parallel.regressions.test.ts

@@ -399,4 +399,70 @@ async function loadDashboard(api) {
 `,
     );
   });
+
+  it("stays silent when the first await is a POST mutation", () => {
+    expectPass(
+      `export default async function handler() {
+  const created = await fetch("/api/users", { method: "POST", body: data });
+  const user = await fetch("/api/user");
+  const posts = await fetch("/api/posts");
+  return { created, user, posts };
+}`,
+    );
+  });
+
+  it("stays silent when any await is a PUT mutation", () => {
+    expectPass(
+      `export default async function handler() {
+  const user = await fetch("/api/user");
+  const updated = await fetch("/api/users/1", { method: "PUT", body: data });
+  const posts = await fetch("/api/posts");
+  return { user, updated, posts };
+}`,
+    );
+  });
+
+  it("stays silent when any await is a PATCH mutation", () => {
+    expectPass(
+      `export default async function handler() {
+  const user = await fetch("/api/user");
+  const posts = await fetch("/api/posts");
+  const patched = await fetch("/api/users/1", { method: "PATCH", body: data });
+  return { user, posts, patched };
+}`,
+    );
+  });
+
+  it("stays silent when any await is a DELETE mutation", () => {
+    expectPass(
+      `export default async function handler() {
+  const user = await fetch("/api/user");
+  const posts = await fetch("/api/posts");
+  const deleted = await fetch("/api/users/1", { method: "DELETE" });
+  return { user, posts, deleted };
+}`,
+    );
+  });
+
+  it("stays silent for lowercase mutating method names", () => {
+    expectPass(
+      `export default async function handler() {
+  const created = await fetch("/api/users", { method: "post", body: data });
+  const user = await fetch("/api/user");
+  const posts = await fetch("/api/posts");
+  return { created, user, posts };
+}`,
+    );
+  });
+
+  it("still flags when all fetches are GET", () => {
+    expectFail(
+      `export default async function handler() {
+  const user = await fetch("/api/user", { method: "GET" });
+  const posts = await fetch("/api/posts", { method: "GET" });
+  const comments = await fetch("/api/comments", { method: "GET" });
+  return { user, posts, comments };
+}`,
+    );
+  });
 });

packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/async-parallel.ts

@@ -7,6 +7,7 @@ import {
 import { SEQUENTIAL_AWAIT_THRESHOLD } from "../../constants/thresholds.js";
 import { defineRule } from "../../utils/define-rule.js";
 import { expressionReadsPatternBinding } from "../../utils/expression-reads-pattern-binding.js";
+import { findSideEffect } from "../../utils/find-side-effect.js";
 import { isFunctionLike } from "../../utils/is-function-like.js";
 import { normalizeFilename } from "../../utils/normalize-filename.js";
 import { getCalleeIdentifierTrail } from "../../utils/get-callee-identifier-trail.js";
@@ -92,11 +93,11 @@ const isNonCallAwait = (statement: EsTreeNode): boolean => {
 
 // Skip a consecutive-await block whenever any one of its awaits is an
 // ordered-UI-flow call, an intentional sequencing call, a bare
-// side-effect await, or an await of an already-started promise. A single
-// `await page.click(...)` in the middle of three otherwise-independent
-// awaits is enough to mark the whole sequence as deliberately
-// serialized — collapsing it into `Promise.all([...])` would change
-// observable behavior.
+// side-effect await, an await of an already-started promise, or a mutating
+// HTTP request. A single `await page.click(...)` in the middle of three
+// otherwise-independent awaits is enough to mark the whole sequence as
+// deliberately serialized — collapsing it into `Promise.all([...])` would
+// change observable behavior.
 const sequenceContainsSerializationSignal = (
   statements: EsTreeNode[],
   context: RuleContext,
@@ -106,6 +107,8 @@ const sequenceContainsSerializationSignal = (
     if (isNonCallAwait(statement)) return true;
     const awaitedCall = getAwaitedCall(statement);
     if (awaitedCall && hasPossibleStaticMemberCallWrite(awaitedCall, context.scopes)) return true;
+    if (awaitedCall && findSideEffect(awaitedCall, { shouldTraverseNestedFunction: () => false }))
+      return true;
     const orderIndependentFunction = awaitedCall
       ? getOrderIndependentLocalFunction(awaitedCall, context.scopes)
       : null;

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

@@ -246,4 +246,153 @@ export default async function Page() {
     expect(result.parseErrors).toEqual([]);
     expect(result.diagnostics).toEqual([]);
   });
+
+  it("stays silent when unstable_rethrow forwards the caught error", () => {
+    const result = runRule(
+      nextjsNoRedirectInTryCatch,
+      `import { redirect, unstable_rethrow } from "next/navigation";
+export default async function Page() {
+  try {
+    await save();
+    redirect("/done");
+  } catch (error) {
+    unstable_rethrow(error);
+    console.error(error);
+  }
+}`,
+    );
+    expect(result.parseErrors).toEqual([]);
+    expect(result.diagnostics).toEqual([]);
+  });
+
+  it("stays silent when unstable_rethrow is used as a namespace import", () => {
+    const result = runRule(
+      nextjsNoRedirectInTryCatch,
+      `import * as nav from "next/navigation";
+export default async function Page() {
+  try {
+    await save();
+    nav.redirect("/done");
+  } catch (error) {
+    nav.unstable_rethrow(error);
+    console.error(error);
+  }
+}`,
+    );
+    expect(result.parseErrors).toEqual([]);
+    expect(result.diagnostics).toEqual([]);
+  });
+
+  it("still flags when unstable_rethrow is called with a different binding", () => {
+    const result = runRule(
+      nextjsNoRedirectInTryCatch,
+      `import { redirect, unstable_rethrow } from "next/navigation";
+const savedError = new Error("saved");
+export default async function Page() {
+  try {
+    redirect("/done");
+  } catch (error) {
+    unstable_rethrow(savedError);
+    console.error(error);
+  }
+}`,
+    );
+    expect(result.parseErrors).toEqual([]);
+    expect(result.diagnostics.length).toBeGreaterThan(0);
+  });
+
+  it("stays silent when a renamed unstable_rethrow forwards the caught error", () => {
+    const result = runRule(
+      nextjsNoRedirectInTryCatch,
+      `import { redirect, unstable_rethrow as rethrow } from "next/navigation";
+export default async function Page() {
+  try {
+    await save();
+    redirect("/done");
+  } catch (error) {
+    rethrow(error);
+    console.error(error);
+  }
+}`,
+    );
+    expect(result.parseErrors).toEqual([]);
+    expect(result.diagnostics).toEqual([]);
+  });
+
+  it("still flags when unstable_rethrow is deferred in a callback", () => {
+    const result = runRule(
+      nextjsNoRedirectInTryCatch,
+      `import { redirect, unstable_rethrow } from "next/navigation";
+export default async function Page() {
+  try {
+    redirect("/done");
+  } catch (error) {
+    setTimeout(() => unstable_rethrow(error), 0);
+  }
+}`,
+    );
+    expect(result.parseErrors).toEqual([]);
+    expect(result.diagnostics.length).toBeGreaterThan(0);
+  });
+
+  it("reports when a local unstable_rethrow shadows the import", () => {
+    const result = runRule(
+      nextjsNoRedirectInTryCatch,
+      `import { redirect } from "next/navigation";
+const unstable_rethrow = (e) => { /* noop */ };
+export default async function Page() {
+  try {
+    redirect("/done");
+  } catch (error) {
+    unstable_rethrow(error);
+  }
+}`,
+    );
+    expect(result.parseErrors).toEqual([]);
+    expect(result.diagnostics.length).toBeGreaterThan(0);
+  });
+  it.each([
+    `try { unstable_rethrow(error); } catch (inner) { console.error(inner); }`,
+    `{ const error = new Error("other"); unstable_rethrow(error); }`,
+  ])("reports when the framework rethrow does not forward the caught error: %s", (catchBody) => {
+    const result = runRule(
+      nextjsNoRedirectInTryCatch,
+      `
+      import { redirect, unstable_rethrow } from "next/navigation";
+      export function Page() {
+        try { redirect("/done"); } catch (error) { ${catchBody} }
+      }
+    `,
+    );
+    expect(result.parseErrors).toEqual([]);
+    expect(result.diagnostics).toHaveLength(1);
+  });
+  it("reports namespace navigation when the catch swallows the error", () => {
+    const result = runRule(
+      nextjsNoRedirectInTryCatch,
+      `
+      import * as navigation from "next/navigation";
+      export function Page() {
+        try { navigation.redirect("/done"); } catch (error) { console.error(error); }
+      }
+    `,
+    );
+    expect(result.parseErrors).toEqual([]);
+    expect(result.diagnostics).toHaveLength(1);
+  });
+  it("accepts a framework rethrow forwarded through a nested catch", () => {
+    const result = runRule(
+      nextjsNoRedirectInTryCatch,
+      `
+      import { redirect, unstable_rethrow } from "next/navigation";
+      export function Page() {
+        try { redirect("/done"); } catch (error) {
+          try { unstable_rethrow(error); } catch (inner) { unstable_rethrow(inner); }
+        }
+      }
+    `,
+    );
+    expect(result.parseErrors).toEqual([]);
+    expect(result.diagnostics).toEqual([]);
+  });
 });

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

@@ -1,10 +1,10 @@
 import { NEXTJS_NAVIGATION_FUNCTIONS } from "../../constants/nextjs.js";
 import { defineRule } from "../../utils/define-rule.js";
 import { findGuardingTryStatement } from "../../utils/find-guarding-try-statement.js";
-import { getImportedNameFromModule } from "../../utils/find-import-source-for-name.js";
 import type { RuleContext } from "../../utils/rule-context.js";
 import { isNodeOfType } from "../../utils/is-node-of-type.js";
 import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js";
+import { resolveImportedApiReference } from "../../utils/resolve-imported-api-reference.js";
 
 export const nextjsNoRedirectInTryCatch = defineRule({
   id: "nextjs-no-redirect-in-try-catch",
@@ -16,21 +16,36 @@ export const nextjsNoRedirectInTryCatch = defineRule({
     "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) => ({
     CallExpression(node: EsTreeNodeOfType<"CallExpression">) {
-      if (!isNodeOfType(node.callee, "Identifier")) return;
-      // Resolve to the actual next/navigation export so a local function of the
-      // same name (`const redirect = ...`) is never flagged.
-      const importedName = getImportedNameFromModule(node, node.callee.name, "next/navigation");
-      if (!importedName || !NEXTJS_NAVIGATION_FUNCTIONS.has(importedName)) return;
+      const navigationReference = resolveImportedApiReference(node.callee, context.scopes);
+      if (
+        navigationReference?.source !== "next/navigation" ||
+        !navigationReference.importedName ||
+        !NEXTJS_NAVIGATION_FUNCTIONS.has(navigationReference.importedName)
+      )
+        return;
 
-      // findGuardingTryStatement resolves the try/catch that actually
-      // swallows the thrown control-flow error, climbing past re-throwing
-      // catches, bare try/finally, and IIFE boundaries.
-      const guardingTry = findGuardingTryStatement(node);
+      const frameworkRethrowPredicate = (
+        expression: EsTreeNodeOfType<"CallExpression">,
+        caughtBinding: EsTreeNodeOfType<"Identifier">,
+      ): boolean => {
+        const rethrowReference = resolveImportedApiReference(expression.callee, context.scopes);
+        if (
+          rethrowReference?.source !== "next/navigation" ||
+          rethrowReference.importedName !== "unstable_rethrow"
+        )
+          return false;
+        const argument = expression.arguments[0];
+        if (!isNodeOfType(argument, "Identifier")) return false;
+        const caughtSymbol = context.scopes.symbolFor(caughtBinding);
+        return Boolean(caughtSymbol && context.scopes.symbolFor(argument) === caughtSymbol);
+      };
+
+      const guardingTry = findGuardingTryStatement(node, frameworkRethrowPredicate);
       if (!guardingTry) return;
 
       context.report({
         node,
-        message: `${node.callee.name}() inside try-catch gets swallowed, so the redirect silently fails.`,
+        message: `${navigationReference.importedName}() inside try-catch gets swallowed, so the redirect silently fails.`,
       });
     },
   }),