fix(react-doctor): forward reactMajorVersion in programmatic diagnose() + cleanups (#174)

* fix(react-doctor): forward reactMajorVersion in programmatic diagnose() + cleanups

H1 (regression introduced by #172) — `diagnose()` in `src/index.ts`
forgot to forward `reactMajorVersion` to `runOxlint`. After the
directional version-gating change in #172, that meant every
"prefer-newer-api" rule (today: `prefer-use-effect-event`) was silently
skipped for every programmatic API consumer, even on React 19+
projects. The CLI (`scan.ts`) was unaffected because it always passed
the version explicitly.

Fix is one line + one import — mirror what `scan.ts` already does.
Added `tests/diagnose.test.ts` with a regression test that asserts
`prefer-use-effect-event` fires on a React 19 fixture, plus a
symmetric guard that it stays skipped when the React version can't
be parsed (e.g. a github: range).

H2 — updated the stale docstring on `runOxlint`'s `reactMajorVersion`
field. The doc still claimed "`null` means unknown — leave those
rules enabled" but after #172 the null branch is directional
(deprecation-warning rules stay on, prefer-newer-api rules go off).

M1 — `SUB_HANDLER_DIRECT_CALLEE_NAMES` was just an alias of
`TIMER_AND_SCHEDULER_DIRECT_CALLEE_NAMES`. Both names existed for
narrative reasons in different files; knip flagged the duplicate
export. Collapsed to the canonical name and updated the one consumer
(`isCallExpressionWithSubHandlerCallee` in state-and-effects).

L1 — moved `walkInsideStatementBlocks` from inline in
`state-and-effects.ts` to `plugin/helpers.ts` next to its sibling
`walkAst`. It already had four call sites and is the natural
"synchronous-only" walker for any rule asking what runs inside an
effect's own body — colocating with `walkAst` makes future rules
discover it.

L2 (audit finding) — proposed receiver-gating
`post`/`put`/`patch` in `EVENT_TRIGGERED_SIDE_EFFECT_CALLEES`
INTENTIONALLY NOT TAKEN. The canonical "You Might Not Need an Effect"
§6 example is `post(jsonToSubmit)` as a bare callee, so removing
those names breaks textbook detection (3 existing tests / fixtures).
Documented the trade-off in the constants.ts docstring.

Validation
- 642 tests passing (640 baseline + 2 new diagnose-API regressions)
- typecheck / lint / format clean

Co-authored-by: Cursor <cursoragent@cursor.com>

* chore: format scripts/benchmark-scores.ts

Pre-existing formatting issue introduced in 6afdc04 — the script was
committed unformatted, breaking `pnpm format:check` on every PR
branched off main since. Picked up by rebasing this PR.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: aidenybai

packages/react-doctor/src/index.ts

@@ -24,6 +24,7 @@ import { clearProjectCache, discoverProject } from "./utils/discover-project.js"
 import { computeJsxIncludePaths } from "./utils/jsx-include-paths.js";
 import { clearConfigCache, loadConfig } from "./utils/load-config.js";
 import { mergeAndFilterDiagnostics } from "./utils/merge-and-filter-diagnostics.js";
+import { parseReactMajor } from "./utils/parse-react-major.js";
 import { clearPackageJsonCache } from "./utils/read-package-json.js";
 import { createNodeReadFileLinesSync } from "./utils/read-file-lines-node.js";
 import { resolveLintIncludePaths } from "./utils/resolve-lint-include-paths.js";
@@ -130,6 +131,7 @@ export const diagnose = async (
         framework: projectInfo.framework,
         hasReactCompiler: projectInfo.hasReactCompiler,
         hasTanStackQuery: projectInfo.hasTanStackQuery,
+        reactMajorVersion: parseReactMajor(projectInfo.reactVersion),
         includePaths: lintIncludePaths,
         customRulesOnly: userConfig?.customRulesOnly ?? false,
         respectInlineDisables: effectiveRespectInlineDisables,

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

@@ -417,8 +417,6 @@ export const TIMER_AND_SCHEDULER_DIRECT_CALLEE_NAMES = new Set([
   "queueMicrotask",
 ]);
 
-export const SUB_HANDLER_DIRECT_CALLEE_NAMES = TIMER_AND_SCHEDULER_DIRECT_CALLEE_NAMES;
-
 // Timer registrations that ALWAYS need a corresponding cleanup call
 // (a stricter subset of the scheduler list above — `requestAnimationFrame`
 // and friends already invoke once and self-clean, but `setTimeout` /
@@ -575,6 +573,12 @@ export const EXTERNAL_SYNC_OBSERVER_CONSTRUCTORS = new Set([
 // network/analytics side effects. Detection still works via the
 // receiver-bound member-call shape (`analytics.track(...)`,
 // `api.del(...)`) in `EVENT_TRIGGERED_SIDE_EFFECT_MEMBER_METHODS`.
+//
+// `post` / `put` / `patch` are KEPT here — the canonical "You Might
+// Not Need an Effect" §6 example is `post(jsonToSubmit)` as a bare
+// callee, so removing them would silently miss the textbook case.
+// The trade-off (FPs on user helpers named `post(...)`) is acceptable
+// at this scope.
 export const EVENT_TRIGGERED_SIDE_EFFECT_CALLEES = new Set([
   ...FETCH_CALLEE_NAMES,
   // Network shorthand verbs (article uses `post`)

packages/react-doctor/src/plugin/helpers.ts

@@ -53,6 +53,41 @@ export const walkAst = (node: EsTreeNode, visitor: (child: EsTreeNode) => boolea
   }
 };
 
+// HACK: variant of `walkAst` that descends through control-flow blocks
+// (IfStatement / TryStatement / SwitchCase / loops / labels) but stops
+// at any nested function boundary. Used by rules that ask "what runs
+// SYNCHRONOUSLY inside this effect's body?" — counts the
+// `if (cond) setX(...)` write but ignores the deferred
+// `setTimeout(() => setX(...))` one.
+//
+// Unlike `walkAst`, this one does not support pruning via `false`
+// return — descent is always complete except at function boundaries.
+export const walkInsideStatementBlocks = (
+  node: EsTreeNode,
+  visitor: (child: EsTreeNode) => void,
+): void => {
+  if (!node || typeof node !== "object") return;
+  if (
+    node.type === "FunctionDeclaration" ||
+    node.type === "FunctionExpression" ||
+    node.type === "ArrowFunctionExpression"
+  ) {
+    return;
+  }
+  visitor(node);
+  for (const key of Object.keys(node)) {
+    if (key === "parent") continue;
+    const child = node[key];
+    if (Array.isArray(child)) {
+      for (const item of child) {
+        if (item && typeof item === "object" && item.type) walkInsideStatementBlocks(item, visitor);
+      }
+    } else if (child && typeof child === "object" && child.type) {
+      walkInsideStatementBlocks(child, visitor);
+    }
+  }
+};
+
 export const isSetterIdentifier = (name: string): boolean => SETTER_PATTERN.test(name);
 
 export const isSetterCall = (node: EsTreeNode): boolean =>

packages/react-doctor/src/plugin/rules/state-and-effects.ts

@@ -17,8 +17,8 @@ import {
   NAVIGATION_RECEIVER_NAMES,
   REACT_HANDLER_PROP_PATTERN,
   RELATED_USE_STATE_THRESHOLD,
-  SUB_HANDLER_DIRECT_CALLEE_NAMES,
   SUBSCRIPTION_METHOD_NAMES,
+  TIMER_AND_SCHEDULER_DIRECT_CALLEE_NAMES,
   TIMER_CALLEE_NAMES_REQUIRING_CLEANUP,
   TIMER_CLEANUP_CALLEE_NAMES,
   TRIVIAL_DERIVATION_CALLEE_NAMES,
@@ -41,6 +41,7 @@ import {
   isSetterIdentifier,
   isUppercaseName,
   walkAst,
+  walkInsideStatementBlocks,
 } from "../helpers.js";
 import type { EsTreeNode, Rule, RuleContext } from "../types.js";
 
@@ -688,32 +689,6 @@ const collectReturnExpressions = (componentBody: EsTreeNode): EsTreeNode[] => {
   return returns;
 };
 
-const walkInsideStatementBlocks = (
-  node: EsTreeNode,
-  visitor: (child: EsTreeNode) => void,
-): void => {
-  if (!node || typeof node !== "object") return;
-  if (
-    node.type === "FunctionDeclaration" ||
-    node.type === "FunctionExpression" ||
-    node.type === "ArrowFunctionExpression"
-  ) {
-    return;
-  }
-  visitor(node);
-  for (const key of Object.keys(node)) {
-    if (key === "parent") continue;
-    const child = node[key];
-    if (Array.isArray(child)) {
-      for (const item of child) {
-        if (item && typeof item === "object" && item.type) walkInsideStatementBlocks(item, visitor);
-      }
-    } else if (child && typeof child === "object" && child.type) {
-      walkInsideStatementBlocks(child, visitor);
-    }
-  }
-};
-
 const collectIdentifierNames = (expression: EsTreeNode): Set<string> => {
   const names = new Set<string>();
   walkAst(expression, (child: EsTreeNode) => {
@@ -2353,9 +2328,9 @@ export const noMutableInDeps: Rule = {
 //         - a destructured prop named `on[A-Z]…`, OR
 //         - a local declared via `const F = useCallback(...)`
 //   (3) every read of `F` inside the effect body sits inside a sub-
-//       handler (SUB_HANDLER_DIRECT_CALLEE_NAMES, OR a MemberExpression
-//       whose property is in SUBSCRIPTION_METHOD_NAMES — same set the
-//       prefer-use-sync-external-store family uses)
+//       handler (TIMER_AND_SCHEDULER_DIRECT_CALLEE_NAMES, OR a
+//       MemberExpression whose property is in SUBSCRIPTION_METHOD_NAMES
+//       — same set the prefer-use-sync-external-store family uses)
 //   (4) `F` is NEVER read at the effect's own top level
 const collectFunctionTypedLocalBindings = (componentBody: EsTreeNode): Set<string> => {
   const functionTypedLocals = new Set<string>();
@@ -2393,7 +2368,7 @@ const findEnclosingFunctionInsideEffect = (
 const isCallExpressionWithSubHandlerCallee = (callExpression: EsTreeNode): boolean => {
   if (callExpression?.type !== "CallExpression") return false;
   const callee = callExpression.callee;
-  if (callee?.type === "Identifier" && SUB_HANDLER_DIRECT_CALLEE_NAMES.has(callee.name)) {
+  if (callee?.type === "Identifier" && TIMER_AND_SCHEDULER_DIRECT_CALLEE_NAMES.has(callee.name)) {
     return true;
   }
   if (

packages/react-doctor/src/utils/run-oxlint.ts

@@ -848,9 +848,13 @@ interface RunOxlintOptions {
   hasTanStackQuery: boolean;
   /**
    * Major version of React detected for the project. Forwarded to
-   * `createOxlintConfig` so React-19-deprecation rules only fire on
-   * projects where they actually apply. `null` means "unknown — leave
-   * those rules enabled".
+   * `createOxlintConfig`, which gates rules directionally:
+   *   - `"deprecation-warning"` rules (e.g. `no-default-props`) stay
+   *     enabled when this is `null` so mid-migration projects still
+   *     get the warning even if version detection failed.
+   *   - `"prefer-newer-api"` rules (e.g. `prefer-use-effect-event`) are
+   *     skipped when this is `null` to avoid recommending APIs that
+   *     may not exist in the consumer's React version.
    */
   reactMajorVersion?: number | null;
   includePaths?: string[];

packages/react-doctor/tests/diagnose.test.ts

@@ -0,0 +1,77 @@
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import { afterAll, describe, expect, it } from "vite-plus/test";
+
+import { diagnose } from "../src/index.js";
+import { setupReactProject } from "./regressions/_helpers.js";
+
+const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "rd-diagnose-api-"));
+
+afterAll(() => {
+  fs.rmSync(tempRoot, { recursive: true, force: true });
+});
+
+describe("diagnose() programmatic API", () => {
+  // Regression: pre-fix the programmatic `diagnose()` entry forgot to
+  // forward `reactMajorVersion` to `runOxlint`. After the directional
+  // version-gating change, that meant every "prefer-newer-api" rule
+  // (today: `prefer-use-effect-event`) was silently skipped for all
+  // programmatic API consumers, even on React 19+ projects. The CLI
+  // entry (`scan.ts`) was unaffected because it always passed the
+  // version explicitly.
+  it("emits prefer-use-effect-event diagnostics on a React 19 project (the prefer-newer-api version-gated rule fires)", async () => {
+    const projectDir = setupReactProject(tempRoot, "diagnose-prefer-use-effect-event-fires", {
+      files: {
+        "src/Debounced.tsx": `import { useEffect, useState } from "react";
+
+export const Debounced = ({ onChange }: { onChange: (value: string) => void }) => {
+  const [text, setText] = useState("");
+  useEffect(() => {
+    const id = setTimeout(() => onChange(text), 300);
+    return () => clearTimeout(id);
+  }, [text, onChange]);
+  return <input value={text} onChange={(event) => setText(event.target.value)} />;
+};
+`,
+      },
+    });
+
+    const result = await diagnose(projectDir, { lint: true, deadCode: false });
+    const preferUseEffectEventHits = result.diagnostics.filter(
+      (diagnostic) => diagnostic.rule === "prefer-use-effect-event",
+    );
+    expect(preferUseEffectEventHits.length).toBeGreaterThanOrEqual(1);
+  });
+
+  it("skips prefer-use-effect-event when the project's React version cannot be resolved (no react dep)", async () => {
+    // Symmetric guard: when the project has no React dependency the
+    // function throws before lint runs, so we synthesize a project
+    // with an unresolvable React version range. Its major can't be
+    // parsed, so `parseReactMajor` returns null and the prefer-newer-
+    // api rule should be skipped pessimistically — confirming the
+    // forward really is honoring the version-gate boundary.
+    const projectDir = setupReactProject(tempRoot, "diagnose-prefer-use-effect-event-skipped", {
+      reactVersion: "github:facebook/react",
+      files: {
+        "src/Debounced.tsx": `import { useEffect, useState } from "react";
+
+export const Debounced = ({ onChange }: { onChange: (value: string) => void }) => {
+  const [text, setText] = useState("");
+  useEffect(() => {
+    const id = setTimeout(() => onChange(text), 300);
+    return () => clearTimeout(id);
+  }, [text, onChange]);
+  return <input value={text} onChange={(event) => setText(event.target.value)} />;
+};
+`,
+      },
+    });
+
+    const result = await diagnose(projectDir, { lint: true, deadCode: false });
+    const preferUseEffectEventHits = result.diagnostics.filter(
+      (diagnostic) => diagnostic.rule === "prefer-use-effect-event",
+    );
+    expect(preferUseEffectEventHits).toHaveLength(0);
+  });
+});

scripts/benchmark-scores.ts

@@ -35,8 +35,7 @@ interface BenchmarkResult {
   errorMessage: string | null;
 }
 
-const gh = (args: string): string =>
-  execSync(`gh ${args}`, { encoding: "utf-8" }).trim();
+const gh = (args: string): string => execSync(`gh ${args}`, { encoding: "utf-8" }).trim();
 
 const getLatestRunId = (): number => {
   const output = gh(
@@ -157,9 +156,7 @@ const main = async (): Promise<void> => {
     const results = await Promise.all(
       artifacts.map(({ id }) => downloadAndParseArtifact(id, tempDirectory)),
     );
-    const validResults = results.filter(
-      (result): result is BenchmarkResult => result !== null,
-    );
+    const validResults = results.filter((result): result is BenchmarkResult => result !== null);
 
     if (validResults.length === 0) {
       console.error("No valid benchmark results found");