chore(hir): strip dead code, redundant comments, debug logging

Cleanup pass following round-5 review. No behavior changes — all 490
tests pass, lint/typecheck/format clean.

Removed:
- types.ts: unused predicates (isStateValueType, isUseStateHookType,
  isUseRefType, isRefValueType, isEffectEventType, isPropCallbackType),
  unused 'prop' Identifier.origin variant, unused EffectKind union
  members (Mutate / Capture / Store / Freeze).
- print.ts: entire 80-line file (printHIR was only referenced by
  hir-unit.test.ts's debug logging, which is also removed).
- validate-no-derived-computations-in-effects.ts: dead 'reason'
  field on findings — only one value was ever consumed by the
  runner; collapsed validator to return null on the other paths.
- hir-unit.test.ts: console.log + JSON.stringify debug output that
  spammed CI on every run; replaced with proper expect()s.
- 200+ lines of doc-style comments across types.ts / lower.ts /
  infer-types.ts / runner.ts / validators / hir-port.test.ts that
  re-explained what the code says or repeated information from the
  PR description.

Kept (with concise // HACK: prefix):
- StateTuple discriminator (real semantic distinction)
- SpreadElement unwrap (real ESTree wart)
- Catch-param binding (real correctness fix)
- Multi-statement-with-locals defer (real overlap mitigation)
- Inner-call-site Place tracking (real diagnostic-anchor concern)

Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

Author: cursoragent

packages/react-doctor/src/oxlint-config.ts

@@ -248,16 +248,11 @@ export const GLOBAL_REACT_DOCTOR_RULES: Record<string, RuleSeverity> = {
   "react-doctor/rerender-defer-reads-hook": "warn",
   "react-doctor/advanced-event-handler-refs": "warn",
 
-  // HACK: HIR-backed rules — v1 of the IR-based analysis pass. Known
-  // overlap with the AST-walker `no-derived-state-effect`: both fire
-  // on the canonical "useEffect with single setter call deriving
-  // from deps" shape, producing two diagnostics on the same line.
-  // The HIR rule additionally catches multi-statement-with-locals
-  // shapes the AST walker misses. Future work: scope the HIR rule
-  // so it only reports on shapes the AST walker wouldn't, then
-  // retire the walker. Until then, users who don't want the
-  // duplicate can disable the AST walker rule (or this one) via
-  // `react-doctor.config.json`.
+  // HACK: HIR-backed rules. `hir-no-derived-computations-in-effects`
+  // is scoped at the validator level to defer to the AST-walker
+  // `no-derived-state-effect` on the simple shape, but
+  // `hir-no-set-state-in-effect` still overlaps with the walker on
+  // single-setter effects — disable either via config to dedupe.
   "react-doctor/hir-no-set-state-in-effect": "warn",
   "react-doctor/hir-no-derived-computations-in-effects": "warn",
 

packages/react-doctor/src/plugin/hir/index.ts

@@ -1,5 +1,4 @@
 export * from "./types.js";
 export { lowerFunction } from "./lower.js";
 export { inferTypes } from "./infer-types.js";
-export { printHIR } from "./print.js";
 export { hirNoSetStateInEffect, hirNoDerivedComputationsInEffects } from "./runner.js";

packages/react-doctor/src/plugin/hir/infer-types.ts

@@ -1,21 +1,9 @@
 import type { HIRFunction, Identifier, ReactType } from "./types.js";
 
-// HACK: pared-down version of the compiler's `inferTypes()` pass. The
-// compiler runs full unification across hook return shapes; we need
-// just enough to power our validators — recognize the React hook
-// callees and tag the values they produce.
-//
-// Strategy: walk every instruction once, tagging identifiers by:
-//   1. LoadGlobal of a known React hook name → tag the binding's type
-//   2. CallExpression / MethodCall whose callee identifier is tagged
-//      as a hook → propagate the return type to the lvalue (and to
-//      array-destructure children for `useState` / `useReducer`)
-//   3. PropertyLoad of `.current` on a `RefValue` → tag the lvalue
-//      as `RefCurrent`
-//   4. StoreLocal / LoadLocal → propagate the source identifier's type
-//      so `const fn = setState; fn(x)` is still seen as a setState
-//      call, the way the compiler tracks setState through `LoadLocal`
-//      and `StoreLocal` in `validateNoSetStateInEffects`.
+// HACK: pared-down `inferTypes` pass — recognizes React hook callees
+// (LoadGlobal name match), propagates return types to call results,
+// and threads types through LoadLocal / StoreLocal so aliased setters
+// stay typed as `StateSetter`.
 
 const REACT_HOOK_NAME_TO_TYPE: Record<string, ReactType> = {
   useState: "UseStateHook",
@@ -84,22 +72,13 @@ export const inferTypes = (fn: HIRFunction): void => {
         }
         case "PropertyLoad": {
           const objectType = instr.value.object.identifier.type;
-          // `const [value, setValue] = useState(...)` lowers to two
-          // PropertyLoad instructions on the useState return. Index 0
-          // is the state value, index 1 is the setter. Gated on
-          // `StateTuple` (not generic `Object`) so a `useMemo`
-          // returning an array doesn't have its destructure
-          // misclassified as state — the lvalue of that PropertyLoad
-          // would otherwise get StateSetter and trigger
-          // `validateNoSetStateInEffects` on a non-setter call.
           if (objectType === "StateTuple" && instr.value.computed) {
             if (instr.value.property === "0" && lvalue) {
               setIdentifierType(lvalue.identifier, "StateValue");
             } else if (instr.value.property === "1" && lvalue) {
               setIdentifierType(lvalue.identifier, "StateSetter");
             }
           }
-          // `<refIdent>.current` access
           if (
             objectType === "RefValue" &&
             !instr.value.computed &&
@@ -126,13 +105,4 @@ export const inferTypes = (fn: HIRFunction): void => {
       }
     }
   }
-
-  // HACK: useState/useReducer destructuring — heuristic above only
-  // works when the lowering went useState → ArrayPattern → indexed
-  // PropertyLoad. To make the StateValue/StateSetter tagging robust
-  // against the call's lvalue staying `Object` (no real array shape),
-  // we run a second sweep that finds `<x> = useState(...)` followed
-  // by a `[a, b] = <x>`-style pattern. This is already covered by the
-  // PropertyLoad branch above as long as `lower.ts` emits the indexed
-  // loads, which it does.
 };

packages/react-doctor/src/plugin/hir/lower.ts

@@ -16,30 +16,10 @@ import type {
   Terminal,
 } from "./types.js";
 
-// HACK: lower ESTree → HIR. Mirrors the structure of React Compiler's
-// `BuildHIR.ts::lower` but with several deliberate simplifications:
-//
-//   - single block per function (no control flow modeled in v1)
-//   - no SSA: a mutable name→Place table tracks current bindings
-//   - no aliasing / mutation effect inference (every Place gets
-//     `effect: 'Read'` for v1; validators don't depend on this yet)
-//   - JSX folded into a single `JSXExpression` placeholder per JSX
-//     node — we don't need to model individual elements/attrs to
-//     detect setState calls in a return position
-//
-// What carries over from the compiler:
-//   - lvalue / value-place / operand-place shape so validators can
-//     `switch (instr.value.kind)` exactly like the upstream code
-//   - identifier IDs are stable per-binding, so propagation analyses
-//     (e.g. setState flowing through a const) work the same way
-//   - source locations threaded through every Place
-
 interface LoweringEnvironment {
-  // HACK: id allocators are SHARED across all nested envs of the same
-  // lowering. A child env references the same allocator object so a
-  // captured outer binding keeps its IdentifierId when seen by the
-  // inner function — this is how the compiler's `loweredFunc.func.context`
-  // ends up referencing identifiers shared with the outer function.
+  // HACK: id allocators are shared across all nested envs of one
+  // lowering so a captured outer binding keeps its IdentifierId when
+  // seen by an inner function.
   ids: { nextIdentifierId: number; nextInstructionId: number; nextSyntheticName: number };
   bindings: Map<string, Place>;
   parent: LoweringEnvironment | null;
@@ -143,11 +123,6 @@ const setBinding = (env: LoweringEnvironment, name: string, place: Place): void
   env.bindings.set(name, place);
 };
 
-// HACK: mirrors the destructured-prop scaffolding in noPropCallbackInEffect.
-// For `function Foo({ value, onChange }) {}`, we walk the ObjectPattern
-// and create one Identifier per shorthand entry, tagging callbacks
-// (`/^on[A-Z]/`) so prop-callback rules don't have to re-derive that
-// every time.
 const PROP_CALLBACK_NAME_PATTERN = /^on[A-Z]/;
 
 const isPropCallbackName = (name: string): boolean => PROP_CALLBACK_NAME_PATTERN.test(name);
@@ -194,13 +169,8 @@ const collectFunctionParams = (
   return places;
 };
 
-// HACK: `f(...args)` in ESTree wraps the spread in `SpreadElement`,
-// which `lowerExpression` doesn't recognize and would otherwise drop
-// silently. Lower the spread's `argument` directly so the spread
-// source still appears as a Place in the call's args list — the
-// "spread"-ness is lost (we have no SpreadPlace shape in v1) but
-// the operand identity is preserved, which is what validators that
-// trace setState propagation through arguments care about.
+// HACK: SpreadElement (`f(...args)`) isn't a real expression node in
+// ESTree, so unwrap to its `argument` to keep operand identity.
 const lowerCallArguments = (
   env: LoweringEnvironment,
   argumentNodes: Array<EsTreeNode> | undefined,
@@ -333,10 +303,8 @@ const lowerExpression = (env: LoweringEnvironment, node: EsTreeNode | null | und
       loc,
       node,
     );
-    // HACK: a nested FunctionDeclaration introduces a binding in its
-    // enclosing scope (`function helper() {}` ≈ `var helper = ...`).
-    // Tie the name to the FunctionExpression's lvalue so subsequent
-    // `helper()` calls resolve to it, not LoadGlobal.
+    // HACK: nested `function helper() {}` declares `helper` in the
+    // enclosing scope; bind the name so call sites resolve to it.
     if (node.type === "FunctionDeclaration" && node.id?.type === "Identifier") {
       setBinding(env, node.id.name, place);
     }
@@ -435,14 +403,10 @@ const lowerExpression = (env: LoweringEnvironment, node: EsTreeNode | null | und
   return emitTemporary(env, { kind: "Unsupported", reason: node.type }, loc, node);
 };
 
-// HACK: collects which Places the lowered inner function reads from
-// any enclosing scope. Because env IDs are shared (root env's
-// allocator is reused by every child), we just look at every
-// LoadLocal whose source identifier was bound OUTSIDE the inner
-// function's own params / destructured props / lvalues — that's a
-// capture. The outer env isn't needed here because the captured
-// Place is the SAME Place object (identifier id and originNode)
-// that exists in the outer scope.
+// HACK: a "capture" is any LoadLocal whose source Identifier wasn't
+// declared inside the inner function (params, destructured props,
+// instruction lvalues). Shared id allocator means the captured Place
+// is already === the outer Place.
 const collectCapturedPlaces = (innerFn: HIRFunction): Array<Place> => {
   const captured: Array<Place> = [];
   const seenIds = new Set<IdentifierId>();
@@ -546,10 +510,8 @@ const lowerStatement = (env: LoweringEnvironment, node: EsTreeNode | null | unde
   }
 
   // HACK: control-flow statements collapse into the surrounding block
-  // for v1 (no real CFG terminals yet). We still recurse into their
-  // bodies so a `useEffect` inside `try { ... }` or `for (...) { ... }`
-  // gets lowered — silently dropping these statements would have
-  // missed real validator findings.
+  // for v1 (no CFG terminals modeled). We recurse into their bodies
+  // so hooks/effects inside them still get lowered.
   if (node.type === "ForStatement" || node.type === "WhileStatement") {
     if (node.test) lowerExpression(env, node.test);
     if (node.update) lowerExpression(env, node.update);
@@ -593,13 +555,8 @@ const lowerStatement = (env: LoweringEnvironment, node: EsTreeNode | null | unde
   if (node.type === "TryStatement") {
     lowerStatement(env, node.block);
     if (node.handler) {
-      // HACK: bind the catch param (`} catch (error) {`) so references
-      // to it inside the handler body resolve as a LoadLocal instead
-      // of falling through to LoadGlobal "error". The param's scope
-      // technically ends with the handler block; we keep it visible
-      // in the surrounding env (no real block scoping in v1) — that's
-      // a minor over-scope which doesn't affect the validators we
-      // currently run.
+      // HACK: bind catch param so references inside the handler body
+      // resolve as LoadLocal, not LoadGlobal.
       if (node.handler.param?.type === "Identifier") {
         const paramName = node.handler.param.name;
         const paramIdentifier = createIdentifier(env, paramName, "local");
@@ -632,7 +589,6 @@ const lowerStatement = (env: LoweringEnvironment, node: EsTreeNode | null | unde
     node.type === "ArrowFunctionExpression" ||
     node.type === "FunctionExpression"
   ) {
-    // Treat as an expression so it gets a FunctionExpression instruction.
     lowerExpression(env, node);
     return;
   }

packages/react-doctor/src/plugin/hir/print.ts

@@ -6,18 +6,8 @@ import { validateNoSetStateInEffects } from "./validators/validate-no-set-state-
 import { validateNoDerivedComputationsInEffects } from "./validators/validate-no-derived-computations-in-effects.js";
 import type { HIRFunction, Place } from "./types.js";
 
-// HACK: bridges HIR validators to the existing oxlint Rule contract.
-//
-// Each rule's `create()` returns visitors that detect a component-shaped
-// function and forward to a shared `getOrLowerHir(node)`. The lowered
-// HIR is cached in a WeakMap keyed by the original component AST node
-// so multiple HIR rules running on the same source file lower it once.
-//
-// Diagnostics carry a `Place` — its `originNode` points back at the
-// ESTree node the place was lowered from. We use that as the report
-// node, falling back to the component declaration when the place was
-// synthetic (no underlying source node).
-
+// HACK: per-component HIR cache so multiple HIR rules visiting the
+// same file lower it once.
 const lowerCache = new WeakMap<EsTreeNode, HIRFunction>();
 
 const getOrLowerHir = (componentNode: EsTreeNode): HIRFunction => {
@@ -67,7 +57,6 @@ export const hirNoDerivedComputationsInEffects: Rule = {
       const fn = getOrLowerHir(functionNode);
       const findings = validateNoDerivedComputationsInEffects(fn);
       for (const finding of findings) {
-        if (finding.reason !== "all-deps-captured") continue;
         const reportNode = resolveReportNode(finding.effectCallPlace, functionNode);
         context.report({
           node: reportNode,