fix(effect-needs-cleanup): recognize owned replacement timers

.changeset/replacement-timer-cleanup.md

@@ -0,0 +1,5 @@
+---
+"oxlint-plugin-react-doctor": patch
+---
+
+Recognize effect-owned timer replacements that clear the previous handle before assignment and release the final timer during teardown.

packages/fuzz/corpus/regressions/effect-needs-cleanup--replacement-timer-cleanup.tsx

@@ -0,0 +1,40 @@
+// verdict: pass
+// rule: effect-needs-cleanup
+// weakness: copy-tracking
+// source: replacement-timer-cleanup pinned overlay audit, minimized
+import { useEffect } from "react";
+
+export const CursorOverlay = ({ enabled, selector, repositionCursor }) => {
+  useEffect(() => {
+    if (!enabled) return;
+    let scrollResetTimer: ReturnType<typeof setTimeout> | undefined;
+
+    const reposition = () => {
+      if (selector) {
+        try {
+          const element = document.querySelector(selector);
+          if (element) {
+            repositionCursor(element.getBoundingClientRect());
+            clearTimeout(scrollResetTimer);
+            scrollResetTimer = setTimeout(() => repositionCursor(null), 150);
+            return;
+          }
+        } catch (error) {
+          console.debug(error);
+        }
+      }
+      repositionCursor(null);
+      clearTimeout(scrollResetTimer);
+      scrollResetTimer = setTimeout(() => repositionCursor(null), 150);
+    };
+
+    window.addEventListener("scroll", reposition, true);
+    window.addEventListener("resize", reposition);
+    return () => {
+      window.removeEventListener("scroll", reposition, true);
+      window.removeEventListener("resize", reposition);
+      clearTimeout(scrollResetTimer);
+    };
+  }, [enabled, selector, repositionCursor]);
+  return null;
+};

packages/fuzz/corpus/true-positives/effect-needs-cleanup--replacement-timer-cleanup-overwrite.tsx

@@ -0,0 +1,22 @@
+// verdict: fail
+// rule: effect-needs-cleanup
+// weakness: copy-tracking
+// source: replacement-timer-cleanup adversarial overwrite control
+import { useEffect } from "react";
+
+export const ReplacedTimer = ({ update }) => {
+  useEffect(() => {
+    let timer: ReturnType<typeof setTimeout> | undefined;
+    const reposition = () => {
+      clearTimeout(timer);
+      timer = setTimeout(update, 150);
+      timer = setTimeout(update, 150);
+    };
+    window.addEventListener("scroll", reposition);
+    return () => {
+      window.removeEventListener("scroll", reposition);
+      clearTimeout(timer);
+    };
+  }, [update]);
+  return null;
+};

packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect-needs-cleanup-replacement-timer-cleanup.test.ts

