diff --git a/packages/react-doctor/src/plugin/rules/state-and-effects.ts b/packages/react-doctor/src/plugin/rules/state-and-effects.ts index 1c4105ba..b65463ec 100644 --- a/packages/react-doctor/src/plugin/rules/state-and-effects.ts +++ b/packages/react-doctor/src/plugin/rules/state-and-effects.ts @@ -170,9 +170,17 @@ export const noDerivedUseState: Rule = { // are not modeled here (a known limitation — pre-existing). const componentPropStack: Array> = []; + // HACK: empty stack frames are barriers — pushed when entering a + // non-component FunctionDeclaration / ArrowFunctionExpression so + // identifiers inside the helper don't resolve against an outer + // component's props (a closed-over `value` is NOT a prop of the + // helper). Stop the walk at the first empty frame so the lookup + // honors the barrier the visitor pushed. const isPropName = (name: string): boolean => { for (let stackIndex = componentPropStack.length - 1; stackIndex >= 0; stackIndex--) { - if (componentPropStack[stackIndex].has(name)) return true; + const frame = componentPropStack[stackIndex]; + if (frame.size === 0) return false; + if (frame.has(name)) return true; } return false; }; @@ -405,9 +413,16 @@ export const noPropCallbackInEffect: Rule = { componentPropParamStack.push(propNames); }; + // HACK: empty stack frames are barriers — pushed when entering a + // non-component FunctionDeclaration / ArrowFunctionExpression so + // identifiers inside the helper don't resolve against an outer + // component's props. Stop the walk at the first empty frame so + // the lookup honors the barrier the visitor pushed. const isPropName = (name: string): boolean => { for (let stackIndex = componentPropParamStack.length - 1; stackIndex >= 0; stackIndex--) { - if (componentPropParamStack[stackIndex].has(name)) return true; + const frame = componentPropParamStack[stackIndex]; + if (frame.size === 0) return false; + if (frame.has(name)) return true; } return false; }; diff --git a/packages/react-doctor/tests/regressions/prop-stack-barrier.test.ts b/packages/react-doctor/tests/regressions/prop-stack-barrier.test.ts new file mode 100644 index 00000000..144ca83d --- /dev/null +++ b/packages/react-doctor/tests/regressions/prop-stack-barrier.test.ts @@ -0,0 +1,140 @@ +/** + * Regression tests for the "empty-frame-as-barrier" semantic in the + * shared prop-stack scaffolding used by `no-prop-callback-in-effect` + * and `no-derived-useState`. The visitor pushes an empty `Set` when + * entering a non-component FunctionDeclaration / ArrowFunctionExpression + * so identifiers inside the helper don't resolve against an outer + * component's props (a closed-over `value` is NOT a prop of the + * helper). + * + * The original `isPropName` walked the entire stack without honoring + * the barrier, so a useState / useEffect inside a nested helper would + * pick up the outer component's prop names and produce false positives. + */ + +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-prop-stack-barrier-")); + +afterAll(() => { + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +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, + })); +}; + +describe("no-derived-useState — empty-frame barrier", () => { + it("flags `useState(value)` when `value` is a real prop of the current component", async () => { + const projectDir = setupReactProject(tempRoot, "no-derived-usestate-real-prop", { + files: { + "src/Field.tsx": `import { useState } from "react"; + +export const Field = ({ value }: { value: string }) => { + const [draft, setDraft] = useState(value); + return setDraft(event.target.value)} />; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "no-derived-useState"); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("value"); + }); + + it("does NOT flag `useState(value)` when `value` is closed over from an outer component", async () => { + // The inner FunctionDeclaration pushes an empty barrier frame; the + // barrier-aware isPropName must stop the walk there and not see + // Outer's prop set. + const projectDir = setupReactProject(tempRoot, "no-derived-usestate-nested-helper", { + files: { + "src/Outer.tsx": `import { useState } from "react"; + +export const Outer = ({ value }: { value: string }) => { + function inner() { + const [draft, setDraft] = useState(value); + void draft; + void setDraft; + } + inner(); + return {value}; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "no-derived-useState"); + expect(hits).toHaveLength(0); + }); +}); + +describe("no-prop-callback-in-effect — empty-frame barrier", () => { + it("flags the canonical `useEffect(() => onChange(state), [state, onChange])` shape", async () => { + const projectDir = setupReactProject(tempRoot, "no-prop-callback-real-prop", { + files: { + "src/Toggle.tsx": `import { useEffect, useState } from "react"; + +export const Toggle = ({ onChange }: { onChange: (next: boolean) => void }) => { + const [isOn, setIsOn] = useState(false); + useEffect(() => { + onChange(isOn); + }, [isOn, onChange]); + return ; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "no-prop-callback-in-effect"); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("onChange"); + }); + + it("does NOT flag `useEffect(() => onChange(state), [state, onChange])` inside a nested helper", async () => { + // Same nested-helper shape — the outer component's `onChange` prop + // must not leak into the helper's effect-callback check. + const projectDir = setupReactProject(tempRoot, "no-prop-callback-nested-helper", { + files: { + "src/Outer.tsx": `import { useEffect, useState } from "react"; + +export const Outer = ({ onChange }: { onChange: (next: boolean) => void }) => { + function inner() { + const [isOn, setIsOn] = useState(false); + useEffect(() => { + onChange(isOn); + }, [isOn, onChange]); + void setIsOn; + } + inner(); + return ; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "no-prop-callback-in-effect"); + expect(hits).toHaveLength(0); + }); +});