diff --git a/packages/react-doctor/package.json b/packages/react-doctor/package.json index 7c55161..159b5ef 100644 --- a/packages/react-doctor/package.json +++ b/packages/react-doctor/package.json @@ -75,6 +75,7 @@ }, "devDependencies": { "@types/prompts": "^2.4.9", + "@typescript-eslint/parser": "^8.59.2", "eslint-plugin-react-hooks": "^7.1.1" }, "peerDependencies": { diff --git a/packages/react-doctor/src/oxlint-config.ts b/packages/react-doctor/src/oxlint-config.ts index 384aa2f..c7fa1c2 100644 --- a/packages/react-doctor/src/oxlint-config.ts +++ b/packages/react-doctor/src/oxlint-config.ts @@ -251,6 +251,14 @@ export const GLOBAL_REACT_DOCTOR_RULES: Record = { "react-doctor/advanced-event-handler-refs": "warn", "react-doctor/effect-needs-cleanup": "error", + // 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", + "react-doctor/no-giant-component": "warn", "react-doctor/no-render-in-render": "warn", "react-doctor/no-many-boolean-props": "warn", diff --git a/packages/react-doctor/src/plugin/hir/index.ts b/packages/react-doctor/src/plugin/hir/index.ts new file mode 100644 index 0000000..a1d5618 --- /dev/null +++ b/packages/react-doctor/src/plugin/hir/index.ts @@ -0,0 +1,4 @@ +export * from "./types.js"; +export { lowerFunction } from "./lower.js"; +export { inferTypes } from "./infer-types.js"; +export { hirNoSetStateInEffect, hirNoDerivedComputationsInEffects } from "./runner.js"; diff --git a/packages/react-doctor/src/plugin/hir/infer-types.ts b/packages/react-doctor/src/plugin/hir/infer-types.ts new file mode 100644 index 0000000..3241d46 --- /dev/null +++ b/packages/react-doctor/src/plugin/hir/infer-types.ts @@ -0,0 +1,108 @@ +import type { HIRFunction, Identifier, ReactType } from "./types.js"; + +// 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 = { + useState: "UseStateHook", + useReducer: "UseStateHook", + useEffect: "UseEffectHook", + useLayoutEffect: "UseLayoutEffectHook", + useInsertionEffect: "UseLayoutEffectHook", + useRef: "UseRefHook", + useCallback: "UseCallbackHook", + useMemo: "UseMemoHook", + useContext: "UseContextHook", + useEffectEvent: "UseEffectEventHook", +}; + +const setIdentifierType = (identifier: Identifier, type: ReactType): void => { + if (identifier.type === "Unknown" || identifier.type === "Function") { + identifier.type = type; + } +}; + +export const inferTypes = (fn: HIRFunction): void => { + for (const block of fn.body.blocks.values()) { + for (const instr of block.instructions) { + const lvalue = instr.lvalue; + + switch (instr.value.kind) { + case "LoadGlobal": { + const reactType = REACT_HOOK_NAME_TO_TYPE[instr.value.name]; + if (reactType && lvalue) { + setIdentifierType(lvalue.identifier, reactType); + setIdentifierType(instr.value.place.identifier, reactType); + } + break; + } + case "LoadLocal": { + if (lvalue) { + setIdentifierType(lvalue.identifier, instr.value.place.identifier.type); + } + break; + } + case "StoreLocal": { + setIdentifierType(instr.value.lvalue.identifier, instr.value.value.identifier.type); + if (lvalue) { + setIdentifierType(lvalue.identifier, instr.value.value.identifier.type); + } + break; + } + case "CallExpression": { + const calleeType = instr.value.callee.identifier.type; + if (lvalue) { + if (calleeType === "UseStateHook") { + setIdentifierType(lvalue.identifier, "StateTuple"); + } else if (calleeType === "UseRefHook") { + setIdentifierType(lvalue.identifier, "RefValue"); + } else if (calleeType === "UseEffectEventHook") { + setIdentifierType(lvalue.identifier, "EffectEvent"); + } else if (calleeType === "UseCallbackHook") { + setIdentifierType(lvalue.identifier, "Function"); + } else if (calleeType === "UseMemoHook") { + setIdentifierType(lvalue.identifier, "Object"); + } else if (calleeType === "UseContextHook") { + setIdentifierType(lvalue.identifier, "Object"); + } + } + break; + } + case "PropertyLoad": { + const objectType = instr.value.object.identifier.type; + 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"); + } + } + if ( + objectType === "RefValue" && + !instr.value.computed && + instr.value.property === "current" && + lvalue + ) { + setIdentifierType(lvalue.identifier, "RefCurrent"); + } + break; + } + case "MethodCall": + case "ArrayExpression": + case "ObjectExpression": + case "Literal": + case "Identifier": + case "BinaryExpression": + case "LogicalExpression": + case "UnaryExpression": + case "ConditionalExpression": + case "FunctionExpression": + case "JSXExpression": + case "Unsupported": + break; + } + } + } +}; diff --git a/packages/react-doctor/src/plugin/hir/lower.ts b/packages/react-doctor/src/plugin/hir/lower.ts new file mode 100644 index 0000000..df829f4 --- /dev/null +++ b/packages/react-doctor/src/plugin/hir/lower.ts @@ -0,0 +1,642 @@ +import type { EsTreeNode } from "../types.js"; +import type { + BasicBlock, + BlockId, + EffectKind, + HIR, + HIRFunction, + Identifier, + IdentifierId, + Instruction, + InstructionId, + InstructionValue, + Place, + ReactType, + SourceLocation, + Terminal, +} from "./types.js"; + +interface LoweringEnvironment { + // 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; + parent: LoweringEnvironment | null; + instructions: Array; +} + +const ZERO_LOCATION: SourceLocation = { + start: { line: 0, column: 0 }, + end: { line: 0, column: 0 }, +}; + +const getLocation = (node: EsTreeNode | null | undefined): SourceLocation => { + if (!node?.loc) return ZERO_LOCATION; + return { + start: { line: node.loc.start.line, column: node.loc.start.column }, + end: { line: node.loc.end.line, column: node.loc.end.column }, + }; +}; + +const createRootEnvironment = (): LoweringEnvironment => ({ + ids: { nextIdentifierId: 0, nextInstructionId: 0, nextSyntheticName: 0 }, + bindings: new Map(), + parent: null, + instructions: [], +}); + +const createChildEnvironment = (parent: LoweringEnvironment): LoweringEnvironment => ({ + ids: parent.ids, + bindings: new Map(), + parent, + instructions: [], +}); + +const allocateIdentifierId = (env: LoweringEnvironment): IdentifierId => env.ids.nextIdentifierId++; + +const allocateInstructionId = (env: LoweringEnvironment): InstructionId => + env.ids.nextInstructionId++; + +const allocateSyntheticName = (env: LoweringEnvironment): string => + `$tmp${env.ids.nextSyntheticName++}`; + +const createIdentifier = ( + env: LoweringEnvironment, + name: string | null, + origin: Identifier["origin"], + type: ReactType = "Unknown", +): Identifier => ({ + id: allocateIdentifierId(env), + name, + type, + origin, +}); + +const createPlace = ( + identifier: Identifier, + loc: SourceLocation, + effect: EffectKind = "Read", + originNode: EsTreeNode | null = null, +): Place => ({ identifier, effect, loc, originNode }); + +const emitInstruction = ( + env: LoweringEnvironment, + lvalue: Place | null, + value: InstructionValue, + loc: SourceLocation, +): void => { + env.instructions.push({ + id: allocateInstructionId(env), + lvalue, + value, + loc, + }); +}; + +const emitTemporary = ( + env: LoweringEnvironment, + value: InstructionValue, + loc: SourceLocation, + originNode: EsTreeNode | null = null, +): Place => { + const identifier = createIdentifier(env, allocateSyntheticName(env), "synthetic"); + const place = createPlace(identifier, loc, "Read", originNode); + emitInstruction(env, place, value, loc); + return place; +}; + +const lookupBinding = (env: LoweringEnvironment, name: string): Place | null => { + // Walk parent chain so a closure can resolve a name to the outer + // function's Place (sharing IdentifierIds), the way the compiler's + // `findContextIdentifiers` exposes captured bindings. + let cursor: LoweringEnvironment | null = env; + while (cursor) { + const place = cursor.bindings.get(name); + if (place) return place; + cursor = cursor.parent; + } + return null; +}; + +const setBinding = (env: LoweringEnvironment, name: string, place: Place): void => { + env.bindings.set(name, place); +}; + +const PROP_CALLBACK_NAME_PATTERN = /^on[A-Z]/; + +const isPropCallbackName = (name: string): boolean => PROP_CALLBACK_NAME_PATTERN.test(name); + +const collectDestructuredProps = ( + env: LoweringEnvironment, + pattern: EsTreeNode | undefined, + destructuredProps: Map, +): void => { + if (!pattern || pattern.type !== "ObjectPattern") return; + for (const property of pattern.properties ?? []) { + if (property.type !== "Property") continue; + if (property.value?.type !== "Identifier") continue; + const propName: string = property.value.name; + const reactType: ReactType = isPropCallbackName(propName) ? "PropCallback" : "Unknown"; + const identifier = createIdentifier(env, propName, "destructured-prop", reactType); + const place = createPlace(identifier, getLocation(property), "Read", property.value); + destructuredProps.set(propName, place); + setBinding(env, propName, place); + } +}; + +const collectFunctionParams = ( + env: LoweringEnvironment, + paramNodes: Array | undefined, + destructuredProps: Map, +): Array => { + const places: Array = []; + for (const param of paramNodes ?? []) { + if (param.type === "Identifier") { + const identifier = createIdentifier(env, param.name, "param"); + const place = createPlace(identifier, getLocation(param), "Read", param); + places.push(place); + setBinding(env, param.name, place); + continue; + } + if (param.type === "ObjectPattern") { + const identifier = createIdentifier(env, "props", "param"); + const place = createPlace(identifier, getLocation(param), "Read", param); + places.push(place); + collectDestructuredProps(env, param, destructuredProps); + } + } + return places; +}; + +// 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 | undefined, +): Array => + (argumentNodes ?? []).map((argumentNode: EsTreeNode) => { + if (argumentNode.type === "SpreadElement") { + return lowerExpression(env, argumentNode.argument); + } + return lowerExpression(env, argumentNode); + }); + +const lowerExpression = (env: LoweringEnvironment, node: EsTreeNode | null | undefined): Place => { + if (!node) { + return emitTemporary(env, { kind: "Unsupported", reason: "missing-node" }, ZERO_LOCATION, null); + } + const loc = getLocation(node); + + if (node.type === "Identifier") { + const existing = lookupBinding(env, node.name); + if (existing) { + return emitTemporary(env, { kind: "LoadLocal", place: existing }, loc, node); + } + const identifier = createIdentifier(env, node.name, "module"); + const place = createPlace(identifier, loc, "Read", node); + return emitTemporary(env, { kind: "LoadGlobal", name: node.name, place }, loc, node); + } + + if (node.type === "Literal") { + return emitTemporary( + env, + { kind: "Literal", value: node.value, raw: String(node.raw ?? "") }, + loc, + node, + ); + } + + if (node.type === "TemplateLiteral") { + const expressions: Array = []; + for (const expression of node.expressions ?? []) { + expressions.push(lowerExpression(env, expression)); + } + return emitTemporary( + env, + { + kind: "Literal", + value: null, + raw: `\`...${expressions.length} interpolations...\``, + }, + loc, + node, + ); + } + + if (node.type === "MemberExpression") { + const objectPlace = lowerExpression(env, node.object); + if (!node.computed && node.property?.type === "Identifier") { + return emitTemporary( + env, + { + kind: "PropertyLoad", + object: objectPlace, + property: node.property.name, + computed: false, + }, + loc, + node, + ); + } + if (node.computed) { + const propertyPlace = lowerExpression(env, node.property); + return emitTemporary( + env, + { + kind: "PropertyLoad", + object: objectPlace, + property: propertyPlace.identifier.name ?? `[computed:${propertyPlace.identifier.id}]`, + computed: true, + }, + loc, + node, + ); + } + return emitTemporary(env, { kind: "Unsupported", reason: "member-expression" }, loc, node); + } + + if (node.type === "CallExpression") { + if (node.callee?.type === "MemberExpression" && !node.callee.computed) { + const receiverPlace = lowerExpression(env, node.callee.object); + const propertyName = + node.callee.property?.type === "Identifier" ? node.callee.property.name : ""; + const propertyPlace = createPlace( + createIdentifier(env, propertyName, "synthetic"), + getLocation(node.callee.property), + "Read", + node.callee.property, + ); + const args = lowerCallArguments(env, node.arguments); + return emitTemporary( + env, + { + kind: "MethodCall", + receiver: receiverPlace, + property: propertyPlace, + propertyName, + args, + }, + loc, + node, + ); + } + const calleePlace = lowerExpression(env, node.callee); + const args = lowerCallArguments(env, node.arguments); + return emitTemporary(env, { kind: "CallExpression", callee: calleePlace, args }, loc, node); + } + + if ( + node.type === "ArrowFunctionExpression" || + node.type === "FunctionExpression" || + node.type === "FunctionDeclaration" + ) { + const lowered = lowerFunctionInEnv(node, env); + const capturedPlaces: Array = collectCapturedPlaces(lowered); + const place = emitTemporary( + env, + { + kind: "FunctionExpression", + loweredFunc: lowered, + capturedPlaces, + }, + loc, + node, + ); + // 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); + } + return place; + } + + if (node.type === "ArrayExpression") { + const elements: Array = (node.elements ?? []).map((element: EsTreeNode | null) => + element ? lowerExpression(env, element) : null, + ); + return emitTemporary(env, { kind: "ArrayExpression", elements }, loc, node); + } + + if (node.type === "ObjectExpression") { + const properties: Array<{ key: string | null; value: Place; spread: boolean }> = []; + for (const property of node.properties ?? []) { + if (property.type === "SpreadElement") { + properties.push({ + key: null, + value: lowerExpression(env, property.argument), + spread: true, + }); + continue; + } + if (property.type === "Property") { + const keyName = + property.key?.type === "Identifier" + ? property.key.name + : property.key?.type === "Literal" + ? String(property.key.value) + : null; + properties.push({ + key: keyName, + value: lowerExpression(env, property.value), + spread: false, + }); + } + } + return emitTemporary(env, { kind: "ObjectExpression", properties }, loc, node); + } + + if (node.type === "BinaryExpression") { + const left = lowerExpression(env, node.left); + const right = lowerExpression(env, node.right); + return emitTemporary( + env, + { kind: "BinaryExpression", left, operator: node.operator, right }, + loc, + node, + ); + } + + if (node.type === "LogicalExpression") { + const left = lowerExpression(env, node.left); + const right = lowerExpression(env, node.right); + return emitTemporary( + env, + { kind: "LogicalExpression", left, operator: node.operator, right }, + loc, + node, + ); + } + + if (node.type === "UnaryExpression") { + const argument = lowerExpression(env, node.argument); + return emitTemporary( + env, + { kind: "UnaryExpression", operator: node.operator, argument }, + loc, + node, + ); + } + + if (node.type === "ConditionalExpression") { + const test = lowerExpression(env, node.test); + const consequent = lowerExpression(env, node.consequent); + const alternate = lowerExpression(env, node.alternate); + return emitTemporary( + env, + { kind: "ConditionalExpression", test, consequent, alternate }, + loc, + node, + ); + } + + if (node.type === "JSXElement" || node.type === "JSXFragment") { + const placeholder = emitTemporary( + env, + { kind: "Literal", value: "", raw: "" }, + loc, + node, + ); + return emitTemporary(env, { kind: "JSXExpression", jsxPlaceholder: placeholder }, loc, node); + } + + return emitTemporary(env, { kind: "Unsupported", reason: node.type }, loc, node); +}; + +// 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 => { + const captured: Array = []; + const seenIds = new Set(); + const innerOwnIds = new Set(); + for (const param of innerFn.params) innerOwnIds.add(param.identifier.id); + for (const place of innerFn.destructuredProps.values()) { + innerOwnIds.add(place.identifier.id); + } + for (const block of innerFn.body.blocks.values()) { + for (const instr of block.instructions) { + if (instr.lvalue) innerOwnIds.add(instr.lvalue.identifier.id); + } + } + for (const block of innerFn.body.blocks.values()) { + for (const instr of block.instructions) { + if (instr.value.kind !== "LoadLocal") continue; + const place = instr.value.place; + if (innerOwnIds.has(place.identifier.id)) continue; + if (seenIds.has(place.identifier.id)) continue; + seenIds.add(place.identifier.id); + captured.push(place); + } + } + return captured; +}; + +const lowerVariableDeclaration = (env: LoweringEnvironment, node: EsTreeNode): void => { + for (const declarator of node.declarations ?? []) { + if (!declarator) continue; + const initPlace = declarator.init ? lowerExpression(env, declarator.init) : null; + + if (declarator.id?.type === "Identifier") { + const name = declarator.id.name; + const identifier = createIdentifier(env, name, "local"); + const lvalue = createPlace(identifier, getLocation(declarator.id), "Read", declarator.id); + if (initPlace) { + emitInstruction( + env, + lvalue, + { kind: "StoreLocal", lvalue, value: initPlace }, + getLocation(declarator), + ); + } + setBinding(env, name, lvalue); + continue; + } + + if (declarator.id?.type === "ArrayPattern" && initPlace) { + const elements = declarator.id.elements ?? []; + for (let elementIndex = 0; elementIndex < elements.length; elementIndex++) { + const element = elements[elementIndex]; + if (!element || element.type !== "Identifier") continue; + const name: string = element.name; + const identifier = createIdentifier(env, name, "local"); + const lvalue = createPlace(identifier, getLocation(element), "Read", element); + emitInstruction( + env, + lvalue, + { + kind: "PropertyLoad", + object: initPlace, + property: String(elementIndex), + computed: true, + }, + getLocation(element), + ); + setBinding(env, name, lvalue); + } + } + } +}; + +const lowerStatement = (env: LoweringEnvironment, node: EsTreeNode | null | undefined): void => { + if (!node) return; + + if (node.type === "VariableDeclaration") { + lowerVariableDeclaration(env, node); + return; + } + + if (node.type === "ExpressionStatement") { + lowerExpression(env, node.expression); + return; + } + + if (node.type === "ReturnStatement") { + if (node.argument) lowerExpression(env, node.argument); + return; + } + + if (node.type === "BlockStatement") { + for (const child of node.body ?? []) lowerStatement(env, child); + return; + } + + if (node.type === "IfStatement") { + lowerExpression(env, node.test); + lowerStatement(env, node.consequent); + if (node.alternate) lowerStatement(env, node.alternate); + return; + } + + // HACK: control-flow statements collapse into the surrounding block + // 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); + if (node.init) { + if (node.init.type === "VariableDeclaration") { + lowerVariableDeclaration(env, node.init); + } else { + lowerExpression(env, node.init); + } + } + lowerStatement(env, node.body); + return; + } + + if (node.type === "DoWhileStatement") { + lowerStatement(env, node.body); + if (node.test) lowerExpression(env, node.test); + return; + } + + if (node.type === "ForOfStatement" || node.type === "ForInStatement") { + if (node.left?.type === "VariableDeclaration") { + lowerVariableDeclaration(env, node.left); + } + if (node.right) lowerExpression(env, node.right); + lowerStatement(env, node.body); + return; + } + + if (node.type === "SwitchStatement") { + if (node.discriminant) lowerExpression(env, node.discriminant); + for (const switchCase of node.cases ?? []) { + if (switchCase.test) lowerExpression(env, switchCase.test); + for (const consequentStatement of switchCase.consequent ?? []) { + lowerStatement(env, consequentStatement); + } + } + return; + } + + if (node.type === "TryStatement") { + lowerStatement(env, node.block); + if (node.handler) { + // 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"); + const paramPlace = createPlace( + paramIdentifier, + getLocation(node.handler.param), + "Read", + node.handler.param, + ); + setBinding(env, paramName, paramPlace); + } + if (node.handler.body) lowerStatement(env, node.handler.body); + } + if (node.finalizer) lowerStatement(env, node.finalizer); + return; + } + + if (node.type === "ThrowStatement") { + if (node.argument) lowerExpression(env, node.argument); + return; + } + + if (node.type === "LabeledStatement") { + lowerStatement(env, node.body); + return; + } + + if ( + node.type === "FunctionDeclaration" || + node.type === "ArrowFunctionExpression" || + node.type === "FunctionExpression" + ) { + lowerExpression(env, node); + return; + } +}; + +const lowerFunctionInEnv = ( + functionNode: EsTreeNode, + parentEnv: LoweringEnvironment | null, +): HIRFunction => { + const env = parentEnv ? createChildEnvironment(parentEnv) : createRootEnvironment(); + const destructuredProps = new Map(); + const params = collectFunctionParams(env, functionNode.params, destructuredProps); + + const body = functionNode.body; + if (body) { + if (body.type === "BlockStatement") { + for (const statement of body.body ?? []) lowerStatement(env, statement); + } else { + lowerExpression(env, body); + } + } + + const entryBlockId: BlockId = "bb0"; + const terminal: Terminal = { + kind: "return", + value: null, + id: allocateInstructionId(env), + loc: getLocation(functionNode), + }; + const entryBlock: BasicBlock = { + id: entryBlockId, + instructions: env.instructions, + terminal, + preds: new Set(), + }; + + const blocks = new Map(); + blocks.set(entryBlockId, entryBlock); + + const hir: HIR = { entry: entryBlockId, blocks }; + + return { + name: functionNode.id?.name ?? null, + params, + destructuredProps, + body: hir, + }; +}; + +export const lowerFunction = (functionNode: EsTreeNode): HIRFunction => + lowerFunctionInEnv(functionNode, null); diff --git a/packages/react-doctor/src/plugin/hir/runner.ts b/packages/react-doctor/src/plugin/hir/runner.ts new file mode 100644 index 0000000..970c714 --- /dev/null +++ b/packages/react-doctor/src/plugin/hir/runner.ts @@ -0,0 +1,81 @@ +import type { EsTreeNode, Rule, RuleContext } from "../types.js"; +import { isComponentAssignment, isUppercaseName } from "../helpers.js"; +import { lowerFunction } from "./lower.js"; +import { inferTypes } from "./infer-types.js"; +import { validateNoSetStateInEffects } from "./validators/validate-no-set-state-in-effect.js"; +import { validateNoDerivedComputationsInEffects } from "./validators/validate-no-derived-computations-in-effects.js"; +import type { HIRFunction, Place } from "./types.js"; + +// HACK: per-component HIR cache so multiple HIR rules visiting the +// same file lower it once. +const lowerCache = new WeakMap(); + +const getOrLowerHir = (componentNode: EsTreeNode): HIRFunction => { + const cached = lowerCache.get(componentNode); + if (cached) return cached; + const fn = lowerFunction(componentNode); + inferTypes(fn); + lowerCache.set(componentNode, fn); + return fn; +}; + +const resolveReportNode = (place: Place, fallbackNode: EsTreeNode): EsTreeNode => + place.originNode ?? fallbackNode; + +export const hirNoSetStateInEffect: Rule = { + create: (context: RuleContext) => { + const visitComponent = (functionNode: EsTreeNode): void => { + const fn = getOrLowerHir(functionNode); + const findings = validateNoSetStateInEffects(fn); + for (const finding of findings) { + const reportNode = resolveReportNode(finding.callSitePlace, functionNode); + const setterName = finding.setterPlace.identifier.name ?? ""; + context.report({ + node: reportNode, + message: `Calling \`${setterName}()\` directly within an effect can trigger cascading renders. Effects should synchronize React with external systems; either move the setState into the event that caused it, or fold the value into a render-time derivation. (HIR-validated)`, + }); + } + }; + + return { + FunctionDeclaration(node: EsTreeNode) { + if (!node.id?.name || !isUppercaseName(node.id.name)) return; + visitComponent(node); + }, + VariableDeclarator(node: EsTreeNode) { + if (!isComponentAssignment(node)) return; + if (!node.init) return; + visitComponent(node.init); + }, + }; + }, +}; + +export const hirNoDerivedComputationsInEffects: Rule = { + create: (context: RuleContext) => { + const visitComponent = (functionNode: EsTreeNode): void => { + const fn = getOrLowerHir(functionNode); + const findings = validateNoDerivedComputationsInEffects(fn); + for (const finding of findings) { + const reportNode = resolveReportNode(finding.effectCallPlace, functionNode); + context.report({ + node: reportNode, + message: + "Effect derives state purely from its dependencies — compute the value during render (or wrap in `useMemo` if expensive). (HIR-validated)", + }); + } + }; + + return { + FunctionDeclaration(node: EsTreeNode) { + if (!node.id?.name || !isUppercaseName(node.id.name)) return; + visitComponent(node); + }, + VariableDeclarator(node: EsTreeNode) { + if (!isComponentAssignment(node)) return; + if (!node.init) return; + visitComponent(node.init); + }, + }; + }, +}; diff --git a/packages/react-doctor/src/plugin/hir/types.ts b/packages/react-doctor/src/plugin/hir/types.ts new file mode 100644 index 0000000..e62b96c --- /dev/null +++ b/packages/react-doctor/src/plugin/hir/types.ts @@ -0,0 +1,117 @@ +import type { EsTreeNode } from "../types.js"; + +export type IdentifierId = number; +export type InstructionId = number; +export type BlockId = string; + +export type ReactType = + | "Unknown" + | "Primitive" + | "Function" + | "Object" + | "UseStateHook" + | "UseEffectHook" + | "UseLayoutEffectHook" + | "UseRefHook" + | "UseCallbackHook" + | "UseMemoHook" + | "UseContextHook" + | "UseEffectEventHook" + // HACK: tuple returned by useState/useReducer. Kept distinct from + // `Object` so the indexed-PropertyLoad branch (StateValue / + // StateSetter) doesn't fire on useMemo destructures. + | "StateTuple" + | "StateValue" + | "StateSetter" + | "RefValue" + | "RefCurrent" + | "EffectEvent" + | "PropCallback"; + +export type EffectKind = "Read" | "Unknown"; + +export interface SourceLocation { + start: { line: number; column: number }; + end: { line: number; column: number }; +} + +export interface Identifier { + id: IdentifierId; + name: string | null; + type: ReactType; + origin: "module" | "destructured-prop" | "param" | "local" | "synthetic"; +} + +export interface Place { + identifier: Identifier; + effect: EffectKind; + loc: SourceLocation; + originNode: EsTreeNode | null; +} + +export type InstructionValue = + | { kind: "LoadLocal"; place: Place } + | { kind: "LoadGlobal"; name: string; place: Place } + | { kind: "StoreLocal"; lvalue: Place; value: Place } + | { kind: "CallExpression"; callee: Place; args: Array } + | { + kind: "MethodCall"; + receiver: Place; + property: Place; + propertyName: string; + args: Array; + } + | { kind: "PropertyLoad"; object: Place; property: string; computed: boolean } + | { kind: "FunctionExpression"; loweredFunc: HIRFunction; capturedPlaces: Array } + | { kind: "ArrayExpression"; elements: Array } + | { + kind: "ObjectExpression"; + properties: Array<{ key: string | null; value: Place; spread: boolean }>; + } + | { kind: "Literal"; value: unknown; raw: string } + | { kind: "Identifier"; place: Place } + | { kind: "BinaryExpression"; left: Place; operator: string; right: Place } + | { kind: "LogicalExpression"; left: Place; operator: string; right: Place } + | { kind: "UnaryExpression"; operator: string; argument: Place } + | { kind: "ConditionalExpression"; test: Place; consequent: Place; alternate: Place } + | { kind: "JSXExpression"; jsxPlaceholder: Place } + | { kind: "Unsupported"; reason: string }; + +export type Terminal = + | { kind: "return"; value: Place | null; id: InstructionId; loc: SourceLocation } + | { kind: "unsupported"; reason: string; id: InstructionId; loc: SourceLocation }; + +export interface Instruction { + id: InstructionId; + lvalue: Place | null; + value: InstructionValue; + loc: SourceLocation; +} + +export interface BasicBlock { + id: BlockId; + instructions: Array; + terminal: Terminal; + preds: Set; +} + +export interface HIRFunction { + name: string | null; + params: Array; + destructuredProps: Map; + body: HIR; +} + +export interface HIR { + entry: BlockId; + blocks: Map; +} + +export const isSetStateType = (identifier: Identifier): boolean => + identifier.type === "StateSetter"; + +export const isUseEffectHookType = (identifier: Identifier): boolean => + identifier.type === "UseEffectHook" || identifier.type === "UseLayoutEffectHook"; + +export const isUseEffectEventType = (identifier: Identifier): boolean => + identifier.type === "UseEffectEventHook"; diff --git a/packages/react-doctor/src/plugin/hir/validators/validate-no-derived-computations-in-effects.ts b/packages/react-doctor/src/plugin/hir/validators/validate-no-derived-computations-in-effects.ts new file mode 100644 index 0000000..d81b433 --- /dev/null +++ b/packages/react-doctor/src/plugin/hir/validators/validate-no-derived-computations-in-effects.ts @@ -0,0 +1,109 @@ +import { + type HIRFunction, + type IdentifierId, + type Place, + isSetStateType, + isUseEffectHookType, +} from "../types.js"; + +// HACK: port of `babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts`. +// Reports when a useEffect's inner function captures only deps + state +// setters AND has at least one intermediate local binding (the +// AST-walker `noDerivedStateEffect` already covers the simple case). + +export interface DerivedComputationInEffectFinding { + effectCallPlace: Place; +} + +interface EffectFunctionEntry { + func: HIRFunction; + captures: Array; +} + +export const validateNoDerivedComputationsInEffects = ( + fn: HIRFunction, +): Array => { + const findings: Array = []; + + const candidateDependencies = new Map>(); + const effectFunctions = new Map(); + const localAliases = new Map(); + + for (const block of fn.body.blocks.values()) { + for (const instr of block.instructions) { + const lvalueId = instr.lvalue?.identifier.id; + + if (instr.value.kind === "LoadLocal" && lvalueId !== undefined) { + localAliases.set(lvalueId, instr.value.place.identifier.id); + continue; + } + + if (instr.value.kind === "ArrayExpression" && lvalueId !== undefined) { + const elementPlaces: Array = []; + for (const element of instr.value.elements) { + if (element) elementPlaces.push(element); + } + candidateDependencies.set(lvalueId, elementPlaces); + continue; + } + + if (instr.value.kind === "FunctionExpression" && lvalueId !== undefined) { + effectFunctions.set(lvalueId, { + func: instr.value.loweredFunc, + captures: instr.value.capturedPlaces, + }); + continue; + } + + if (instr.value.kind === "CallExpression" || instr.value.kind === "MethodCall") { + const callee = + instr.value.kind === "CallExpression" ? instr.value.callee : instr.value.property; + if (!isUseEffectHookType(callee.identifier)) continue; + if (instr.value.args.length !== 2) continue; + const callbackArgument = instr.value.args[0]; + const depsArgument = instr.value.args[1]; + const effectEntry = effectFunctions.get(callbackArgument.identifier.id); + const depsList = candidateDependencies.get(depsArgument.identifier.id); + if (!effectEntry || !depsList || depsList.length === 0) continue; + + const dependencyIds = depsList.map( + (dep) => localAliases.get(dep.identifier.id) ?? dep.identifier.id, + ); + + const finding = validateEffect(effectEntry, dependencyIds, callee); + if (finding) findings.push(finding); + } + } + } + + return findings; +}; + +const validateEffect = ( + effectEntry: EffectFunctionEntry, + effectDependencyIds: Array, + effectCallPlace: Place, +): DerivedComputationInEffectFinding | null => { + const capturedIds = new Set(); + for (const capture of effectEntry.captures) { + capturedIds.add(capture.identifier.id); + if (isSetStateType(capture.identifier) || effectDependencyIds.includes(capture.identifier.id)) { + continue; + } + return null; + } + + for (const dependencyId of effectDependencyIds) { + if (!capturedIds.has(dependencyId)) return null; + } + + // HACK: defer to AST-walker `noDerivedStateEffect` on the simple + // single-setter-call shape; HIR rule's unique value is the + // multi-statement-with-locals shape, which produces a StoreLocal. + for (const block of effectEntry.func.body.blocks.values()) { + for (const instr of block.instructions) { + if (instr.value.kind === "StoreLocal") return { effectCallPlace }; + } + } + return null; +}; diff --git a/packages/react-doctor/src/plugin/hir/validators/validate-no-set-state-in-effect.ts b/packages/react-doctor/src/plugin/hir/validators/validate-no-set-state-in-effect.ts new file mode 100644 index 0000000..291fd0b --- /dev/null +++ b/packages/react-doctor/src/plugin/hir/validators/validate-no-set-state-in-effect.ts @@ -0,0 +1,192 @@ +import { + type HIRFunction, + type IdentifierId, + type Place, + isSetStateType, + isUseEffectEventType, + isUseEffectHookType, +} from "../types.js"; + +// HACK: port of `babel-plugin-react-compiler/src/Validation/ValidateNoSetStateInEffects.ts`. +// Tracks IdentifierIds that resolve back to a state setter through +// LoadLocal / StoreLocal / FunctionExpression / useEffectEvent, then +// reports each useEffect whose callback id is in that set. + +export interface SetStateInEffectFinding { + setterPlace: Place; + callSitePlace: Place; + effectCallPlace: Place; +} + +interface InnerSetStateMatch { + setterPlace: Place; + callSitePlace: Place; +} + +export const validateNoSetStateInEffects = (fn: HIRFunction): Array => { + const findings: Array = []; + + const setStateBindings = new Map(); + // Parallel map: id → call-site Place inside the effect body. Used + // so the diagnostic anchors at `setX(...)` not the setter decl. + const innerCallSites = new Map(); + + for (const block of fn.body.blocks.values()) { + for (const instr of block.instructions) { + switch (instr.value.kind) { + case "LoadLocal": { + const sourceId = instr.value.place.identifier.id; + if (isSetStateType(instr.value.place.identifier) || setStateBindings.has(sourceId)) { + const originPlace = setStateBindings.get(sourceId) ?? instr.value.place; + if (instr.lvalue) { + setStateBindings.set(instr.lvalue.identifier.id, originPlace); + const callSite = innerCallSites.get(sourceId); + if (callSite) innerCallSites.set(instr.lvalue.identifier.id, callSite); + } + } + break; + } + + case "StoreLocal": { + const sourceId = instr.value.value.identifier.id; + if (isSetStateType(instr.value.value.identifier) || setStateBindings.has(sourceId)) { + const originPlace = setStateBindings.get(sourceId) ?? instr.value.value; + setStateBindings.set(instr.value.lvalue.identifier.id, originPlace); + if (instr.lvalue) setStateBindings.set(instr.lvalue.identifier.id, originPlace); + const callSite = innerCallSites.get(sourceId); + if (callSite) { + innerCallSites.set(instr.value.lvalue.identifier.id, callSite); + if (instr.lvalue) innerCallSites.set(instr.lvalue.identifier.id, callSite); + } + } + break; + } + + case "FunctionExpression": { + const innerMatch = findTopLevelSetStateCall(instr.value.loweredFunc, setStateBindings); + if (innerMatch && instr.lvalue) { + setStateBindings.set(instr.lvalue.identifier.id, innerMatch.setterPlace); + innerCallSites.set(instr.lvalue.identifier.id, innerMatch.callSitePlace); + } + break; + } + + case "CallExpression": { + const callee = instr.value.callee; + + if (isUseEffectEventType(callee.identifier)) { + const firstArgument = instr.value.args[0]; + if (firstArgument) { + const originPlace = setStateBindings.get(firstArgument.identifier.id); + if (originPlace && instr.lvalue) { + setStateBindings.set(instr.lvalue.identifier.id, originPlace); + const callSite = innerCallSites.get(firstArgument.identifier.id); + if (callSite) innerCallSites.set(instr.lvalue.identifier.id, callSite); + } + } + break; + } + + if (isUseEffectHookType(callee.identifier)) { + const callbackArgument = instr.value.args[0]; + if (callbackArgument) { + const setterOrigin = setStateBindings.get(callbackArgument.identifier.id); + if (setterOrigin) { + const callSite = + innerCallSites.get(callbackArgument.identifier.id) ?? callbackArgument; + findings.push({ + setterPlace: setterOrigin, + callSitePlace: callSite, + effectCallPlace: callee, + }); + } + } + } + break; + } + + case "MethodCall": + case "PropertyLoad": + case "ArrayExpression": + case "ObjectExpression": + case "Literal": + case "LoadGlobal": + case "Identifier": + case "BinaryExpression": + case "LogicalExpression": + case "UnaryExpression": + case "ConditionalExpression": + case "JSXExpression": + case "Unsupported": + break; + } + } + } + + return findings; +}; + +// Walk an inner HIRFunction and return the setter + call-site if it +// synchronously calls a setState at the top level. Mirrors upstream +// `getSetStateCall` without the ref-derived exception. +const findTopLevelSetStateCall = ( + fn: HIRFunction, + outerSetStateBindings: Map, +): InnerSetStateMatch | null => { + const innerSetStateBindings = new Map(outerSetStateBindings); + + for (const block of fn.body.blocks.values()) { + for (const instr of block.instructions) { + switch (instr.value.kind) { + case "LoadLocal": { + const sourceId = instr.value.place.identifier.id; + if (isSetStateType(instr.value.place.identifier) || innerSetStateBindings.has(sourceId)) { + const originPlace = innerSetStateBindings.get(sourceId) ?? instr.value.place; + if (instr.lvalue) { + innerSetStateBindings.set(instr.lvalue.identifier.id, originPlace); + } + } + break; + } + case "StoreLocal": { + const sourceId = instr.value.value.identifier.id; + if (isSetStateType(instr.value.value.identifier) || innerSetStateBindings.has(sourceId)) { + const originPlace = innerSetStateBindings.get(sourceId) ?? instr.value.value; + innerSetStateBindings.set(instr.value.lvalue.identifier.id, originPlace); + if (instr.lvalue) { + innerSetStateBindings.set(instr.lvalue.identifier.id, originPlace); + } + } + break; + } + case "CallExpression": { + const calleeIdentifier = instr.value.callee.identifier; + if (isSetStateType(calleeIdentifier) || innerSetStateBindings.has(calleeIdentifier.id)) { + const setterPlace = + innerSetStateBindings.get(calleeIdentifier.id) ?? instr.value.callee; + const callSitePlace = instr.lvalue ?? instr.value.callee; + return { setterPlace, callSitePlace }; + } + break; + } + case "FunctionExpression": + case "MethodCall": + case "PropertyLoad": + case "ArrayExpression": + case "ObjectExpression": + case "Literal": + case "LoadGlobal": + case "Identifier": + case "BinaryExpression": + case "LogicalExpression": + case "UnaryExpression": + case "ConditionalExpression": + case "JSXExpression": + case "Unsupported": + break; + } + } + } + + return null; +}; diff --git a/packages/react-doctor/src/plugin/index.ts b/packages/react-doctor/src/plugin/index.ts index 7649599..9c2011f 100644 --- a/packages/react-doctor/src/plugin/index.ts +++ b/packages/react-doctor/src/plugin/index.ts @@ -198,6 +198,7 @@ import { rerenderLazyStateInit, rerenderStateOnlyInHandlers, } from "./rules/state-and-effects.js"; +import { hirNoDerivedComputationsInEffects, hirNoSetStateInEffect } from "./hir/index.js"; import { noDocumentStartViewTransition, noFlushSync } from "./rules/view-transitions.js"; import type { RulePlugin } from "./types.js"; @@ -227,6 +228,9 @@ const plugin: RulePlugin = { "advanced-event-handler-refs": advancedEventHandlerRefs, "effect-needs-cleanup": effectNeedsCleanup, + "hir-no-set-state-in-effect": hirNoSetStateInEffect, + "hir-no-derived-computations-in-effects": hirNoDerivedComputationsInEffects, + "no-generic-handler-names": noGenericHandlerNames, "no-giant-component": noGiantComponent, "no-many-boolean-props": noManyBooleanProps, diff --git a/packages/react-doctor/src/utils/run-oxlint.ts b/packages/react-doctor/src/utils/run-oxlint.ts index 23444e1..889ce18 100644 --- a/packages/react-doctor/src/utils/run-oxlint.ts +++ b/packages/react-doctor/src/utils/run-oxlint.ts @@ -65,6 +65,8 @@ const RULE_CATEGORY_MAP: Record = { "react-doctor/rerender-defer-reads-hook": "Performance", "react-doctor/advanced-event-handler-refs": "Performance", "react-doctor/effect-needs-cleanup": "State & Effects", + "react-doctor/hir-no-set-state-in-effect": "State & Effects", + "react-doctor/hir-no-derived-computations-in-effects": "State & Effects", "react-doctor/no-generic-handler-names": "Architecture", "react-doctor/no-giant-component": "Architecture", @@ -324,6 +326,10 @@ const RULE_HELP_MAP: Record = { "Store the handler in a ref and have the listener read `handlerRef.current()` — the subscription stays put while the latest handler is always called", "effect-needs-cleanup": "Return a cleanup function that releases the subscription / timer: `return () => target.removeEventListener(name, handler)` for listeners, `return () => clearInterval(id)` / `clearTimeout(id)` for timers, or `return unsubscribe` if the subscribe call already returned one", + "hir-no-set-state-in-effect": + "Move the setState into the event that caused the change, or compute the value during render. setState inside an effect body triggers cascading renders. (Detected via HIR data flow analysis — the setState is propagated through assignments and useEffectEvent wrappers.)", + "hir-no-derived-computations-in-effects": + "The effect captures only its declared dependencies (and setStates) — that means it's deriving state. Compute the value during render; if the derivation is expensive, wrap it in `useMemo`. (Detected via HIR data flow analysis.)", "async-defer-await": "Move the `await` after the synchronous early-return guard so the skip path stays fast", "async-await-in-loop": diff --git a/packages/react-doctor/tests/regressions/hir-port.test.ts b/packages/react-doctor/tests/regressions/hir-port.test.ts new file mode 100644 index 0000000..9117057 --- /dev/null +++ b/packages/react-doctor/tests/regressions/hir-port.test.ts @@ -0,0 +1,200 @@ +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 { runOxlint } from "../../src/utils/run-oxlint.js"; +import { setupReactProject } from "./_helpers.js"; + +const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "rd-hir-port-")); + +afterAll(() => { + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +const setupHirProject = (caseId: string, files: Record): string => + setupReactProject(tempRoot, caseId, { files }); + +const collectRuleHits = async ( + projectDir: string, + ruleId: string, +): Promise> => { + const diagnostics = await runOxlint({ + rootDirectory: projectDir, + hasTypeScript: true, + framework: "unknown", + hasReactCompiler: false, + hasTanStackQuery: false, + }); + return diagnostics + .filter((diagnostic) => diagnostic.rule === ruleId) + .map((diagnostic) => ({ + filePath: diagnostic.filePath, + message: diagnostic.message, + line: diagnostic.line, + })); +}; + +describe("hir-no-set-state-in-effect — HIR-validated rule", () => { + it("flags a useEffect that calls a setState directly", async () => { + const projectDir = setupHirProject("hir-no-set-state-direct", { + "src/Counter.tsx": `import { useEffect, useState } from "react"; + +export const Counter = () => { + const [count, setCount] = useState(0); + useEffect(() => { + setCount(1); + }, []); + return {count}; +}; +`, + }); + + const hits = await collectRuleHits(projectDir, "hir-no-set-state-in-effect"); + expect(hits.length).toBeGreaterThanOrEqual(1); + expect(hits[0].message).toContain("setCount"); + expect(hits[0].message).toContain("HIR-validated"); + }); + + it("flags a useEffect that calls a setState via an aliased const (SSA propagation)", async () => { + const projectDir = setupHirProject("hir-no-set-state-aliased", { + "src/Aliased.tsx": `import { useEffect, useState } from "react"; + +export const Aliased = () => { + const [count, setCount] = useState(0); + const writer = setCount; + useEffect(() => { + writer(2); + }, []); + return {count}; +}; +`, + }); + + const hits = await collectRuleHits(projectDir, "hir-no-set-state-in-effect"); + expect(hits.length).toBeGreaterThanOrEqual(1); + }); + + it("reports at the setState call site (not the component declaration line)", async () => { + const projectDir = setupHirProject("hir-no-set-state-call-site-loc", { + "src/Counter.tsx": `import { useEffect, useState } from "react"; + +export const Counter = () => { + const [count, setCount] = useState(0); + useEffect(() => { + setCount(1); + }, []); + return {count}; +}; +`, + }); + + const hits = await collectRuleHits(projectDir, "hir-no-set-state-in-effect"); + expect(hits.length).toBeGreaterThanOrEqual(1); + expect(hits[0].line).toBe(6); + }); + + it("does NOT flag setState inside a sub-handler (subscription callback) — that's legit", async () => { + const projectDir = setupHirProject("hir-no-set-state-subscribe", { + "src/Sync.tsx": `import { useEffect, useState } from "react"; + +declare const subscribe: (handler: () => void) => () => void; + +export const Sync = () => { + const [tick, setTick] = useState(0); + useEffect(() => { + return subscribe(() => setTick(tick + 1)); + }, [tick]); + return {tick}; +}; +`, + }); + + const hits = await collectRuleHits(projectDir, "hir-no-set-state-in-effect"); + expect(hits).toHaveLength(0); + }); + + it("does NOT misclassify `[a, b] = useMemo(...)` as a state destructure", async () => { + const projectDir = setupHirProject("hir-no-set-state-usememo-tuple", { + "src/Memo.tsx": `import { useEffect, useMemo } from "react"; + +declare const compute: () => [number, () => void]; + +export const Memo = () => { + const [n, runIt] = useMemo(() => compute(), []); + useEffect(() => { + runIt(); + }, [n]); + return {n}; +}; +`, + }); + + const hits = await collectRuleHits(projectDir, "hir-no-set-state-in-effect"); + expect(hits).toHaveLength(0); + }); +}); + +describe("hir-no-derived-computations-in-effects — HIR-validated rule", () => { + it("defers to noDerivedStateEffect on the single-setter-call shape", async () => { + const projectDir = setupHirProject("hir-derived-fullname-defer", { + "src/Form.tsx": `import { useEffect, useState } from "react"; + +export const Form = () => { + const [firstName] = useState("Taylor"); + const [lastName] = useState("Swift"); + const [fullName, setFullName] = useState(""); + useEffect(() => { + setFullName(firstName + " " + lastName); + }, [firstName, lastName]); + return

{fullName}

; +}; +`, + }); + + const hits = await collectRuleHits(projectDir, "hir-no-derived-computations-in-effects"); + expect(hits).toHaveLength(0); + }); + + it("flags the article §1 example when the derivation is bound to a local first (HIR-unique)", async () => { + const projectDir = setupHirProject("hir-derived-with-local", { + "src/Form.tsx": `import { useEffect, useState } from "react"; + +export const Form = () => { + const [firstName] = useState("Taylor"); + const [lastName] = useState("Swift"); + const [fullName, setFullName] = useState(""); + useEffect(() => { + const combined = firstName + " " + lastName; + setFullName(combined); + }, [firstName, lastName]); + return

{fullName}

; +}; +`, + }); + + const hits = await collectRuleHits(projectDir, "hir-no-derived-computations-in-effects"); + expect(hits.length).toBeGreaterThanOrEqual(1); + expect(hits[0].message).toContain("HIR-validated"); + }); + + it("does NOT flag a useEffect that reads a value NOT in deps (genuine sync)", async () => { + const projectDir = setupHirProject("hir-derived-not-pure", { + "src/Logger.tsx": `import { useEffect, useState } from "react"; + +declare const log: (message: string) => void; + +export const Logger = ({ name }: { name: string }) => { + const [count, setCount] = useState(0); + useEffect(() => { + log(\`\${name}: \${count}\`); + }, [count]); + return {count}; +}; +`, + }); + + const hits = await collectRuleHits(projectDir, "hir-no-derived-computations-in-effects"); + expect(hits).toHaveLength(0); + }); +}); diff --git a/packages/react-doctor/tests/regressions/hir-unit.test.ts b/packages/react-doctor/tests/regressions/hir-unit.test.ts new file mode 100644 index 0000000..11d45ed --- /dev/null +++ b/packages/react-doctor/tests/regressions/hir-unit.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from "vite-plus/test"; +import { parse } from "@typescript-eslint/parser"; +import type { EsTreeNode } from "../../src/plugin/types.js"; +import { lowerFunction } from "../../src/plugin/hir/lower.js"; +import { inferTypes } from "../../src/plugin/hir/infer-types.js"; +import { validateNoSetStateInEffects } from "../../src/plugin/hir/validators/validate-no-set-state-in-effect.js"; +import { validateNoDerivedComputationsInEffects } from "../../src/plugin/hir/validators/validate-no-derived-computations-in-effects.js"; + +const lowerFromSource = (source: string) => { + const ast = parse(source, { loc: true, range: true, jsx: true }); + // HACK: parser returns a Program node; .body[0] is a + // FunctionDeclaration whose dynamic shape conforms to EsTreeNode. + const componentNode = ast.body[0] as unknown as EsTreeNode; + const fn = lowerFunction(componentNode); + inferTypes(fn); + return fn; +}; + +describe("HIR — direct lower + validate", () => { + it("lowers a Counter and emits a setState-in-effect finding", () => { + const fn = lowerFromSource(` +function Counter() { + const [count, setCount] = useState(0); + useEffect(() => { + setCount(1); + }, []); + return null; +} +`); + const hits = validateNoSetStateInEffects(fn); + expect(hits.length).toBeGreaterThanOrEqual(1); + expect(hits[0].setterPlace.identifier.name).toBe("setCount"); + expect(hits[0].setterPlace.identifier.type).toBe("StateSetter"); + }); + + it("emits a derived-state finding only when the body has an intermediate local", () => { + const fn = lowerFromSource(` +function Form() { + const [firstName] = useState("Taylor"); + const [lastName] = useState("Swift"); + const [fullName, setFullName] = useState(""); + useEffect(() => { + const combined = firstName + " " + lastName; + setFullName(combined); + }, [firstName, lastName]); + return null; +} +`); + const hits = validateNoDerivedComputationsInEffects(fn); + expect(hits.length).toBeGreaterThanOrEqual(1); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2c69c5c..eb37535 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -60,6 +60,9 @@ importers: '@types/prompts': specifier: ^2.4.9 version: 2.4.9 + '@typescript-eslint/parser': + specifier: ^8.59.2 + version: 8.59.2(eslint@9.39.2(jiti@2.6.1))(typescript@5.9.3) eslint-plugin-react-hooks: specifier: ^7.1.1 version: 7.1.1(eslint@9.39.2(jiti@2.6.1)) @@ -1672,6 +1675,43 @@ packages: '@types/react@19.2.14': resolution: {integrity: sha512-ilcTH/UniCkMdtexkoCN0bI7pMcJDvmQFPvuPvmEaYA/NSfFTAgdUSLAoVjaRJm7+6PvcM+q1zYOwS4wTYMF9w==} + '@typescript-eslint/parser@8.59.2': + resolution: {integrity: sha512-plR3pp6D+SSUn1HM7xvSkx12/DhoHInI2YF35KAcVFNZvlC0gtrWqx7Qq1oH2Ssgi0vlFRCTbP+DZc7B9+TtsQ==} + engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} + peerDependencies: + eslint: ^8.57.0 || ^9.0.0 || ^10.0.0 + typescript: '>=4.8.4 <6.1.0' + + '@typescript-eslint/project-service@8.59.2': + resolution: {integrity: sha512-+2hqvEkeyf/0FBor67duF0Ll7Ot8jyKzDQOSrxazF/danillRq2DwR9dLptsXpoZQqxE1UisSmoZewrlPas9Vw==} + engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} + peerDependencies: + typescript: '>=4.8.4 <6.1.0' + + '@typescript-eslint/scope-manager@8.59.2': + resolution: {integrity: sha512-JzfyEpEtOU89CcFSwyNS3mu4MLvLSXqnmX05+aKBDM+TdR5jzcGOEBwxwGNxrEQ7p/z6kK2WyioCGBf2zZBnvg==} + engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} + + '@typescript-eslint/tsconfig-utils@8.59.2': + resolution: {integrity: sha512-BKK4alN7oi4C/zv4VqHQ+uRU+lTa6JGIZ7s1juw7b3RHo9OfKB+bKX3u0iVZetdsUCBBkSbdWbarJbmN0fTeSw==} + engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} + peerDependencies: + typescript: '>=4.8.4 <6.1.0' + + '@typescript-eslint/types@8.59.2': + resolution: {integrity: sha512-e82GVOE8Ps3E++Egvb6Y3Dw0S10u8NkQ9KXmtRhCWJJ8kDhOJTvtMAWnFL16kB1583goCWXsr0NieKCZMs2/0Q==} + engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} + + '@typescript-eslint/typescript-estree@8.59.2': + resolution: {integrity: sha512-o0XPGNwcWw+FIwStOWn+BwBuEmL6QXP0rsvAFg7ET1dey1Nr6Wb1ac8p5HEsK0ygO/6mUxlk+YWQD9xcb/nnXg==} + engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} + peerDependencies: + typescript: '>=4.8.4 <6.1.0' + + '@typescript-eslint/visitor-keys@8.59.2': + resolution: {integrity: sha512-NwjLUnGy8/Zfx23fl50tRC8rYaYnM52xNRYFAXvmiil9yh1+K6aRVQMnzW6gQB/1DLgWt977lYQn7C+wtgXZiA==} + engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} + '@vercel/analytics@2.0.1': resolution: {integrity: sha512-MTQG6V9qQrt1tsDeF+2Uoo5aPjqbVPys1xvnIftXSJYG2SrwXRHnqEvVoYID7BTruDz4lCd2Z7rM1BdkUehk2g==} peerDependencies: @@ -1894,6 +1934,10 @@ packages: balanced-match@1.0.2: resolution: {integrity: sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==} + balanced-match@4.0.4: + resolution: {integrity: sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==} + engines: {node: 18 || 20 || >=22} + baseline-browser-mapping@2.9.19: resolution: {integrity: sha512-ipDqC8FrAl/76p2SSWKSI+H9tFwm7vYqXQrItCuiVPt26Km0jS+NzSsBWAaBusvSbQcfJG+JitdMm+wZAgTYqg==} hasBin: true @@ -1905,6 +1949,10 @@ packages: brace-expansion@1.1.13: resolution: {integrity: sha512-9ZLprWS6EENmhEOpjCYW2c8VkmOvckIJZfkr7rBW6dObmfgJ/L1GpSYW5Hpo9lDz4D1+n0Ckz8rU7FwHDQiG/w==} + brace-expansion@5.0.5: + resolution: {integrity: sha512-VZznLgtwhn+Mact9tfiwx64fA9erHH/MCXEUfB/0bX/6Fz6ny5EGTXYltMocqg4xFAQZtnO3DHWWXi8RiuN7cQ==} + engines: {node: 18 || 20 || >=22} + braces@3.0.3: resolution: {integrity: sha512-yQbXgO/OSZVD2IsiLlro+7Hf6Q18EJrKSEsdoMzKePKXct3gvD8oLcOQdIzGupr5Fj+EDe8gO/lxc1BzfMpxvA==} engines: {node: '>=8'} @@ -2042,6 +2090,10 @@ packages: resolution: {integrity: sha512-Uhdk5sfqcee/9H/rCOJikYz67o0a2Tw2hGRPOG2Y1R2dg7brRe1uG0yaNQDHu+TO/uQPF/5eCapvYSmHUjt7JQ==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} + eslint-visitor-keys@5.0.1: + resolution: {integrity: sha512-tD40eHxA35h0PEIZNeIjkHoDR4YjjJp34biM0mDvplBe//mB+IHCqHDGV7pxF+7MklTvighcCPPZC7ynWyjdTA==} + engines: {node: ^20.19.0 || ^22.13.0 || >=24} + eslint@9.39.2: resolution: {integrity: sha512-LEyamqS7W5HB3ujJyvi0HQK/dtVINZvd5mAAp9eT5S/ujByGjiZLCzPcHVzuXbpJDJF/cxwHlfceVUDZ2lnSTw==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} @@ -2411,6 +2463,10 @@ packages: resolution: {integrity: sha512-VP79XUPxV2CigYP3jWwAUFSku2aKqBH7uTAapFWCBqutsbmDo96KY5o8uh6U+/YSIn5OxJnXp73beVkpqMIGhA==} engines: {node: '>=18'} + minimatch@10.2.5: + resolution: {integrity: sha512-MULkVLfKGYDFYejP07QOurDLLQpcjk7Fw+7jXS2R2czRQzR56yHRveU5NDJEOviH+hETZKSkIk5c+T23GjFUMg==} + engines: {node: 18 || 20 || >=22} + minimatch@3.1.5: resolution: {integrity: sha512-VgjWUsnnT6n+NUk6eZq77zeFdpW2LWDzP6zFGrCbHXiYNul5Dzqk2HHQ5uFH2DNW5Xbp8+jVzaeNt94ssEEl4w==} @@ -2809,6 +2865,12 @@ packages: resolution: {integrity: sha512-sf4i37nQ2LBx4m3wB74y+ubopq6W/dIzXg0FDGjsYnZHVa1Da8FH853wlL2gtUhg+xJXjfk3kUZS3BRoQeoQBQ==} engines: {node: '>=6'} + ts-api-utils@2.5.0: + resolution: {integrity: sha512-OJ/ibxhPlqrMM0UiNHJ/0CKQkoKF243/AEmplt3qpRgkW8VG7IfOS41h7V8TjITqdByHzrjcS/2si+y4lIh8NA==} + engines: {node: '>=18.12'} + peerDependencies: + typescript: '>=4.8.4' + tslib@2.8.1: resolution: {integrity: sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==} @@ -4093,6 +4155,58 @@ snapshots: dependencies: csstype: 3.2.3 + '@typescript-eslint/parser@8.59.2(eslint@9.39.2(jiti@2.6.1))(typescript@5.9.3)': + dependencies: + '@typescript-eslint/scope-manager': 8.59.2 + '@typescript-eslint/types': 8.59.2 + '@typescript-eslint/typescript-estree': 8.59.2(typescript@5.9.3) + '@typescript-eslint/visitor-keys': 8.59.2 + debug: 4.4.3 + eslint: 9.39.2(jiti@2.6.1) + typescript: 5.9.3 + transitivePeerDependencies: + - supports-color + + '@typescript-eslint/project-service@8.59.2(typescript@5.9.3)': + dependencies: + '@typescript-eslint/tsconfig-utils': 8.59.2(typescript@5.9.3) + '@typescript-eslint/types': 8.59.2 + debug: 4.4.3 + typescript: 5.9.3 + transitivePeerDependencies: + - supports-color + + '@typescript-eslint/scope-manager@8.59.2': + dependencies: + '@typescript-eslint/types': 8.59.2 + '@typescript-eslint/visitor-keys': 8.59.2 + + '@typescript-eslint/tsconfig-utils@8.59.2(typescript@5.9.3)': + dependencies: + typescript: 5.9.3 + + '@typescript-eslint/types@8.59.2': {} + + '@typescript-eslint/typescript-estree@8.59.2(typescript@5.9.3)': + dependencies: + '@typescript-eslint/project-service': 8.59.2(typescript@5.9.3) + '@typescript-eslint/tsconfig-utils': 8.59.2(typescript@5.9.3) + '@typescript-eslint/types': 8.59.2 + '@typescript-eslint/visitor-keys': 8.59.2 + debug: 4.4.3 + minimatch: 10.2.5 + semver: 7.7.4 + tinyglobby: 0.2.16 + ts-api-utils: 2.5.0(typescript@5.9.3) + typescript: 5.9.3 + transitivePeerDependencies: + - supports-color + + '@typescript-eslint/visitor-keys@8.59.2': + dependencies: + '@typescript-eslint/types': 8.59.2 + eslint-visitor-keys: 5.0.1 + '@vercel/analytics@2.0.1(next@16.2.4(react-dom@19.2.5(react@19.2.5))(react@19.2.5))(react@19.2.5)': optionalDependencies: next: 16.2.4(react-dom@19.2.5(react@19.2.5))(react@19.2.5) @@ -4220,6 +4334,8 @@ snapshots: balanced-match@1.0.2: {} + balanced-match@4.0.4: {} + baseline-browser-mapping@2.9.19: {} better-path-resolve@1.0.0: @@ -4231,6 +4347,10 @@ snapshots: balanced-match: 1.0.2 concat-map: 0.0.1 + brace-expansion@5.0.5: + dependencies: + balanced-match: 4.0.4 + braces@3.0.3: dependencies: fill-range: 7.1.1 @@ -4371,6 +4491,8 @@ snapshots: eslint-visitor-keys@4.2.1: {} + eslint-visitor-keys@5.0.1: {} + eslint@9.39.2(jiti@2.6.1): dependencies: '@eslint-community/eslint-utils': 4.9.1(eslint@9.39.2(jiti@2.6.1)) @@ -4720,6 +4842,10 @@ snapshots: mimic-function@5.0.1: {} + minimatch@10.2.5: + dependencies: + brace-expansion: 5.0.5 + minimatch@3.1.5: dependencies: brace-expansion: 1.1.13 @@ -5204,6 +5330,10 @@ snapshots: totalist@3.0.1: {} + ts-api-utils@2.5.0(typescript@5.9.3): + dependencies: + typescript: 5.9.3 + tslib@2.8.1: {} turbo@2.9.7: