FP104: accept shared browser timer cleanup APIs

.changeset/fp104-timer-cleanup-pairing.md

@@ -0,0 +1,7 @@
+---
+"oxlint-plugin-react-doctor": patch
+"eslint-plugin-react-doctor": patch
+"react-doctor": patch
+---
+
+Recognize either global timer-clear API as releasing a matching timeout or interval handle, including exhaustive retained timer collections.

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

@@ -0,0 +1,8 @@
+import { useEffect } from "react";
+export const Clock = () => {
+  useEffect(() => {
+    const timer = setInterval(() => {}, 100);
+    return () => clearTimeout(timer);
+  }, []);
+  return null;
+};

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

@@ -0,0 +1,73 @@
+import { describe, expect, it } from "vite-plus/test";
+import { runRule } from "../../../test-utils/run-rule.js";
+import { effectNeedsCleanup } from "./effect-needs-cleanup.js";
+
+describe("shared timeout and interval handle cleanup", () => {
+  it.each([
+    ["setInterval", "clearTimeout"],
+    ["setTimeout", "clearInterval"],
+  ])("accepts %s released with %s", (allocate, release) => {
+    const result = runRule(
+      effectNeedsCleanup,
+      `
+      import { useEffect } from "react";
+      const Clock = () => { useEffect(() => {
+        const timer = ${allocate}(() => update(), 100);
+        return () => ${release}(timer);
+      }, []); return null; };
+    `,
+    );
+    expect(result.parseErrors).toEqual([]);
+    expect(result.diagnostics).toHaveLength(0);
+  });
+
+  it("accepts exhaustive collection cleanup with the other timer API", () => {
+    const result = runRule(
+      effectNeedsCleanup,
+      `
+      import { useEffect } from "react";
+      const Clock = () => { useEffect(() => {
+        const timers = [setInterval(update, 100), setInterval(update, 200)];
+        return () => timers.forEach(clearTimeout);
+      }, []); return null; };
+    `,
+    );
+    expect(result.diagnostics).toHaveLength(0);
+  });
+
+  it.each([
+    "timers.reduce((count, timer) => { clearTimeout(timer); return count + 1; }, 0)",
+    "Array.from(timers, (timer) => { clearTimeout(timer); })",
+  ])("preserves collection callback cleanup: %s", (cleanup) => {
+    const result = runRule(
+      effectNeedsCleanup,
+      `
+      import { useEffect } from "react";
+      const Clock = () => { useEffect(() => {
+        const timers = [setInterval(update, 100), setInterval(update, 200)];
+        return () => { ${cleanup}; };
+      }, []); return null; };
+    `,
+    );
+    expect(result.parseErrors).toEqual([]);
+    expect(result.diagnostics).toHaveLength(0);
+  });
+
+  it.each([
+    "return () => clearTimeout(other);",
+    "return () => { if (enabled) clearTimeout(timer); };",
+    "const clearTimeout = () => {}; return () => clearTimeout(timer);",
+  ])("retains reports for incomplete or invalid release: %s", (cleanup) => {
+    const result = runRule(
+      effectNeedsCleanup,
+      `
+      import { useEffect } from "react";
+      const Clock = () => { useEffect(() => {
+        const timer = setInterval(update, 100);
+        ${cleanup}
+      }, []); return null; };
+    `,
+    );
+    expect(result.diagnostics).toHaveLength(1);
+  });
+});

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

@@ -4242,7 +4242,7 @@ export const Pollers = ({ items, previousTimerIds }) => {
     expect(result.diagnostics).toHaveLength(1);
   });
 