@@ -0,0 +1,150 @@
+import * as fs from "node:fs";
+import { describe, expect, it } from "vite-plus/test";
+import { runRule } from "../../../test-utils/run-rule.js";
+import { effectNeedsCleanup } from "./effect-needs-cleanup.js";
+
+const overlaySource = fs
+  .readFileSync(
+    new URL(
+      "../../../../../fuzz/corpus/regressions/effect-needs-cleanup--replacement-timer-cleanup.tsx",
+      import.meta.url,
+    ),
+    "utf8",
+  )
+  .replaceAll("\r\n", "\n");
+
+describe("effect-needs-cleanup replacement timer ownership", () => {
+  it.each([
+    { name: "event-only branch replacements", source: overlaySource },
+    {
+      name: "consecutive replacements with separate clears",
+      source: overlaySource.replace(
+        "scrollResetTimer = setTimeout(() => repositionCursor(null), 150);",
+        `scrollResetTimer = setTimeout(() => repositionCursor(null), 150);
+            clearTimeout(scrollResetTimer);
+            scrollResetTimer = setTimeout(() => repositionCursor(null), 150);`,
+      ),
+    },
+    {
+      name: "transparent wrappers around release and assignment",
+      source: overlaySource
+        .replaceAll("clearTimeout(scrollResetTimer);", "(clearTimeout(scrollResetTimer));")
+        .replaceAll(
+          "scrollResetTimer = setTimeout(() => repositionCursor(null), 150);",
+          "(scrollResetTimer = (setTimeout(() => repositionCursor(null), 150)));",
+        ),
+    },
+    {
+      name: "aliased effect imports",
+      source: overlaySource
+        .replace("{ useEffect }", "{ useEffect as useLifecycle }")
+        .replace("useEffect(()", "useLifecycle(()"),
+    },
+    {
+      name: "interval replacements with matching releases",
+      source: overlaySource
+        .replaceAll("setTimeout", "setInterval")
+        .replaceAll("clearTimeout", "clearInterval"),
+    },
+  ])("accepts $name", ({ source }) => {
+    const result = runRule(effectNeedsCleanup, source);
+    expect(result.parseErrors).toEqual([]);
+    expect(result.diagnostics).toEqual([]);
+  });
+
+  it.each([
+    {
+      name: "missing final timer cleanup",
+      source: overlaySource.replace("clearTimeout(scrollResetTimer);\n    };", "};"),
+    },
+    {
+      name: "missing replacement clear in the selector branch",
+      source: overlaySource.replace("clearTimeout(scrollResetTimer);", ""),
+    },
+    {
+      name: "overwriting before clearing a replacement",
+      source: overlaySource.replace(
+        "clearTimeout(scrollResetTimer);\n            scrollResetTimer = setTimeout(() => repositionCursor(null), 150);",
+        "scrollResetTimer = setTimeout(() => repositionCursor(null), 150);\n            clearTimeout(scrollResetTimer);",
+      ),
+    },
+    {
+      name: "reusing one clear for two consecutive allocations",
+      source: overlaySource.replace(
+        "scrollResetTimer = setTimeout(() => repositionCursor(null), 150);",
+        "scrollResetTimer = setTimeout(() => repositionCursor(null), 150);\n            scrollResetTimer = setTimeout(() => repositionCursor(null), 150);",
+      ),
+    },
+    {
+      name: "conditional final cleanup",
+      source: overlaySource.replace(
+        "clearTimeout(scrollResetTimer);\n    };",
+        "if (selector) clearTimeout(scrollResetTimer);\n    };",
+      ),
+    },
+    {
+      name: "an uncalled nested callback containing the final clear",
+      source: overlaySource.replace(
+        "clearTimeout(scrollResetTimer);\n    };",
+        "const cancel = () => clearTimeout(scrollResetTimer);\n    };",
+      ),
+    },
+    {
+      name: "deferred final cleanup",
+      source: overlaySource.replace(
+        "clearTimeout(scrollResetTimer);\n    };",
+        "queueMicrotask(() => clearTimeout(scrollResetTimer));\n    };",
+      ),
+    },
+    {
+      name: "a listener left registered after unmount",
+      source: overlaySource.replace('window.removeEventListener("resize", reposition);', ""),
+    },
+    {
+      name: "clearing a different handle before replacement",
+      source: overlaySource.replace("clearTimeout(scrollResetTimer);", "clearTimeout(otherTimer);"),
+    },
+    {
+      name: "shadowed timer release",
+      source: overlaySource.replace(
+        "let scrollResetTimer:",
+        "const clearTimeout = () => {};\n    let scrollResetTimer:",
+      ),
+    },
+    {
+      name: "a nested function containing the replacement clear",
+      source: overlaySource.replace(
+        "clearTimeout(scrollResetTimer);",
+        "const cancel = () => clearTimeout(scrollResetTimer);",
+      ),
+    },
+    {
+      name: "replacement clear on only one branch",
+      source: overlaySource.replace(
+        "clearTimeout(scrollResetTimer);",
+        "if (enabled) clearTimeout(scrollResetTimer);",
+      ),
+    },
+    {
+      name: "missing cleanup on one effect exit",
+      source: overlaySource.replace("return () => {", "if (selector) return;\n    return () => {"),
+    },
+    {
+      name: "a callback escaping to an unknown scheduler",
+      source: overlaySource.replace(
+        'window.addEventListener("resize", reposition);',
+        'window.addEventListener("resize", reposition);\n    scheduleLater(reposition);',
+      ),
+    },
+    {
+      name: "a callback that can resume after teardown",
+      source: overlaySource
+        .replace("const reposition = () => {", "const reposition = async () => {")
+        .replace("if (selector) {", "await Promise.resolve();\n      if (selector) {"),
+    },
+  ])("reports $name", ({ source }) => {
+    const result = runRule(effectNeedsCleanup, source);
+    expect(result.parseErrors).toEqual([]);
+    expect(result.diagnostics.length).toBeGreaterThan(0);
+  });
+});

packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/effect-needs-cleanup.ts

@@ -5176,6 +5176,29 @@ const hasOnlySafeHandleStorageAssignments = (
     ) {
       return true;
     }
+    if (assignedTimerUsage && assignmentOwner === currentUsageOwner) {
+      const assignmentStatement = findTransparentExpressionRoot(assignment).parent;
+      const assignmentBlock = assignmentStatement?.parent;
+      if (
+        !isNodeOfType(assignmentStatement, "ExpressionStatement") ||
+        !isNodeOfType(assignmentBlock, "BlockStatement")
+      ) {
+        return false;
+      }
+      const previousStatement =
+        assignmentBlock.body[
+          assignmentBlock.body.findIndex((statement) => statement === assignmentStatement) - 1
+        ];
+      const releaseCall = isNodeOfType(previousStatement, "ExpressionStatement")
+        ? stripParenExpression(previousStatement.expression)
+        : null;
+      return Boolean(
+        isNodeOfType(releaseCall, "CallExpression") &&
+        isNodeOfType(releaseCall.callee, "Identifier") &&
+        context.scopes.isGlobalReference(releaseCall.callee) &&
+        doesReleaseCallMatchUsage(releaseCall, usage, context),
+      );
+    }
     const isNullishReset =
       (isNodeOfType(assignedValue, "Literal") && assignedValue.value === null) ||
       (isNodeOfType(assignedValue, "Identifier") &&
@@ -5330,12 +5353,15 @@ const hasEffectOwnedNestedTimerCleanup = (
   );
   return functionSymbol.references.every((reference) => {
     const referenceKey = resolveExpressionKey(reference.identifier, context);
+    const referenceParent = findTransparentExpressionRoot(reference.identifier).parent;
     if (selfSchedulingReferences.some((candidate) => candidate === reference)) return true;
     const callbackOwnerUsage = allUsages.find(
       (candidateUsage) =>
         candidateUsage !== usage &&
         referenceKey !== null &&
-        getUsageCallbackKey(candidateUsage, context) === referenceKey,
+        getUsageCallbackKey(candidateUsage, context) === referenceKey &&
+        (isAstDescendant(reference.identifier, candidateUsage.node) ||
+          (referenceParent && doesReleaseCallMatchUsage(referenceParent, candidateUsage, context))),
     );
     const callbackOwnerArgument = callbackOwnerUsage
       ? getSubscribeUsageCallbackArgument(callbackOwnerUsage)