diff --git a/packages/react-doctor/src/plugin/helpers.ts b/packages/react-doctor/src/plugin/helpers.ts index bed7d4a..bfde38e 100644 --- a/packages/react-doctor/src/plugin/helpers.ts +++ b/packages/react-doctor/src/plugin/helpers.ts @@ -55,12 +55,11 @@ export const getCallbackStatements = (callback: EsTreeNode): EsTreeNode[] => { export const countSetStateCalls = (node: EsTreeNode): number => { let setStateCallCount = 0; walkAst(node, (child) => { - if ( - child.type === "CallExpression" && - child.callee?.type === "Identifier" && - isSetterIdentifier(child.callee.name) - ) { - setStateCallCount++; + if (child.type === "CallExpression") { + const name = getCalleeName(child.callee); + if (name && isSetterIdentifier(name)) { + setStateCallCount++; + } } }); return setStateCallCount; @@ -100,10 +99,25 @@ export const isComponentAssignment = (node: EsTreeNode): boolean => Boolean(node.init) && (node.init.type === "ArrowFunctionExpression" || node.init.type === "FunctionExpression"); -export const isHookCall = (node: EsTreeNode, hookName: string | Set): boolean => - node.type === "CallExpression" && - node.callee?.type === "Identifier" && - (typeof hookName === "string" ? node.callee.name === hookName : hookName.has(node.callee.name)); +export const getCalleeName = (node: EsTreeNode): string | null => { + if (!node) return null; + if (node.type === "Identifier") return node.name; + if ( + node.type === "MemberExpression" && + node.object?.type === "Identifier" && + node.property?.type === "Identifier" + ) { + return node.property.name; + } + return null; +}; + +export const isHookCall = (node: EsTreeNode, hookName: string | Set): boolean => { + if (node.type !== "CallExpression") return false; + const name = getCalleeName(node.callee); + if (!name) return false; + return typeof hookName === "string" ? name === hookName : hookName.has(name); +}; export const hasDirective = (programNode: EsTreeNode, directive: string): boolean => Boolean( diff --git a/packages/react-doctor/src/plugin/rules/performance.ts b/packages/react-doctor/src/plugin/rules/performance.ts index 2f015c2..50db5d2 100644 --- a/packages/react-doctor/src/plugin/rules/performance.ts +++ b/packages/react-doctor/src/plugin/rules/performance.ts @@ -9,6 +9,7 @@ import { SETTER_PATTERN, } from "../constants.js"; import { + getCalleeName, getEffectCallback, isComponentAssignment, isHookCall, @@ -412,12 +413,12 @@ export const renderingHydrationNoFlicker: Rule = { if (!bodyStatements || bodyStatements.length !== 1) return; const soleStatement = bodyStatements[0]; - if ( + const setterName = soleStatement?.type === "ExpressionStatement" && - soleStatement.expression?.type === "CallExpression" && - soleStatement.expression.callee?.type === "Identifier" && - SETTER_PATTERN.test(soleStatement.expression.callee.name) - ) { + soleStatement.expression?.type === "CallExpression" + ? getCalleeName(soleStatement.expression.callee) + : null; + if (setterName && SETTER_PATTERN.test(setterName)) { context.report({ node, message: 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 7c40f54..f3fd674 100644 --- a/packages/react-doctor/src/plugin/rules/state-and-effects.ts +++ b/packages/react-doctor/src/plugin/rules/state-and-effects.ts @@ -9,6 +9,7 @@ import { containsFetchCall, countSetStateCalls, extractDestructuredPropNames, + getCalleeName, getCallbackStatements, getEffectCallback, isComponentAssignment, @@ -40,13 +41,15 @@ export const noDerivedStateEffect: Rule = { const statements = getCallbackStatements(callback); if (statements.length === 0) return; - const containsOnlySetStateCalls = statements.every( - (statement: EsTreeNode) => - statement.type === "ExpressionStatement" && - statement.expression?.type === "CallExpression" && - statement.expression.callee?.type === "Identifier" && - isSetterIdentifier(statement.expression.callee.name), - ); + const containsOnlySetStateCalls = statements.every((statement: EsTreeNode) => { + if ( + statement.type !== "ExpressionStatement" || + statement.expression?.type !== "CallExpression" + ) + return false; + const name = getCalleeName(statement.expression.callee); + return name !== null && isSetterIdentifier(name); + }); if (!containsOnlySetStateCalls) return; let allArgumentsDeriveFromDeps = true; @@ -248,7 +251,8 @@ export const rerenderLazyStateInit: Rule = { export const rerenderFunctionalSetstate: Rule = { create: (context: RuleContext) => ({ CallExpression(node: EsTreeNode) { - if (node.callee?.type !== "Identifier" || !isSetterIdentifier(node.callee.name)) return; + const calleeName = getCalleeName(node.callee); + if (!calleeName || !isSetterIdentifier(calleeName)) return; if (!node.arguments?.length) return; const argument = node.arguments[0]; @@ -259,7 +263,7 @@ export const rerenderFunctionalSetstate: Rule = { ) { context.report({ node, - message: `${node.callee.name}(${argument.left.name} ${argument.operator} ...) — use functional update to avoid stale closures`, + message: `${calleeName}(${argument.left.name} ${argument.operator} ...) — use functional update to avoid stale closures`, }); } }, diff --git a/packages/react-doctor/tests/helpers.test.ts b/packages/react-doctor/tests/helpers.test.ts new file mode 100644 index 0000000..d5f1ee7 --- /dev/null +++ b/packages/react-doctor/tests/helpers.test.ts @@ -0,0 +1,153 @@ +import { describe, expect, it } from "vitest"; +import { getCalleeName, isHookCall, countSetStateCalls } from "../src/plugin/helpers.js"; + +describe("getCalleeName", () => { + it("returns name from Identifier node", () => { + expect(getCalleeName({ type: "Identifier", name: "useEffect" })).toBe("useEffect"); + }); + + it("returns property name from MemberExpression node", () => { + expect( + getCalleeName({ + type: "MemberExpression", + object: { type: "Identifier", name: "React" }, + property: { type: "Identifier", name: "useEffect" }, + }), + ).toBe("useEffect"); + }); + + it("returns null for non-Identifier MemberExpression property", () => { + expect( + getCalleeName({ + type: "MemberExpression", + object: { type: "Identifier", name: "React" }, + property: { type: "Literal", value: "useEffect" }, + }), + ).toBeNull(); + }); + + it("returns null for null/undefined input", () => { + expect(getCalleeName(null)).toBeNull(); + expect(getCalleeName(undefined)).toBeNull(); + }); + + it("returns null for unsupported node types", () => { + expect(getCalleeName({ type: "CallExpression" })).toBeNull(); + }); +}); + +describe("isHookCall", () => { + const makeDirectCall = (name: string) => ({ + type: "CallExpression", + callee: { type: "Identifier", name }, + arguments: [], + }); + + const makeNamespaceCall = (namespace: string, name: string) => ({ + type: "CallExpression", + callee: { + type: "MemberExpression", + object: { type: "Identifier", name: namespace }, + property: { type: "Identifier", name }, + }, + arguments: [], + }); + + it("matches direct hook call with string hookName", () => { + expect(isHookCall(makeDirectCall("useEffect"), "useEffect")).toBe(true); + }); + + it("does not match wrong direct hook call", () => { + expect(isHookCall(makeDirectCall("useState"), "useEffect")).toBe(false); + }); + + it("matches namespace hook call with string hookName", () => { + expect(isHookCall(makeNamespaceCall("React", "useEffect"), "useEffect")).toBe(true); + }); + + it("matches namespace hook call with any namespace", () => { + expect(isHookCall(makeNamespaceCall("MyLib", "useEffect"), "useEffect")).toBe(true); + }); + + it("matches direct hook call with Set hookName", () => { + const hooks = new Set(["useEffect", "useLayoutEffect"]); + expect(isHookCall(makeDirectCall("useEffect"), hooks)).toBe(true); + expect(isHookCall(makeDirectCall("useLayoutEffect"), hooks)).toBe(true); + expect(isHookCall(makeDirectCall("useState"), hooks)).toBe(false); + }); + + it("matches namespace hook call with Set hookName", () => { + const hooks = new Set(["useEffect", "useLayoutEffect"]); + expect(isHookCall(makeNamespaceCall("React", "useEffect"), hooks)).toBe(true); + expect(isHookCall(makeNamespaceCall("React", "useState"), hooks)).toBe(false); + }); + + it("rejects non-CallExpression nodes", () => { + expect(isHookCall({ type: "Identifier", name: "useEffect" }, "useEffect")).toBe(false); + }); +}); + +describe("countSetStateCalls", () => { + it("counts direct setter calls", () => { + const node = { + type: "BlockStatement", + body: [ + { + type: "ExpressionStatement", + expression: { + type: "CallExpression", + callee: { type: "Identifier", name: "setName" }, + arguments: [{ type: "Literal", value: "John" }], + }, + }, + { + type: "ExpressionStatement", + expression: { + type: "CallExpression", + callee: { type: "Identifier", name: "setAge" }, + arguments: [{ type: "Literal", value: 30 }], + }, + }, + ], + }; + expect(countSetStateCalls(node)).toBe(2); + }); + + it("counts namespace setter calls (React.useState pattern)", () => { + const node = { + type: "BlockStatement", + body: [ + { + type: "ExpressionStatement", + expression: { + type: "CallExpression", + callee: { + type: "MemberExpression", + object: { type: "Identifier", name: "actions" }, + property: { type: "Identifier", name: "setName" }, + }, + arguments: [{ type: "Literal", value: "John" }], + }, + }, + ], + }; + expect(countSetStateCalls(node)).toBe(1); + }); + + it("does not count non-setter calls", () => { + const node = { + type: "BlockStatement", + body: [ + { + type: "ExpressionStatement", + expression: { + type: "CallExpression", + callee: { type: "Identifier", name: "fetchData" }, + arguments: [], + }, + }, + ], + }; + expect(countSetStateCalls(node)).toBe(0); + }); +});