-  it("rejects the wrong clear verb for a mapped interval collection", () => {
+  it("accepts either clear verb for a mapped interval collection", () => {
     const result = runRule(
       effectNeedsCleanup,
       `import { useEffect } from "react";
@@ -4258,7 +4258,7 @@ export const Pollers = ({ items }) => {
 };`,
     );
     expect(result.parseErrors).toEqual([]);
-    expect(result.diagnostics).toHaveLength(1);
+    expect(result.diagnostics).toHaveLength(0);
   });
 
   it("rejects a local mapped timer that is not returned", () => {

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

@@ -3103,8 +3103,6 @@ const isDirectExhaustiveTimerCollectionCleanup = (
   const cleanupCallback = isNodeOfType(cleanupCall, "CallExpression")
     ? cleanupCall.arguments[0]
     : null;
-  const expectedCleanupName =
-    usage.registrationVerbName === "setInterval" ? "clearInterval" : "clearTimeout";
   const retainedCollectionKey = findContainingCollectionKey(usage.node, context);
   if (
     !isNodeOfType(cleanupCall, "CallExpression") ||
@@ -3113,7 +3111,7 @@ const isDirectExhaustiveTimerCollectionCleanup = (
     !isNodeOfType(cleanupCallee.property, "Identifier") ||
     cleanupCallee.property.name !== "forEach" ||
     !isNodeOfType(cleanupCallback, "Identifier") ||
-    cleanupCallback.name !== expectedCleanupName ||
+    !TIMER_CLEANUP_CALLEE_NAMES.has(cleanupCallback.name) ||
     !context.scopes.isGlobalReference(cleanupCallback) ||
     retainedCollectionKey === null ||
     retainedCollectionKey !== resolveExpressionKey(cleanupCallee.object, context)
@@ -6486,13 +6484,14 @@ const effectHasCleanupForUsage = (
       : null;
   const requiresDirectReleasePathCoverage =
     usage.kind === "timer" &&
-    findEnclosingFunction(usage.node) !== callback &&
-    Boolean(
-      assignedHandleSymbol &&
-      (assignedHandleSymbol.kind === "let" || assignedHandleSymbol.kind === "var") &&
-      isNodeOfType(assignedHandleSymbol.declarationNode, "VariableDeclarator") &&
-      findEnclosingFunction(assignedHandleSymbol.declarationNode) === callback,
-    );
+    ((findEnclosingFunction(usage.node) === callback &&
+      isNodeOfType(usageAssignment, "VariableDeclarator")) ||
+      Boolean(
+        assignedHandleSymbol &&
+        (assignedHandleSymbol.kind === "let" || assignedHandleSymbol.kind === "var") &&
+        isNodeOfType(assignedHandleSymbol.declarationNode, "VariableDeclarator") &&
+        findEnclosingFunction(assignedHandleSymbol.declarationNode) === callback,
+      ));
   const matchingCleanupReturns: EsTreeNode[] = [];
   walkInsideStatementBlocks(callback.body, (child: EsTreeNode) => {
     if (!isNodeOfType(child, "ReturnStatement")) return;
@@ -7628,12 +7627,10 @@ const doesReleaseCallMatchUsage = (
   const callee = stripParenExpression(callNode.callee);
 
   if (usage.kind === "timer") {
-    const expectedCleanupName =
-      usage.registrationVerbName === "setInterval" ? "clearInterval" : "clearTimeout";
     if (
       !isNodeOfType(callee, "Identifier") ||
       !TIMER_CLEANUP_CALLEE_NAMES.has(callee.name) ||
-      callee.name !== expectedCleanupName
+      !context.scopes.isGlobalReference(callee)
     ) {
       return false;
     }
@@ -8401,9 +8398,9 @@ const fileContainsReleaseForUsage = (usage: SubscribeLikeUsage, context: RuleCon
   }
   let candidates: ReadonlyArray<EsTreeNode>;
   if (usage.kind === "timer") {
-    const expectedCleanupName =
-      usage.registrationVerbName === "setInterval" ? "clearInterval" : "clearTimeout";
-    candidates = releaseCallIndex.identifierCallsByName.get(expectedCleanupName) ?? [];
+    candidates = [...TIMER_CLEANUP_CALLEE_NAMES].flatMap(
+      (cleanupName) => releaseCallIndex.identifierCallsByName.get(cleanupName) ?? [],
+    );
   } else {
     candidates = releaseCallIndex.potentialNonTimerCalls;
   }