diff --git a/packages/react-doctor/package.json b/packages/react-doctor/package.json index 755d3a1e43..f15958b416 100644 --- a/packages/react-doctor/package.json +++ b/packages/react-doctor/package.json @@ -82,6 +82,7 @@ }, "devDependencies": { "@types/prompts": "^2.4.9", + "@typescript-eslint/types": "^8.59.3", "eslint-plugin-react-hooks": "^7.1.1", "eslint-plugin-react-you-might-not-need-an-effect": "^0.10.1" }, diff --git a/packages/react-doctor/src/plugin/helpers.ts b/packages/react-doctor/src/plugin/helpers.ts index 829383352e..383654ef07 100644 --- a/packages/react-doctor/src/plugin/helpers.ts +++ b/packages/react-doctor/src/plugin/helpers.ts @@ -1,584 +1,28 @@ -import { - FETCH_CALLEE_NAMES, - FETCH_MEMBER_OBJECTS, - LOOP_TYPES, - MUTATING_HTTP_METHODS, - MUTATION_METHOD_NAMES, - SETTER_PATTERN, - UPPERCASE_PATTERN, -} from "./constants.js"; -import type { EsTreeNode, RuleVisitors } from "./types.js"; - -interface ComponentPropStackTrackerCallbacks { - onComponentEnter?: (componentBody: EsTreeNode | undefined) => void; -} - -interface ComponentPropStackTracker { - isPropName: (name: string) => boolean; - getCurrentPropNames: () => Set; - visitors: RuleVisitors; -} - -interface ComponentBindingStackTrackerCallbacks { - onVariableDeclarator?: (node: EsTreeNode) => void; -} - -interface ComponentBindingStackTracker { - isInsideComponent: () => boolean; - isBoundName: (name: string) => boolean; - addBindingToCurrentFrame: (name: string) => void; - visitors: RuleVisitors; -} - -// HACK: AST is acyclic except for `parent` back-references, which we skip. -// Visitors may return `false` to prune the subtree below `node` (e.g. to -// stop walking into nested functions when collecting `await` expressions -// for the enclosing function only). Returning anything else (including -// `undefined`, the natural value of statements) continues the walk. -export const walkAst = (node: EsTreeNode, visitor: (child: EsTreeNode) => boolean | void): void => { - if (!node || typeof node !== "object") return; - if (visitor(node) === false) return; - 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) { - walkAst(item, visitor); - } - } - } else if (child && typeof child === "object" && child.type) { - walkAst(child, visitor); - } - } -}; - -// 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 => - node.type === "CallExpression" && - node.callee?.type === "Identifier" && - isSetterIdentifier(node.callee.name); - -export const isUppercaseName = (name: string): boolean => UPPERCASE_PATTERN.test(name); - -export const isMemberProperty = (node: EsTreeNode, propertyName: string): boolean => - node.type === "MemberExpression" && - node.property?.type === "Identifier" && - node.property.name === propertyName; - -// HACK: walk a MemberExpression chain (computed or not) down to the -// underlying root identifier. `state.nested.items` → "state", -// `items[0]` → "items". Returns null if the chain bottoms out at -// anything other than a plain Identifier (e.g. a CallExpression, -// `this`, etc.). Bare Identifiers also resolve to themselves. -// -// When `followCallChains` is true, also walks past the receiver of -// any intermediate CallExpression — `items.toSorted().filter(fn)` → -// "items". Off by default because most callers want the receiver of -// the call (e.g. for "did this assignment write to props?"), not the -// expression that produced the receiver. -export const getRootIdentifierName = ( - node: EsTreeNode | undefined | null, - options?: { followCallChains?: boolean }, -): string | null => { - if (!node) return null; - if (node.type === "Identifier") return node.name; - const followCallChains = options?.followCallChains === true; - let cursor: EsTreeNode | undefined = node; - while (cursor) { - if (cursor.type === "MemberExpression") { - cursor = cursor.object; - continue; - } - if (followCallChains && cursor.type === "CallExpression") { - const callee = cursor.callee; - if (callee?.type !== "MemberExpression") return null; - cursor = callee.object; - continue; - } - break; - } - return cursor?.type === "Identifier" ? cursor.name : null; -}; - -// HACK: structural equality for "value-shaped" expressions used by -// detectors that need to assert two reads of the same external value -// (e.g. `prefer-use-sync-external-store` checks that the -// `useState(getSnapshot())` initializer matches the -// `setSnapshot(getSnapshot())` inside the subscribe handler). -// Deliberately conservative — we only model Identifier / Literal / -// MemberExpression / CallExpression because any other shape -// (assignments, ternaries, template strings) shouldn't be relied on -// for a "same external store read" claim. -export const areExpressionsStructurallyEqual = ( - a: EsTreeNode | null | undefined, - b: EsTreeNode | null | undefined, -): boolean => { - if (!a || !b) return a === b; - if (a.type !== b.type) return false; - if (a.type === "Identifier") return a.name === b.name; - if (a.type === "Literal") return a.value === b.value; - if (a.type === "MemberExpression") { - if (a.computed !== b.computed) return false; - return ( - areExpressionsStructurallyEqual(a.object, b.object) && - areExpressionsStructurallyEqual(a.property, b.property) - ); - } - if (a.type === "CallExpression") { - if (!areExpressionsStructurallyEqual(a.callee, b.callee)) return false; - const argumentsA = a.arguments ?? []; - const argumentsB = b.arguments ?? []; - if (argumentsA.length !== argumentsB.length) return false; - return argumentsA.every((argument: EsTreeNode, index: number) => - areExpressionsStructurallyEqual(argument, argumentsB[index]), - ); - } - return false; -}; - -export const getEffectCallback = (node: EsTreeNode): EsTreeNode | null => { - if (!node.arguments?.length) return null; - const callback = node.arguments[0]; - if (callback.type === "ArrowFunctionExpression" || callback.type === "FunctionExpression") { - return callback; - } - return null; -}; - -export const getCallbackStatements = (callback: EsTreeNode): EsTreeNode[] => { - if (callback.body?.type === "BlockStatement") { - return callback.body.body ?? []; - } - return callback.body ? [callback.body] : []; -}; - -export const countSetStateCalls = (node: EsTreeNode): number => { - let setStateCallCount = 0; - walkAst(node, (child) => { - if (isSetterCall(child)) setStateCallCount++; - }); - return setStateCallCount; -}; - -export const isSimpleExpression = (node: EsTreeNode | null): boolean => { - if (!node) return false; - switch (node.type) { - case "Identifier": - case "Literal": - case "TemplateLiteral": - return true; - case "BinaryExpression": - return isSimpleExpression(node.left) && isSimpleExpression(node.right); - case "UnaryExpression": - return isSimpleExpression(node.argument); - case "MemberExpression": - return !node.computed && isSimpleExpression(node.object); - case "ConditionalExpression": - return ( - isSimpleExpression(node.test) && - isSimpleExpression(node.consequent) && - isSimpleExpression(node.alternate) - ); - default: - return false; - } -}; - -export const isComponentDeclaration = (node: EsTreeNode): boolean => - node.type === "FunctionDeclaration" && Boolean(node.id?.name) && isUppercaseName(node.id.name); - -export const isComponentAssignment = (node: EsTreeNode): boolean => - node.type === "VariableDeclarator" && - node.id?.type === "Identifier" && - isUppercaseName(node.id.name) && - Boolean(node.init) && - (node.init.type === "ArrowFunctionExpression" || node.init.type === "FunctionExpression"); - -export const getCalleeName = (node: EsTreeNode): string | null => { - if (node.callee?.type === "Identifier") return node.callee.name; - if (node.callee?.type === "MemberExpression" && node.callee.property?.type === "Identifier") { - return node.callee.property.name; - } - return null; -}; - -export const isHookCall = (node: EsTreeNode, hookName: string | Set): boolean => { - if (node.type !== "CallExpression") return false; - const calleeName = getCalleeName(node); - if (!calleeName) return false; - return typeof hookName === "string" ? calleeName === hookName : hookName.has(calleeName); -}; - -export const hasDirective = (programNode: EsTreeNode, directive: string): boolean => - Boolean( - programNode.body?.some( - (statement: EsTreeNode) => - statement.type === "ExpressionStatement" && - statement.expression?.type === "Literal" && - statement.expression.value === directive, - ), - ); - -export const hasUseServerDirective = (node: EsTreeNode): boolean => { - if (node.body?.type !== "BlockStatement") return false; - return Boolean( - node.body.body?.some( - (statement: EsTreeNode) => - statement.type === "ExpressionStatement" && statement.directive === "use server", - ), - ); -}; - -export const containsFetchCall = (node: EsTreeNode): boolean => { - let didFindFetchCall = false; - walkAst(node, (child) => { - if (didFindFetchCall || child.type !== "CallExpression") return; - if (child.callee?.type === "Identifier" && FETCH_CALLEE_NAMES.has(child.callee.name)) { - didFindFetchCall = true; - } - if ( - child.callee?.type === "MemberExpression" && - child.callee.object?.type === "Identifier" && - FETCH_MEMBER_OBJECTS.has(child.callee.object.name) - ) { - didFindFetchCall = true; - } - }); - return didFindFetchCall; -}; - -export const findJsxAttribute = ( - attributes: EsTreeNode[], - attributeName: string, -): EsTreeNode | undefined => - attributes?.find( - (attr: EsTreeNode) => - attr.type === "JSXAttribute" && - attr.name?.type === "JSXIdentifier" && - attr.name.name === attributeName, - ); - -export const hasJsxAttribute = (attributes: EsTreeNode[], attributeName: string): boolean => - Boolean(findJsxAttribute(attributes, attributeName)); - -export const createLoopAwareVisitors = ( - innerVisitors: Record void>, -): RuleVisitors => { - let loopDepth = 0; - const incrementLoopDepth = (): void => { - loopDepth++; - }; - const decrementLoopDepth = (): void => { - loopDepth--; - }; - - const visitors: RuleVisitors = {}; - - for (const loopType of LOOP_TYPES) { - visitors[loopType] = incrementLoopDepth; - visitors[`${loopType}:exit`] = decrementLoopDepth; - } - - for (const [nodeType, handler] of Object.entries(innerVisitors)) { - visitors[nodeType] = (node: EsTreeNode) => { - if (loopDepth > 0) handler(node); - }; - } - - return visitors; -}; - -const isCookiesOrHeadersCall = (node: EsTreeNode, methodName: string): boolean => { - if (node.type !== "CallExpression" || node.callee?.type !== "MemberExpression") return false; - const { object, property } = node.callee; - if (property?.type !== "Identifier" || !MUTATION_METHOD_NAMES.has(property.name)) return false; - if (object?.type !== "CallExpression" || object.callee?.type !== "Identifier") return false; - return object.callee.name === methodName; -}; - -const isMutatingDbCall = (node: EsTreeNode): boolean => { - if (node.type !== "CallExpression" || node.callee?.type !== "MemberExpression") return false; - const { property } = node.callee; - return property?.type === "Identifier" && MUTATION_METHOD_NAMES.has(property.name); -}; - -// HACK: extracted so `findSideEffect` can re-use the EXACT same shape -// predicate when it goes hunting for the literal method to render in -// the diagnostic. Previously `findSideEffect` used a looser `key.name -// === "method"` predicate and could pick a non-Literal `method:` entry -// (when duplicate keys are present), producing -// `"fetch() with method undefined"` in the message. -const isMutatingMethodProperty = (property: EsTreeNode): boolean => - property.type === "Property" && - property.key?.type === "Identifier" && - property.key.name === "method" && - property.value?.type === "Literal" && - typeof property.value.value === "string" && - MUTATING_HTTP_METHODS.has(property.value.value.toUpperCase()); - -const isMutatingFetchCall = (node: EsTreeNode): boolean => { - if (node.type !== "CallExpression") return false; - if (node.callee?.type !== "Identifier" || node.callee.name !== "fetch") return false; - const optionsArgument = node.arguments?.[1]; - if (!optionsArgument || optionsArgument.type !== "ObjectExpression") return false; - return Boolean(optionsArgument.properties?.some(isMutatingMethodProperty)); -}; - -export const findSideEffect = (node: EsTreeNode): string | null => { - let sideEffectDescription: string | null = null; - walkAst(node, (child: EsTreeNode) => { - if (sideEffectDescription) return; - if (isCookiesOrHeadersCall(child, "cookies")) { - const methodName = child.callee.property.name; - sideEffectDescription = `cookies().${methodName}()`; - } else if (isCookiesOrHeadersCall(child, "headers")) { - const methodName = child.callee.property.name; - sideEffectDescription = `headers().${methodName}()`; - } else if (isMutatingFetchCall(child)) { - // HACK: re-use the EXACT predicate `isMutatingFetchCall` already - // matched on so we can't pick a non-Literal duplicate `method:` - // entry by mistake (a looser `key.name === "method"` predicate - // would). - const methodProperty = child.arguments[1].properties.find(isMutatingMethodProperty); - sideEffectDescription = `fetch() with method ${methodProperty.value.value}`; - } else if (isMutatingDbCall(child)) { - const methodName = child.callee.property.name; - const objectName = - child.callee.object?.type === "Identifier" ? child.callee.object.name : null; - sideEffectDescription = objectName ? `${objectName}.${methodName}()` : `.${methodName}()`; - } - }); - return sideEffectDescription; -}; - -// HACK: collects every locally-bound name introduced by a parameter list, -// recursing into nested object/array patterns. We need every binding so -// `noDerivedUseState` can detect e.g. `function Foo({ user: { name } })` → -// `useState(name)` (false negative if we only added "user"). -export const collectPatternNames = (pattern: EsTreeNode | null, into: Set): void => { - if (!pattern) return; - - if (pattern.type === "Identifier") { - into.add(pattern.name); - return; - } - - if (pattern.type === "AssignmentPattern") { - collectPatternNames(pattern.left, into); - return; - } - - if (pattern.type === "RestElement") { - collectPatternNames(pattern.argument, into); - return; - } - - if (pattern.type === "ArrayPattern") { - for (const element of pattern.elements ?? []) { - collectPatternNames(element, into); - } - return; - } - - if (pattern.type === "ObjectPattern") { - for (const property of pattern.properties ?? []) { - if (property.type === "RestElement") { - collectPatternNames(property.argument, into); - continue; - } - if (property.type === "Property") { - // The bound name lives in `property.value` (which may itself be - // a nested pattern). The `property.key` is the source-side name - // and only matters when it equals `property.value` (shorthand). - collectPatternNames(property.value, into); - } - } - } -}; - -const extractDestructuredPropNames = (params: EsTreeNode[]): Set => { - const propNames = new Set(); - for (const param of params) { - collectPatternNames(param, propNames); - } - return propNames; -}; - -// HACK: barrier-frame predicate used by `createComponentPropStackTracker` -// — a non-component arrow / function-expression VariableDeclarator -// pushes an empty stack frame so closed-over names from an outer -// component don't leak into the helper's prop check. -const isFunctionLikeVariableDeclarator = (node: EsTreeNode): boolean => { - if (node.type !== "VariableDeclarator") return false; - return node.init?.type === "ArrowFunctionExpression" || node.init?.type === "FunctionExpression"; -}; - -// HACK: every rule that walks "what props does the enclosing component -// have?" needs the SAME prop-stack machinery — push the destructured -// param set on FunctionDeclaration / VariableDeclarator entry, push -// an empty barrier for non-component nested helpers (so closed-over -// names don't leak in), pop on exit. Four rules previously inlined -// near-identical copies of this — they now compose this tracker. -// -// `isPropName(name)` is the lookup form most rules want during a -// CallExpression visit (returns false at the first barrier). -// -// `getCurrentPropNames()` returns a snapshot — useful when the rule -// runs eagerly on component entry instead of deferring to a later -// CallExpression visit. -// -// `onComponentEnter(body)` is invoked AFTER the prop set is pushed, -// from inside the FunctionDeclaration / VariableDeclarator visitor — -// rules that compute everything once per component (e.g. mirror-prop -// detection) hook in here. -export const createComponentPropStackTracker = ( - callbacks?: ComponentPropStackTrackerCallbacks, -): ComponentPropStackTracker => { - const propParamStack: Array> = []; - - const isPropName = (name: string): boolean => { - for (let frameIndex = propParamStack.length - 1; frameIndex >= 0; frameIndex--) { - const frame = propParamStack[frameIndex]; - if (frame.size === 0) return false; - if (frame.has(name)) return true; - } - return false; - }; - - const getCurrentPropNames = (): Set => { - for (let frameIndex = propParamStack.length - 1; frameIndex >= 0; frameIndex--) { - const frame = propParamStack[frameIndex]; - if (frame.size === 0) return new Set(); - return frame; - } - return new Set(); - }; - - const visitors: RuleVisitors = { - FunctionDeclaration(node: EsTreeNode) { - if (!node.id?.name || !isUppercaseName(node.id.name)) { - propParamStack.push(new Set()); - return; - } - propParamStack.push(extractDestructuredPropNames(node.params ?? [])); - callbacks?.onComponentEnter?.(node.body); - }, - "FunctionDeclaration:exit"() { - propParamStack.pop(); - }, - VariableDeclarator(node: EsTreeNode) { - if (isComponentAssignment(node)) { - propParamStack.push(extractDestructuredPropNames(node.init?.params ?? [])); - callbacks?.onComponentEnter?.(node.init?.body); - return; - } - if (isFunctionLikeVariableDeclarator(node)) { - propParamStack.push(new Set()); - } - }, - "VariableDeclarator:exit"(node: EsTreeNode) { - if (isComponentAssignment(node) || isFunctionLikeVariableDeclarator(node)) { - propParamStack.pop(); - } - }, - }; - - return { isPropName, getCurrentPropNames, visitors }; -}; - -// HACK: sibling of `createComponentPropStackTracker` for rules that need -// to track *binding* sets per component scope rather than the destructured -// prop set — e.g. `no-effect-event-in-deps` accumulates the names of -// `useEffectEvent` declarators while inside a component and then queries -// "is this dep-array identifier one of our useEffectEvent bindings?". -// -// Three rules previously reimplemented this push/pop bookkeeping inline. -// They now share the same scaffold; the per-rule predicate (e.g. "is the -// initializer a `useEffectEvent(...)` call?") lives in the -// `onVariableDeclarator` callback. -// -// The barrier semantic is intentionally simpler than the prop-stack -// tracker: the rule (e.g. `no-effect-event-in-deps`) only mutates the -// top frame for VariableDeclarators directly inside a component, and -// the stack only grows on FunctionDeclaration / VariableDeclarator -// component entries, so a closed-over name from an outer component -// can't leak in via a nested helper. -export const createComponentBindingStackTracker = ( - callbacks?: ComponentBindingStackTrackerCallbacks, -): ComponentBindingStackTracker => { - const componentBindingStack: Array> = []; - - const isInsideComponent = (): boolean => componentBindingStack.length > 0; - - const isBoundName = (name: string): boolean => { - for (let frameIndex = componentBindingStack.length - 1; frameIndex >= 0; frameIndex--) { - if (componentBindingStack[frameIndex].has(name)) return true; - } - return false; - }; - - const addBindingToCurrentFrame = (name: string): void => { - if (componentBindingStack.length === 0) return; - componentBindingStack[componentBindingStack.length - 1].add(name); - }; - - const visitors: RuleVisitors = { - FunctionDeclaration(node: EsTreeNode) { - if (!node.id?.name || !isUppercaseName(node.id.name)) return; - componentBindingStack.push(new Set()); - }, - "FunctionDeclaration:exit"(node: EsTreeNode) { - if (!node.id?.name || !isUppercaseName(node.id.name)) return; - componentBindingStack.pop(); - }, - VariableDeclarator(node: EsTreeNode) { - if (isComponentAssignment(node)) { - componentBindingStack.push(new Set()); - return; - } - callbacks?.onVariableDeclarator?.(node); - }, - "VariableDeclarator:exit"(node: EsTreeNode) { - if (isComponentAssignment(node)) componentBindingStack.pop(); - }, - }; - - return { isInsideComponent, isBoundName, addBindingToCurrentFrame, visitors }; -}; +export { + areExpressionsStructurallyEqual, + collectPatternNames, + containsFetchCall, + countSetStateCalls, + createComponentBindingStackTracker, + createComponentPropStackTracker, + createLoopAwareVisitors, + findJsxAttribute, + findSideEffect, + getCallbackStatements, + getCalleeName, + getEffectCallback, + getRootIdentifierName, + hasDirective, + hasJsxAttribute, + hasUseServerDirective, + isComponentAssignment, + isComponentDeclaration, + isHookCall, + isMemberProperty, + isSetterCall, + isSetterIdentifier, + isSimpleExpression, + isUppercaseName, + walkAst, + walkInsideStatementBlocks, +} from "./utils/index.js"; diff --git a/packages/react-doctor/src/plugin/rules/architecture.ts b/packages/react-doctor/src/plugin/rules/architecture.ts index 75c4c6ee17..679d7d813d 100644 --- a/packages/react-doctor/src/plugin/rules/architecture.ts +++ b/packages/react-doctor/src/plugin/rules/architecture.ts @@ -1,690 +1,12 @@ -import { - BOOLEAN_PROP_THRESHOLD, - GENERIC_EVENT_SUFFIXES, - GIANT_COMPONENT_LINE_THRESHOLD, - RENDER_FUNCTION_PATTERN, - RENDER_PROP_PROLIFERATION_THRESHOLD, -} from "../constants.js"; -import { - isComponentAssignment, - isComponentDeclaration, - isUppercaseName, - walkAst, -} from "../helpers.js"; -import type { EsTreeNode, Rule, RuleContext } from "../types.js"; - -export const noGenericHandlerNames: Rule = { - create: (context: RuleContext) => ({ - JSXAttribute(node: EsTreeNode) { - if (node.name?.type !== "JSXIdentifier" || !node.name.name.startsWith("on")) return; - if (!node.value || node.value.type !== "JSXExpressionContainer") return; - - const eventSuffix = node.name.name.slice(2); - if (!GENERIC_EVENT_SUFFIXES.has(eventSuffix)) return; - - const mirroredHandlerName = `handle${eventSuffix}`; - const expression = node.value.expression; - if (expression?.type === "Identifier" && expression.name === mirroredHandlerName) { - context.report({ - node, - message: `Non-descriptive handler name "${expression.name}" — name should describe what it does, not when it runs`, - }); - } - }, - }), -}; - -export const noGiantComponent: Rule = { - create: (context: RuleContext) => { - const reportOversizedComponent = ( - nameNode: EsTreeNode, - componentName: string, - bodyNode: EsTreeNode, - ): void => { - if (!bodyNode.loc) return; - const lineCount = bodyNode.loc.end.line - bodyNode.loc.start.line + 1; - if (lineCount > GIANT_COMPONENT_LINE_THRESHOLD) { - context.report({ - node: nameNode, - message: `Component "${componentName}" is ${lineCount} lines — consider breaking it into smaller focused components`, - }); - } - }; - - return { - FunctionDeclaration(node: EsTreeNode) { - if (!node.id?.name || !isUppercaseName(node.id.name)) return; - reportOversizedComponent(node.id, node.id.name, node); - }, - VariableDeclarator(node: EsTreeNode) { - if (!isComponentAssignment(node)) return; - reportOversizedComponent(node.id, node.id.name, node.init); - }, - }; - }, -}; - -export const noRenderInRender: Rule = { - create: (context: RuleContext) => ({ - JSXExpressionContainer(node: EsTreeNode) { - const expression = node.expression; - if (expression?.type !== "CallExpression") return; - - let calleeName: string | null = null; - if (expression.callee?.type === "Identifier") { - calleeName = expression.callee.name; - } else if ( - expression.callee?.type === "MemberExpression" && - expression.callee.property?.type === "Identifier" - ) { - calleeName = expression.callee.property.name; - } - - if (calleeName && RENDER_FUNCTION_PATTERN.test(calleeName)) { - context.report({ - node: expression, - message: `Inline render function "${calleeName}()" — extract to a separate component for proper reconciliation`, - }); - } - }, - }), -}; - -export const noNestedComponentDefinition: Rule = { - create: (context: RuleContext) => { - const componentStack: string[] = []; - - return { - FunctionDeclaration(node: EsTreeNode) { - if (!isComponentDeclaration(node)) return; - if (componentStack.length > 0) { - context.report({ - node: node.id, - message: `Component "${node.id.name}" defined inside "${componentStack[componentStack.length - 1]}" — creates new instance every render, destroying state`, - }); - } - componentStack.push(node.id.name); - }, - "FunctionDeclaration:exit"(node: EsTreeNode) { - if (isComponentDeclaration(node)) componentStack.pop(); - }, - VariableDeclarator(node: EsTreeNode) { - if (!isComponentAssignment(node)) return; - if (componentStack.length > 0) { - context.report({ - node: node.id, - message: `Component "${node.id.name}" defined inside "${componentStack[componentStack.length - 1]}" — creates new instance every render, destroying state`, - }); - } - componentStack.push(node.id.name); - }, - "VariableDeclarator:exit"(node: EsTreeNode) { - if (isComponentAssignment(node)) componentStack.pop(); - }, - }; - }, -}; - -const BOOLEAN_PROP_PREFIX_PATTERN = /^(?:is|has|should|can|show|hide|enable|disable|with)[A-Z]/; - -const collectBooleanLikePropsFromBody = ( - componentBody: EsTreeNode | undefined, - propsParamName: string, -): Set => { - const found = new Set(); - if (!componentBody) return found; - walkAst(componentBody, (child: EsTreeNode) => { - if (child.type !== "MemberExpression") return; - if (child.computed) return; - if (child.object?.type !== "Identifier") return; - if (child.object.name !== propsParamName) return; - if (child.property?.type !== "Identifier") return; - if (!BOOLEAN_PROP_PREFIX_PATTERN.test(child.property.name)) return; - found.add(child.property.name); - }); - return found; -}; - -// HACK: components with many boolean props (isLoading, hasIcon, showHeader, -// canEdit...) typically signal "many UI variants jammed into one component" -// — a sign that the component should be split via composition (compound -// components, explicit variant components). We use a name-based heuristic -// because TypeScript types aren't visible at this AST layer. Detects -// both destructured form (`{ isPrimary, hasIcon }`) and non-destructured -// (`function Foo(props) { props.isPrimary }`) by walking member-access -// patterns on the parameter binding. -export const noManyBooleanProps: Rule = { - create: (context: RuleContext) => { - const reportIfMany = ( - booleanLikePropNames: string[], - componentName: string, - reportNode: EsTreeNode, - ): void => { - if (booleanLikePropNames.length >= BOOLEAN_PROP_THRESHOLD) { - context.report({ - node: reportNode, - message: `Component "${componentName}" takes ${booleanLikePropNames.length} boolean-like props (${booleanLikePropNames.slice(0, 3).join(", ")}…) — consider compound components or explicit variants instead of stacking flags`, - }); - } - }; - - const checkComponent = ( - param: EsTreeNode | undefined, - body: EsTreeNode | undefined, - componentName: string, - reportNode: EsTreeNode, - ): void => { - if (!param) return; - if (param.type === "ObjectPattern") { - const booleanLikePropNames: string[] = []; - for (const property of param.properties ?? []) { - if (property.type !== "Property") continue; - const keyName = property.key?.type === "Identifier" ? property.key.name : null; - if (!keyName) continue; - if (BOOLEAN_PROP_PREFIX_PATTERN.test(keyName)) { - booleanLikePropNames.push(keyName); - } - } - reportIfMany(booleanLikePropNames, componentName, reportNode); - return; - } - if (param.type === "Identifier") { - const accessed = collectBooleanLikePropsFromBody(body, param.name); - reportIfMany([...accessed], componentName, reportNode); - } - }; - - return { - FunctionDeclaration(node: EsTreeNode) { - if (!isComponentDeclaration(node)) return; - checkComponent(node.params?.[0], node.body, node.id.name, node.id); - }, - VariableDeclarator(node: EsTreeNode) { - if (!isComponentAssignment(node)) return; - checkComponent(node.init?.params?.[0], node.init?.body, node.id.name, node.id); - }, - }; - }, -}; - -// HACK: React 19+ deprecated `forwardRef` (refs are now regular props on -// function components) and `useContext` (replaced by the more flexible -// `use()`). Catches both named imports (`import { forwardRef } from "react"`) -// AND member access on namespace/default imports (`React.forwardRef`, -// `React.useContext` after `import React from "react"` or -// `import * as React from "react"`). -// -// Stored as a Map (not a plain object) because plain-object lookups inherit -// from `Object.prototype` — `messages["constructor"]` returns the native -// `Object` function, which is truthy and would silently false-positive on -// `import { constructor } from "react"` or `React.toString()`. Maps return -// `undefined` for missing keys with no prototype fall-through. -const REACT_19_DEPRECATED_MESSAGES = new Map([ - [ - "forwardRef", - "forwardRef is no longer needed on React 19+ — refs are regular props on function components; remove forwardRef and pass ref directly", - ], - [ - "useContext", - "useContext is superseded by `use()` on React 19+ — `use()` reads context conditionally inside hooks, branches, and loops; switch to `import { use } from 'react'`", - ], -]); - -interface DeprecatedReactImportRuleOptions { - /** The exact `import "..."` source string this rule watches. */ - source: string; - /** Per-imported-name message dictionary. Exact-match lookup. */ - messages: ReadonlyMap; - /** - * Optional extra ImportDeclaration handler invoked BEFORE the standard - * source check — used by the react-dom rule to flag every import from - * `react-dom/test-utils` (whole entry point gone in React 19). - * Return `true` to mark "handled, skip the standard branch". - */ - handleExtraSource?: (node: EsTreeNode, context: RuleContext) => boolean; -} - -// HACK: shared scaffolding for "report deprecated React-package imports". -// Both `noReact19DeprecatedApis` (for `react`) and -// `noReactDomDeprecatedApis` (for `react-dom`) want the same shape: -// - bind namespace/default imports of the source to a Set -// - on ImportSpecifier, look the imported name up in a message map -// - on MemberExpression off a tracked binding, look the property up -// Hoisting the pattern keeps the two call sites tiny and means future -// React deprecations (e.g. a `react/jsx-runtime` rule) need just one -// new factory call. -const createDeprecatedReactImportRule = ({ - source, - messages, - handleExtraSource, -}: DeprecatedReactImportRuleOptions): Rule => ({ - create: (context: RuleContext) => { - const namespaceBindings = new Set(); - - return { - ImportDeclaration(node: EsTreeNode) { - const sourceValue = node.source?.value; - if (typeof sourceValue !== "string") return; - if (handleExtraSource?.(node, context)) return; - if (sourceValue !== source) return; - - for (const specifier of node.specifiers ?? []) { - if (specifier.type === "ImportSpecifier") { - const importedName = specifier.imported?.name; - if (!importedName) continue; - const message = messages.get(importedName); - if (message) context.report({ node: specifier, message }); - continue; - } - if ( - specifier.type === "ImportDefaultSpecifier" || - specifier.type === "ImportNamespaceSpecifier" - ) { - const localName = specifier.local?.name; - if (localName) namespaceBindings.add(localName); - } - } - }, - MemberExpression(node: EsTreeNode) { - if (namespaceBindings.size === 0) return; - if (node.computed) return; - if (node.object?.type !== "Identifier") return; - if (!namespaceBindings.has(node.object.name)) return; - if (node.property?.type !== "Identifier") return; - const message = messages.get(node.property.name); - if (message) context.report({ node, message }); - }, - }; - }, -}); - -export const noReact19DeprecatedApis: Rule = createDeprecatedReactImportRule({ - source: "react", - messages: REACT_19_DEPRECATED_MESSAGES, -}); - -const RENDER_PROP_PATTERN = /^render[A-Z]/; - -// HACK: render-prop proliferation (``) is the smell — a single render-prop is often -// the legitimate library API (MUI Autocomplete's `renderInput`, FlatList's -// `renderItem`, react-hook-form's Controller `render`, etc.) and we -// shouldn't fire on those. Instead we flag the COMPOUND case: when a -// single element receives 3 or more `render*` props, that's the smell -// of "many slots cobbled together where compound components or -// `children` would be cleaner". -export const noRenderPropChildren: Rule = { - create: (context: RuleContext) => ({ - JSXOpeningElement(node: EsTreeNode) { - const renderPropAttrs: Array<{ name: string; node: EsTreeNode }> = []; - for (const attr of node.attributes ?? []) { - if (attr.type !== "JSXAttribute") continue; - if (attr.name?.type !== "JSXIdentifier") continue; - const name = attr.name.name; - if (!RENDER_PROP_PATTERN.test(name)) continue; - renderPropAttrs.push({ name, node: attr }); - } - if (renderPropAttrs.length < RENDER_PROP_PROLIFERATION_THRESHOLD) return; - - const propList = renderPropAttrs - .slice(0, 3) - .map((entry) => entry.name) - .join(", "); - context.report({ - node: renderPropAttrs[0].node, - message: `${renderPropAttrs.length} render-prop slots on the same element (${propList}…) — collapse into compound subcomponents or \`children\` so consumers don't need to know about every customization point`, - }); - }, - }), -}; - -const HOOK_OBJECTS_WITH_METHODS = new Map>([ - ["useRouter", new Set(["push", "replace", "back", "forward", "refresh", "prefetch"])], - [ - "useNavigation", - new Set(["navigate", "push", "goBack", "popToTop", "reset", "replace", "dispatch"]), - ], - ["useSearchParams", new Set(["get", "getAll", "has", "set"])], -]); - -// HACK: O(1) lookup. Indexes top-level `const x = useFooBar(...)` -// declarations once per component on enter, so subsequent -// MemberExpression visitors don't re-walk the whole body for every -// access. -const buildHookBindingMap = (componentBody: EsTreeNode): Map => { - const result = new Map(); - if (componentBody?.type !== "BlockStatement") return result; - for (const statement of componentBody.body ?? []) { - if (statement.type !== "VariableDeclaration") continue; - for (const declarator of statement.declarations ?? []) { - if (declarator.id?.type !== "Identifier") continue; - if (declarator.init?.type !== "CallExpression") continue; - const callee = declarator.init.callee; - if (callee?.type !== "Identifier") continue; - result.set(declarator.id.name, callee.name); - } - } - return result; -}; - -// HACK: React Compiler memoizes inside a component based on stable -// reference equality of *destructured* values. `router.push("/x")` -// reads `push` off the hook return on every render, which the compiler -// can't memoize as cleanly as a destructured `const { push } = useRouter()`. -// The destructured form also makes the dependency graph obvious — if -// you only need `push`, the compiler doesn't need to track all of -// `router`. This is a soft signal even without React Compiler enabled -// (it makes intent clearer and reduces accidental capture). -// -// Heuristic: `router.push(...)` (or any of the canonical hook objects) -// where `router` is bound to a `useRouter()` call in the same component. -// We don't fire when the binding is destructured already. -export const reactCompilerDestructureMethod: Rule = { - create: (context: RuleContext) => { - const hookBindingMapStack: Array> = []; - - const isComponent = (node: EsTreeNode): boolean => { - if (node.type === "FunctionDeclaration") { - return Boolean(node.id?.name && isUppercaseName(node.id.name)); - } - if (node.type === "VariableDeclarator") { - return isComponentAssignment(node); - } - return false; - }; - - // HACK: push UNCONDITIONALLY for every component so push/pop stay - // balanced. A concise-arrow component (`const Foo = () =>
`) - // has no BlockStatement body and therefore no hook bindings, but it - // still triggers the matching `:exit` — without an unconditional - // push, the exit would pop the *outer* component's frame and silently - // drop diagnostics on every member access in the parent. The empty - // Map returned by `buildHookBindingMap` for non-Block bodies is the - // correct semantic for "this component declares zero hook bindings". - const enter = (node: EsTreeNode): void => { - if (!isComponent(node)) return; - const body = node.type === "FunctionDeclaration" ? node.body : node.init?.body; - hookBindingMapStack.push(buildHookBindingMap(body)); - }; - const exit = (node: EsTreeNode): void => { - if (isComponent(node)) hookBindingMapStack.pop(); - }; - - return { - FunctionDeclaration: enter, - "FunctionDeclaration:exit": exit, - VariableDeclarator: enter, - "VariableDeclarator:exit": exit, - MemberExpression(node: EsTreeNode) { - if (hookBindingMapStack.length === 0) return; - if (node.computed) return; - if (node.object?.type !== "Identifier") return; - if (node.property?.type !== "Identifier") return; - - const bindingName = node.object.name; - const methodName = node.property.name; - const hookBindings = hookBindingMapStack[hookBindingMapStack.length - 1]; - const hookSource = hookBindings.get(bindingName); - if (!hookSource) return; - - const allowedMethods = HOOK_OBJECTS_WITH_METHODS.get(hookSource); - if (!allowedMethods || !allowedMethods.has(methodName)) return; - - if (node.parent?.type !== "CallExpression" || node.parent.callee !== node) return; - - context.report({ - node, - message: `Destructure for clarity: \`const { ${methodName} } = ${hookSource}()\` then call \`${methodName}(...)\` directly — easier for React Compiler to memoize and clearer about which methods this component depends on`, - }); - }, - }; - }, -}; - -// HACK: the three legacy class lifecycles `componentWillMount`, -// `componentWillReceiveProps`, and `componentWillUpdate` are unsafe -// under concurrent rendering because the renderer can call them, throw -// the work away, and call them again. React 18.3.1 emits a warning; -// React 19 REMOVES them entirely (the `UNSAFE_` prefix included). We -// flag both forms so the prefix doesn't get treated as a permanent fix. -// -// Stored as a Map (not a plain object) because plain-object lookups inherit -// from `Object.prototype` — `LEGACY_LIFECYCLE_REPLACEMENTS["constructor"]` -// returns the native `Object` function (truthy), which previously made the -// rule false-positive on every class with a constructor (Lexical nodes, -// MobX stores, custom Error subclasses, etc.). Maps return `undefined` for -// missing keys with no prototype fall-through. -const LEGACY_LIFECYCLE_REPLACEMENTS = new Map([ - [ - "componentWillMount", - "Move side effects to `componentDidMount`; move initial state to `constructor`", - ], - [ - "componentWillReceiveProps", - "Move side effects to `componentDidUpdate` (compare prevProps); move pure state derivation to the static `getDerivedStateFromProps`", - ], - [ - "componentWillUpdate", - "Move DOM reads to `getSnapshotBeforeUpdate` (passes the value to `componentDidUpdate`); move other work to `componentDidUpdate`", - ], -]); - -interface UnsafePrefixSplit { - baseName: string; - hasUnsafePrefix: boolean; -} - -const stripUnsafePrefix = (name: string): UnsafePrefixSplit => { - if (name.startsWith("UNSAFE_")) { - return { baseName: name.slice("UNSAFE_".length), hasUnsafePrefix: true }; - } - return { baseName: name, hasUnsafePrefix: false }; -}; - -const buildLegacyLifecycleMessage = (originalName: string): string | null => { - const { baseName, hasUnsafePrefix } = stripUnsafePrefix(originalName); - const replacement = LEGACY_LIFECYCLE_REPLACEMENTS.get(baseName); - if (!replacement) return null; - const removalNote = hasUnsafePrefix - ? `\`${originalName}\` is removed in React 19 (the UNSAFE_ prefix only silences the React 18 warning, it doesn't fix the concurrent-mode hazard).` - : `\`${originalName}\` is removed in React 19 and warns in React 18.3.1.`; - return `${removalNote} ${replacement}.`; -}; - -export const noLegacyClassLifecycles: Rule = { - create: (context: RuleContext) => { - const checkMember = (memberNode: EsTreeNode | undefined): void => { - if (!memberNode) return; - if (memberNode.type !== "MethodDefinition" && memberNode.type !== "PropertyDefinition") - return; - if (memberNode.key?.type !== "Identifier") return; - const message = buildLegacyLifecycleMessage(memberNode.key.name); - if (message) context.report({ node: memberNode.key, message }); - }; - - return { - ClassBody(node: EsTreeNode) { - for (const member of node.body ?? []) { - checkMember(member); - } - }, - }; - }, -}; - -// HACK: legacy context (`childContextTypes` + `getChildContext` on -// providers, `contextTypes` on consumers) was deprecated in 16.3, warns -// in 18.3.1, and is REMOVED in 19. Migration is cross-file (provider + -// every consumer must be moved together) so flagging surface area early -// is high-leverage. We catch the static class-property forms AND the -// `Foo.contextTypes = {...}` shape — both styles appear in the wild, -// and missing one leaves silent gaps. -const LEGACY_CONTEXT_NAMES: ReadonlySet = new Set([ - "childContextTypes", - "contextTypes", - "getChildContext", -]); - -const buildLegacyContextMessage = (memberName: string): string => { - if (memberName === "childContextTypes" || memberName === "getChildContext") { - return `${memberName} is part of the legacy context API (REMOVED in React 19). Replace the provider with \`createContext\` + \`\` and consume via \`useContext()\` (or \`use()\` on React 19+) — every consumer must migrate together`; - } - return "contextTypes is part of the legacy context API (REMOVED in React 19). Replace with `static contextType = MyContext` (single context) or read the modern context with `useContext()` / `use()` from a function component — coordinate with the provider's migration"; -}; - -const isInsideClassBody = (node: EsTreeNode): boolean => { - let current = node.parent; - while (current) { - if (current.type === "ClassBody") return true; - if ( - current.type === "FunctionDeclaration" || - current.type === "FunctionExpression" || - current.type === "ArrowFunctionExpression" - ) { - return false; - } - current = current.parent; - } - return false; -}; - -export const noLegacyContextApi: Rule = { - create: (context: RuleContext) => { - const checkMember = (memberNode: EsTreeNode | undefined): void => { - if (!memberNode) return; - if (memberNode.type !== "MethodDefinition" && memberNode.type !== "PropertyDefinition") - return; - if (memberNode.key?.type !== "Identifier") return; - if (!LEGACY_CONTEXT_NAMES.has(memberNode.key.name)) return; - context.report({ - node: memberNode.key, - message: buildLegacyContextMessage(memberNode.key.name), - }); - }; - - return { - ClassBody(node: EsTreeNode) { - for (const member of node.body ?? []) { - checkMember(member); - } - }, - AssignmentExpression(node: EsTreeNode) { - if (node.operator !== "=") return; - const left = node.left; - if (left?.type !== "MemberExpression") return; - if (left.computed) return; - if (left.property?.type !== "Identifier") return; - if (!LEGACY_CONTEXT_NAMES.has(left.property.name)) return; - if (left.object?.type !== "Identifier") return; - if (!isUppercaseName(left.object.name)) return; - if (isInsideClassBody(node)) return; - context.report({ - node: left, - message: buildLegacyContextMessage(left.property.name), - }); - }, - }; - }, -}; - -// HACK: React 19 removes `Component.defaultProps` for FUNCTION components -// (class components still tolerate it but the team recommends ES6 -// default parameters anyway). Detection target: any -// `.defaultProps = ` assignment where the -// identifier looks like a component (uppercase first letter). We can't -// distinguish class vs function from the assignment alone, but the -// recommendation is the same either way — switch to ES6 default params -// in destructured props — so the guidance is uniform. -export const noDefaultProps: Rule = { - create: (context: RuleContext) => ({ - AssignmentExpression(node: EsTreeNode) { - if (node.operator !== "=") return; - const left = node.left; - if (left?.type !== "MemberExpression") return; - if (left.computed) return; - if (left.property?.type !== "Identifier" || left.property.name !== "defaultProps") return; - if (left.object?.type !== "Identifier") return; - if (!isUppercaseName(left.object.name)) return; - context.report({ - node: left, - message: `${left.object.name}.defaultProps — React 19 removes \`defaultProps\` for function components and discourages it for class components. Move defaults into the destructured props parameter (e.g. \`function ${left.object.name}({ size = "md", ...rest })\`) so the rule applies cleanly to both shapes`, - }); - }, - }), -}; - -// HACK: companion to `noReact19DeprecatedApis` for the react-dom side -// of the React 19 migration. Catches the legacy root API (render / -// hydrate / unmountComponentAtNode) and findDOMNode. The whole -// `react-dom/test-utils` entry point is gone in 19; we flag every -// import from it and steer users to `act` from `react` plus -// `fireEvent` / `render` from @testing-library/react. Kept as a -// separate rule from `noReact19DeprecatedApis` so the per-source -// binding tracking stays simple — `react` and `react-dom` namespace -// imports never collide. -// -// Deliberately omitted: `useFormState`. It's the *current* correct API -// in React 18 (`react-dom`) — only renamed to `useActionState` and -// moved to `react` in 19. A whole-rule version gate (`>= 18`) can't -// distinguish "still on 18" from "should have migrated" inside the -// rule, so we drop the entry rather than false-positive on 18 code. -const REACT_DOM_DEPRECATED_MESSAGES = new Map([ - [ - "render", - "ReactDOM.render is the legacy root API — switch to `import { createRoot } from 'react-dom/client'` and call `createRoot(container).render(...)` (REMOVED in React 19)", - ], - [ - "hydrate", - "ReactDOM.hydrate is the legacy SSR API — switch to `import { hydrateRoot } from 'react-dom/client'` and call `hydrateRoot(container, )` (REMOVED in React 19)", - ], - [ - "unmountComponentAtNode", - "ReactDOM.unmountComponentAtNode no longer works on roots created with `createRoot` — keep a reference to the root and call `root.unmount()` instead (REMOVED in React 19)", - ], - [ - "findDOMNode", - "ReactDOM.findDOMNode crawls the rendered tree and breaks composition — accept a ref directly and read `ref.current` (REMOVED in React 19)", - ], -]); - -const REACT_DOM_TEST_UTILS_REPLACEMENTS = new Map([ - ["act", "`import { act } from 'react'` instead"], - ["Simulate", "`fireEvent` from `@testing-library/react` instead"], - ["renderIntoDocument", "`render` from `@testing-library/react` instead"], - ["findRenderedDOMComponentWithTag", "`getByRole` / `getByTestId` from `@testing-library/react`"], - ["findRenderedDOMComponentWithClass", "`getByRole` or `container.querySelector` from RTL"], - ["scryRenderedDOMComponentsWithTag", "`getAllByRole` from `@testing-library/react`"], -]); - -const buildTestUtilsMessage = (importedName: string): string => { - const replacement = REACT_DOM_TEST_UTILS_REPLACEMENTS.get(importedName); - const replacementText = replacement - ? `Use ${replacement}.` - : "Switch to `act` from `react` or the equivalent in `@testing-library/react`."; - return `react-dom/test-utils is removed in React 19. ${replacementText}`; -}; - -const reportTestUtilsImports = (node: EsTreeNode, context: RuleContext): void => { - for (const specifier of node.specifiers ?? []) { - if (specifier.type === "ImportSpecifier") { - const importedName = specifier.imported?.name ?? "default"; - context.report({ node: specifier, message: buildTestUtilsMessage(importedName) }); - continue; - } - context.report({ - node: specifier, - message: - "react-dom/test-utils is removed in React 19. Use `act` from `react` and `fireEvent` / `render` from `@testing-library/react` instead", - }); - } -}; - -export const noReactDomDeprecatedApis: Rule = createDeprecatedReactImportRule({ - source: "react-dom", - messages: REACT_DOM_DEPRECATED_MESSAGES, - handleExtraSource: (node, context) => { - if (node.source?.value !== "react-dom/test-utils") return false; - reportTestUtilsImports(node, context); - return true; - }, -}); +export { noGenericHandlerNames } from "./architecture/no-generic-handler-names.js"; +export { noGiantComponent } from "./architecture/no-giant-component.js"; +export { noRenderInRender } from "./architecture/no-render-in-render.js"; +export { noNestedComponentDefinition } from "./architecture/no-nested-component-definition.js"; +export { noManyBooleanProps } from "./architecture/no-many-boolean-props.js"; +export { noReact19DeprecatedApis } from "./architecture/no-react19-deprecated-apis.js"; +export { noRenderPropChildren } from "./architecture/no-render-prop-children.js"; +export { reactCompilerDestructureMethod } from "./architecture/react-compiler-destructure-method.js"; +export { noLegacyClassLifecycles } from "./architecture/no-legacy-class-lifecycles.js"; +export { noLegacyContextApi } from "./architecture/no-legacy-context-api.js"; +export { noDefaultProps } from "./architecture/no-default-props.js"; +export { noReactDomDeprecatedApis } from "./architecture/no-react-dom-deprecated-apis.js"; diff --git a/packages/react-doctor/src/plugin/rules/architecture/no-default-props.ts b/packages/react-doctor/src/plugin/rules/architecture/no-default-props.ts new file mode 100644 index 0000000000..4631fef4f8 --- /dev/null +++ b/packages/react-doctor/src/plugin/rules/architecture/no-default-props.ts @@ -0,0 +1,31 @@ +import { defineRule } from "../../utils/define-rule.js"; +import { isUppercaseName } from "../../utils/is-uppercase-name.js"; +import type { EsTreeNode } from "../../utils/es-tree-node.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; + +// HACK: React 19 removes `Component.defaultProps` for FUNCTION components +// (class components still tolerate it but the team recommends ES6 +// default parameters anyway). Detection target: any +// `.defaultProps = ` assignment where the +// identifier looks like a component (uppercase first letter). We can't +// distinguish class vs function from the assignment alone, but the +// recommendation is the same either way — switch to ES6 default params +// in destructured props — so the guidance is uniform. +export const noDefaultProps = defineRule({ + create: (context: RuleContext) => ({ + AssignmentExpression(node: EsTreeNode) { + if (node.operator !== "=") return; + const left = node.left; + if (left?.type !== "MemberExpression") return; + if (left.computed) return; + if (left.property?.type !== "Identifier" || left.property.name !== "defaultProps") return; + if (left.object?.type !== "Identifier") return; + if (!isUppercaseName(left.object.name)) return; + context.report({ + node: left, + message: `${left.object.name}.defaultProps — React 19 removes \`defaultProps\` for function components and discourages it for class components. Move defaults into the destructured props parameter (e.g. \`function ${left.object.name}({ size = "md", ...rest })\`) so the rule applies cleanly to both shapes`, + }); + }, + }), +}); diff --git a/packages/react-doctor/src/plugin/rules/architecture/no-generic-handler-names.ts b/packages/react-doctor/src/plugin/rules/architecture/no-generic-handler-names.ts new file mode 100644 index 0000000000..f9d72ba72c --- /dev/null +++ b/packages/react-doctor/src/plugin/rules/architecture/no-generic-handler-names.ts @@ -0,0 +1,26 @@ +import { GENERIC_EVENT_SUFFIXES } from "../../constants.js"; +import { defineRule } from "../../utils/define-rule.js"; +import type { EsTreeNode } from "../../utils/es-tree-node.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; + +export const noGenericHandlerNames = defineRule({ + create: (context: RuleContext) => ({ + JSXAttribute(node: EsTreeNode) { + if (node.name?.type !== "JSXIdentifier" || !node.name.name.startsWith("on")) return; + if (!node.value || node.value.type !== "JSXExpressionContainer") return; + + const eventSuffix = node.name.name.slice(2); + if (!GENERIC_EVENT_SUFFIXES.has(eventSuffix)) return; + + const mirroredHandlerName = `handle${eventSuffix}`; + const expression = node.value.expression; + if (expression?.type === "Identifier" && expression.name === mirroredHandlerName) { + context.report({ + node, + message: `Non-descriptive handler name "${expression.name}" — name should describe what it does, not when it runs`, + }); + } + }, + }), +}); diff --git a/packages/react-doctor/src/plugin/rules/architecture/no-giant-component.ts b/packages/react-doctor/src/plugin/rules/architecture/no-giant-component.ts new file mode 100644 index 0000000000..3448902f93 --- /dev/null +++ b/packages/react-doctor/src/plugin/rules/architecture/no-giant-component.ts @@ -0,0 +1,37 @@ +import { GIANT_COMPONENT_LINE_THRESHOLD } from "../../constants.js"; +import { defineRule } from "../../utils/define-rule.js"; +import { isComponentAssignment } from "../../utils/is-component-assignment.js"; +import { isUppercaseName } from "../../utils/is-uppercase-name.js"; +import type { EsTreeNode } from "../../utils/es-tree-node.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; + +export const noGiantComponent = defineRule({ + create: (context: RuleContext) => { + const reportOversizedComponent = ( + nameNode: EsTreeNode, + componentName: string, + bodyNode: EsTreeNode, + ): void => { + if (!bodyNode.loc) return; + const lineCount = bodyNode.loc.end.line - bodyNode.loc.start.line + 1; + if (lineCount > GIANT_COMPONENT_LINE_THRESHOLD) { + context.report({ + node: nameNode, + message: `Component "${componentName}" is ${lineCount} lines — consider breaking it into smaller focused components`, + }); + } + }; + + return { + FunctionDeclaration(node: EsTreeNode) { + if (!node.id?.name || !isUppercaseName(node.id.name)) return; + reportOversizedComponent(node.id, node.id.name, node); + }, + VariableDeclarator(node: EsTreeNode) { + if (!isComponentAssignment(node)) return; + reportOversizedComponent(node.id, node.id.name, node.init); + }, + }; + }, +}); diff --git a/packages/react-doctor/src/plugin/rules/architecture/no-legacy-class-lifecycles.ts b/packages/react-doctor/src/plugin/rules/architecture/no-legacy-class-lifecycles.ts new file mode 100644 index 0000000000..a3aa744d70 --- /dev/null +++ b/packages/react-doctor/src/plugin/rules/architecture/no-legacy-class-lifecycles.ts @@ -0,0 +1,75 @@ +import { defineRule } from "../../utils/define-rule.js"; +import type { EsTreeNode } from "../../utils/es-tree-node.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; + +// HACK: the three legacy class lifecycles `componentWillMount`, +// `componentWillReceiveProps`, and `componentWillUpdate` are unsafe +// under concurrent rendering because the renderer can call them, throw +// the work away, and call them again. React 18.3.1 emits a warning; +// React 19 REMOVES them entirely (the `UNSAFE_` prefix included). We +// flag both forms so the prefix doesn't get treated as a permanent fix. +// +// Stored as a Map (not a plain object) because plain-object lookups inherit +// from `Object.prototype` — `LEGACY_LIFECYCLE_REPLACEMENTS["constructor"]` +// returns the native `Object` function (truthy), which previously made the +// rule false-positive on every class with a constructor (Lexical nodes, +// MobX stores, custom Error subclasses, etc.). Maps return `undefined` for +// missing keys with no prototype fall-through. +const LEGACY_LIFECYCLE_REPLACEMENTS = new Map([ + [ + "componentWillMount", + "Move side effects to `componentDidMount`; move initial state to `constructor`", + ], + [ + "componentWillReceiveProps", + "Move side effects to `componentDidUpdate` (compare prevProps); move pure state derivation to the static `getDerivedStateFromProps`", + ], + [ + "componentWillUpdate", + "Move DOM reads to `getSnapshotBeforeUpdate` (passes the value to `componentDidUpdate`); move other work to `componentDidUpdate`", + ], +]); + +interface UnsafePrefixSplit { + baseName: string; + hasUnsafePrefix: boolean; +} + +const stripUnsafePrefix = (name: string): UnsafePrefixSplit => { + if (name.startsWith("UNSAFE_")) { + return { baseName: name.slice("UNSAFE_".length), hasUnsafePrefix: true }; + } + return { baseName: name, hasUnsafePrefix: false }; +}; + +const buildLegacyLifecycleMessage = (originalName: string): string | null => { + const { baseName, hasUnsafePrefix } = stripUnsafePrefix(originalName); + const replacement = LEGACY_LIFECYCLE_REPLACEMENTS.get(baseName); + if (!replacement) return null; + const removalNote = hasUnsafePrefix + ? `\`${originalName}\` is removed in React 19 (the UNSAFE_ prefix only silences the React 18 warning, it doesn't fix the concurrent-mode hazard).` + : `\`${originalName}\` is removed in React 19 and warns in React 18.3.1.`; + return `${removalNote} ${replacement}.`; +}; + +export const noLegacyClassLifecycles = defineRule({ + create: (context: RuleContext) => { + const checkMember = (memberNode: EsTreeNode | undefined): void => { + if (!memberNode) return; + if (memberNode.type !== "MethodDefinition" && memberNode.type !== "PropertyDefinition") + return; + if (memberNode.key?.type !== "Identifier") return; + const message = buildLegacyLifecycleMessage(memberNode.key.name); + if (message) context.report({ node: memberNode.key, message }); + }; + + return { + ClassBody(node: EsTreeNode) { + for (const member of node.body ?? []) { + checkMember(member); + } + }, + }; + }, +}); diff --git a/packages/react-doctor/src/plugin/rules/architecture/no-legacy-context-api.ts b/packages/react-doctor/src/plugin/rules/architecture/no-legacy-context-api.ts new file mode 100644 index 0000000000..5ef7e5d1c5 --- /dev/null +++ b/packages/react-doctor/src/plugin/rules/architecture/no-legacy-context-api.ts @@ -0,0 +1,80 @@ +import { defineRule } from "../../utils/define-rule.js"; +import { isUppercaseName } from "../../utils/is-uppercase-name.js"; +import type { EsTreeNode } from "../../utils/es-tree-node.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; + +// HACK: legacy context (`childContextTypes` + `getChildContext` on +// providers, `contextTypes` on consumers) was deprecated in 16.3, warns +// in 18.3.1, and is REMOVED in 19. Migration is cross-file (provider + +// every consumer must be moved together) so flagging surface area early +// is high-leverage. We catch the static class-property forms AND the +// `Foo.contextTypes = {...}` shape — both styles appear in the wild, +// and missing one leaves silent gaps. +const LEGACY_CONTEXT_NAMES: ReadonlySet = new Set([ + "childContextTypes", + "contextTypes", + "getChildContext", +]); + +const buildLegacyContextMessage = (memberName: string): string => { + if (memberName === "childContextTypes" || memberName === "getChildContext") { + return `${memberName} is part of the legacy context API (REMOVED in React 19). Replace the provider with \`createContext\` + \`\` and consume via \`useContext()\` (or \`use()\` on React 19+) — every consumer must migrate together`; + } + return "contextTypes is part of the legacy context API (REMOVED in React 19). Replace with `static contextType = MyContext` (single context) or read the modern context with `useContext()` / `use()` from a function component — coordinate with the provider's migration"; +}; + +const isInsideClassBody = (node: EsTreeNode): boolean => { + let current = node.parent; + while (current) { + if (current.type === "ClassBody") return true; + if ( + current.type === "FunctionDeclaration" || + current.type === "FunctionExpression" || + current.type === "ArrowFunctionExpression" + ) { + return false; + } + current = current.parent; + } + return false; +}; + +export const noLegacyContextApi = defineRule({ + create: (context: RuleContext) => { + const checkMember = (memberNode: EsTreeNode | undefined): void => { + if (!memberNode) return; + if (memberNode.type !== "MethodDefinition" && memberNode.type !== "PropertyDefinition") + return; + if (memberNode.key?.type !== "Identifier") return; + if (!LEGACY_CONTEXT_NAMES.has(memberNode.key.name)) return; + context.report({ + node: memberNode.key, + message: buildLegacyContextMessage(memberNode.key.name), + }); + }; + + return { + ClassBody(node: EsTreeNode) { + for (const member of node.body ?? []) { + checkMember(member); + } + }, + AssignmentExpression(node: EsTreeNode) { + if (node.operator !== "=") return; + const left = node.left; + if (left?.type !== "MemberExpression") return; + if (left.computed) return; + if (left.property?.type !== "Identifier") return; + if (!LEGACY_CONTEXT_NAMES.has(left.property.name)) return; + if (left.object?.type !== "Identifier") return; + if (!isUppercaseName(left.object.name)) return; + if (isInsideClassBody(node)) return; + context.report({ + node: left, + message: buildLegacyContextMessage(left.property.name), + }); + }, + }; + }, +}); diff --git a/packages/react-doctor/src/plugin/rules/architecture/no-many-boolean-props.ts b/packages/react-doctor/src/plugin/rules/architecture/no-many-boolean-props.ts new file mode 100644 index 0000000000..01c175a00a --- /dev/null +++ b/packages/react-doctor/src/plugin/rules/architecture/no-many-boolean-props.ts @@ -0,0 +1,90 @@ +import { BOOLEAN_PROP_THRESHOLD } from "../../constants.js"; +import { defineRule } from "../../utils/define-rule.js"; +import { isComponentAssignment } from "../../utils/is-component-assignment.js"; +import { isComponentDeclaration } from "../../utils/is-component-declaration.js"; +import { walkAst } from "../../utils/walk-ast.js"; +import type { EsTreeNode } from "../../utils/es-tree-node.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; + +const BOOLEAN_PROP_PREFIX_PATTERN = /^(?:is|has|should|can|show|hide|enable|disable|with)[A-Z]/; + +const collectBooleanLikePropsFromBody = ( + componentBody: EsTreeNode | undefined, + propsParamName: string, +): Set => { + const found = new Set(); + if (!componentBody) return found; + walkAst(componentBody, (child: EsTreeNode) => { + if (child.type !== "MemberExpression") return; + if (child.computed) return; + if (child.object?.type !== "Identifier") return; + if (child.object.name !== propsParamName) return; + if (child.property?.type !== "Identifier") return; + if (!BOOLEAN_PROP_PREFIX_PATTERN.test(child.property.name)) return; + found.add(child.property.name); + }); + return found; +}; + +// HACK: components with many boolean props (isLoading, hasIcon, showHeader, +// canEdit...) typically signal "many UI variants jammed into one component" +// — a sign that the component should be split via composition (compound +// components, explicit variant components). We use a name-based heuristic +// because TypeScript types aren't visible at this AST layer. Detects +// both destructured form (`{ isPrimary, hasIcon }`) and non-destructured +// (`function Foo(props) { props.isPrimary }`) by walking member-access +// patterns on the parameter binding. +export const noManyBooleanProps = defineRule({ + create: (context: RuleContext) => { + const reportIfMany = ( + booleanLikePropNames: string[], + componentName: string, + reportNode: EsTreeNode, + ): void => { + if (booleanLikePropNames.length >= BOOLEAN_PROP_THRESHOLD) { + context.report({ + node: reportNode, + message: `Component "${componentName}" takes ${booleanLikePropNames.length} boolean-like props (${booleanLikePropNames.slice(0, 3).join(", ")}…) — consider compound components or explicit variants instead of stacking flags`, + }); + } + }; + + const checkComponent = ( + param: EsTreeNode | undefined, + body: EsTreeNode | undefined, + componentName: string, + reportNode: EsTreeNode, + ): void => { + if (!param) return; + if (param.type === "ObjectPattern") { + const booleanLikePropNames: string[] = []; + for (const property of param.properties ?? []) { + if (property.type !== "Property") continue; + const keyName = property.key?.type === "Identifier" ? property.key.name : null; + if (!keyName) continue; + if (BOOLEAN_PROP_PREFIX_PATTERN.test(keyName)) { + booleanLikePropNames.push(keyName); + } + } + reportIfMany(booleanLikePropNames, componentName, reportNode); + return; + } + if (param.type === "Identifier") { + const accessed = collectBooleanLikePropsFromBody(body, param.name); + reportIfMany([...accessed], componentName, reportNode); + } + }; + + return { + FunctionDeclaration(node: EsTreeNode) { + if (!isComponentDeclaration(node)) return; + checkComponent(node.params?.[0], node.body, node.id.name, node.id); + }, + VariableDeclarator(node: EsTreeNode) { + if (!isComponentAssignment(node)) return; + checkComponent(node.init?.params?.[0], node.init?.body, node.id.name, node.id); + }, + }; + }, +}); diff --git a/packages/react-doctor/src/plugin/rules/architecture/no-nested-component-definition.ts b/packages/react-doctor/src/plugin/rules/architecture/no-nested-component-definition.ts new file mode 100644 index 0000000000..483b4e949a --- /dev/null +++ b/packages/react-doctor/src/plugin/rules/architecture/no-nested-component-definition.ts @@ -0,0 +1,41 @@ +import { defineRule } from "../../utils/define-rule.js"; +import { isComponentAssignment } from "../../utils/is-component-assignment.js"; +import { isComponentDeclaration } from "../../utils/is-component-declaration.js"; +import type { EsTreeNode } from "../../utils/es-tree-node.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; + +export const noNestedComponentDefinition = defineRule({ + create: (context: RuleContext) => { + const componentStack: string[] = []; + + return { + FunctionDeclaration(node: EsTreeNode) { + if (!isComponentDeclaration(node)) return; + if (componentStack.length > 0) { + context.report({ + node: node.id, + message: `Component "${node.id.name}" defined inside "${componentStack[componentStack.length - 1]}" — creates new instance every render, destroying state`, + }); + } + componentStack.push(node.id.name); + }, + "FunctionDeclaration:exit"(node: EsTreeNode) { + if (isComponentDeclaration(node)) componentStack.pop(); + }, + VariableDeclarator(node: EsTreeNode) { + if (!isComponentAssignment(node)) return; + if (componentStack.length > 0) { + context.report({ + node: node.id, + message: `Component "${node.id.name}" defined inside "${componentStack[componentStack.length - 1]}" — creates new instance every render, destroying state`, + }); + } + componentStack.push(node.id.name); + }, + "VariableDeclarator:exit"(node: EsTreeNode) { + if (isComponentAssignment(node)) componentStack.pop(); + }, + }; + }, +}); diff --git a/packages/react-doctor/src/plugin/rules/architecture/no-react-dom-deprecated-apis.ts b/packages/react-doctor/src/plugin/rules/architecture/no-react-dom-deprecated-apis.ts new file mode 100644 index 0000000000..61c86f3675 --- /dev/null +++ b/packages/react-doctor/src/plugin/rules/architecture/no-react-dom-deprecated-apis.ts @@ -0,0 +1,83 @@ +import { defineRule } from "../../utils/define-rule.js"; +import type { EsTreeNode } from "../../utils/es-tree-node.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; +import { createDeprecatedReactImportRule } from "./utils/create-deprecated-react-import-rule.js"; + +// HACK: companion to `noReact19DeprecatedApis` for the react-dom side +// of the React 19 migration. Catches the legacy root API (render / +// hydrate / unmountComponentAtNode) and findDOMNode. The whole +// `react-dom/test-utils` entry point is gone in 19; we flag every +// import from it and steer users to `act` from `react` plus +// `fireEvent` / `render` from @testing-library/react. Kept as a +// separate rule from `noReact19DeprecatedApis` so the per-source +// binding tracking stays simple — `react` and `react-dom` namespace +// imports never collide. +// +// Deliberately omitted: `useFormState`. It's the *current* correct API +// in React 18 (`react-dom`) — only renamed to `useActionState` and +// moved to `react` in 19. A whole-rule version gate (`>= 18`) can't +// distinguish "still on 18" from "should have migrated" inside the +// rule, so we drop the entry rather than false-positive on 18 code. +const REACT_DOM_DEPRECATED_MESSAGES = new Map([ + [ + "render", + "ReactDOM.render is the legacy root API — switch to `import { createRoot } from 'react-dom/client'` and call `createRoot(container).render(...)` (REMOVED in React 19)", + ], + [ + "hydrate", + "ReactDOM.hydrate is the legacy SSR API — switch to `import { hydrateRoot } from 'react-dom/client'` and call `hydrateRoot(container, )` (REMOVED in React 19)", + ], + [ + "unmountComponentAtNode", + "ReactDOM.unmountComponentAtNode no longer works on roots created with `createRoot` — keep a reference to the root and call `root.unmount()` instead (REMOVED in React 19)", + ], + [ + "findDOMNode", + "ReactDOM.findDOMNode crawls the rendered tree and breaks composition — accept a ref directly and read `ref.current` (REMOVED in React 19)", + ], +]); + +const REACT_DOM_TEST_UTILS_REPLACEMENTS = new Map([ + ["act", "`import { act } from 'react'` instead"], + ["Simulate", "`fireEvent` from `@testing-library/react` instead"], + ["renderIntoDocument", "`render` from `@testing-library/react` instead"], + ["findRenderedDOMComponentWithTag", "`getByRole` / `getByTestId` from `@testing-library/react`"], + ["findRenderedDOMComponentWithClass", "`getByRole` or `container.querySelector` from RTL"], + ["scryRenderedDOMComponentsWithTag", "`getAllByRole` from `@testing-library/react`"], +]); + +const buildTestUtilsMessage = (importedName: string): string => { + const replacement = REACT_DOM_TEST_UTILS_REPLACEMENTS.get(importedName); + const replacementText = replacement + ? `Use ${replacement}.` + : "Switch to `act` from `react` or the equivalent in `@testing-library/react`."; + return `react-dom/test-utils is removed in React 19. ${replacementText}`; +}; + +const reportTestUtilsImports = (node: EsTreeNode, context: RuleContext): void => { + for (const specifier of node.specifiers ?? []) { + if (specifier.type === "ImportSpecifier") { + const importedName = specifier.imported?.name ?? "default"; + context.report({ node: specifier, message: buildTestUtilsMessage(importedName) }); + continue; + } + context.report({ + node: specifier, + message: + "react-dom/test-utils is removed in React 19. Use `act` from `react` and `fireEvent` / `render` from `@testing-library/react` instead", + }); + } +}; + +export const noReactDomDeprecatedApis = defineRule( + createDeprecatedReactImportRule({ + source: "react-dom", + messages: REACT_DOM_DEPRECATED_MESSAGES, + handleExtraSource: (node, context) => { + if (node.source?.value !== "react-dom/test-utils") return false; + reportTestUtilsImports(node, context); + return true; + }, + }), +); diff --git a/packages/react-doctor/src/plugin/rules/architecture/no-react19-deprecated-apis.ts b/packages/react-doctor/src/plugin/rules/architecture/no-react19-deprecated-apis.ts new file mode 100644 index 0000000000..7d0df3b332 --- /dev/null +++ b/packages/react-doctor/src/plugin/rules/architecture/no-react19-deprecated-apis.ts @@ -0,0 +1,33 @@ +import { defineRule } from "../../utils/define-rule.js"; +import type { Rule } from "../../utils/rule.js"; +import { createDeprecatedReactImportRule } from "./utils/create-deprecated-react-import-rule.js"; + +// HACK: React 19+ deprecated `forwardRef` (refs are now regular props on +// function components) and `useContext` (replaced by the more flexible +// `use()`). Catches both named imports (`import { forwardRef } from "react"`) +// AND member access on namespace/default imports (`React.forwardRef`, +// `React.useContext` after `import React from "react"` or +// `import * as React from "react"`). +// +// Stored as a Map (not a plain object) because plain-object lookups inherit +// from `Object.prototype` — `messages["constructor"]` returns the native +// `Object` function, which is truthy and would silently false-positive on +// `import { constructor } from "react"` or `React.toString()`. Maps return +// `undefined` for missing keys with no prototype fall-through. +const REACT_19_DEPRECATED_MESSAGES = new Map([ + [ + "forwardRef", + "forwardRef is no longer needed on React 19+ — refs are regular props on function components; remove forwardRef and pass ref directly", + ], + [ + "useContext", + "useContext is superseded by `use()` on React 19+ — `use()` reads context conditionally inside hooks, branches, and loops; switch to `import { use } from 'react'`", + ], +]); + +export const noReact19DeprecatedApis = defineRule( + createDeprecatedReactImportRule({ + source: "react", + messages: REACT_19_DEPRECATED_MESSAGES, + }), +); diff --git a/packages/react-doctor/src/plugin/rules/architecture/no-render-in-render.ts b/packages/react-doctor/src/plugin/rules/architecture/no-render-in-render.ts new file mode 100644 index 0000000000..c591542f87 --- /dev/null +++ b/packages/react-doctor/src/plugin/rules/architecture/no-render-in-render.ts @@ -0,0 +1,31 @@ +import { RENDER_FUNCTION_PATTERN } from "../../constants.js"; +import { defineRule } from "../../utils/define-rule.js"; +import type { EsTreeNode } from "../../utils/es-tree-node.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; + +export const noRenderInRender = defineRule({ + create: (context: RuleContext) => ({ + JSXExpressionContainer(node: EsTreeNode) { + const expression = node.expression; + if (expression?.type !== "CallExpression") return; + + let calleeName: string | null = null; + if (expression.callee?.type === "Identifier") { + calleeName = expression.callee.name; + } else if ( + expression.callee?.type === "MemberExpression" && + expression.callee.property?.type === "Identifier" + ) { + calleeName = expression.callee.property.name; + } + + if (calleeName && RENDER_FUNCTION_PATTERN.test(calleeName)) { + context.report({ + node: expression, + message: `Inline render function "${calleeName}()" — extract to a separate component for proper reconciliation`, + }); + } + }, + }), +}); diff --git a/packages/react-doctor/src/plugin/rules/architecture/no-render-prop-children.ts b/packages/react-doctor/src/plugin/rules/architecture/no-render-prop-children.ts new file mode 100644 index 0000000000..e4add66ac7 --- /dev/null +++ b/packages/react-doctor/src/plugin/rules/architecture/no-render-prop-children.ts @@ -0,0 +1,40 @@ +import { RENDER_PROP_PROLIFERATION_THRESHOLD } from "../../constants.js"; +import { defineRule } from "../../utils/define-rule.js"; +import type { EsTreeNode } from "../../utils/es-tree-node.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; + +const RENDER_PROP_PATTERN = /^render[A-Z]/; + +// HACK: render-prop proliferation (``) is the smell — a single render-prop is often +// the legitimate library API (MUI Autocomplete's `renderInput`, FlatList's +// `renderItem`, react-hook-form's Controller `render`, etc.) and we +// shouldn't fire on those. Instead we flag the COMPOUND case: when a +// single element receives 3 or more `render*` props, that's the smell +// of "many slots cobbled together where compound components or +// `children` would be cleaner". +export const noRenderPropChildren = defineRule({ + create: (context: RuleContext) => ({ + JSXOpeningElement(node: EsTreeNode) { + const renderPropAttrs: Array<{ name: string; node: EsTreeNode }> = []; + for (const attr of node.attributes ?? []) { + if (attr.type !== "JSXAttribute") continue; + if (attr.name?.type !== "JSXIdentifier") continue; + const name = attr.name.name; + if (!RENDER_PROP_PATTERN.test(name)) continue; + renderPropAttrs.push({ name, node: attr }); + } + if (renderPropAttrs.length < RENDER_PROP_PROLIFERATION_THRESHOLD) return; + + const propList = renderPropAttrs + .slice(0, 3) + .map((entry) => entry.name) + .join(", "); + context.report({ + node: renderPropAttrs[0].node, + message: `${renderPropAttrs.length} render-prop slots on the same element (${propList}…) — collapse into compound subcomponents or \`children\` so consumers don't need to know about every customization point`, + }); + }, + }), +}); diff --git a/packages/react-doctor/src/plugin/rules/architecture/react-compiler-destructure-method.ts b/packages/react-doctor/src/plugin/rules/architecture/react-compiler-destructure-method.ts new file mode 100644 index 0000000000..585f5b9367 --- /dev/null +++ b/packages/react-doctor/src/plugin/rules/architecture/react-compiler-destructure-method.ts @@ -0,0 +1,109 @@ +import { defineRule } from "../../utils/define-rule.js"; +import { isComponentAssignment } from "../../utils/is-component-assignment.js"; +import { isUppercaseName } from "../../utils/is-uppercase-name.js"; +import type { EsTreeNode } from "../../utils/es-tree-node.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; + +const HOOK_OBJECTS_WITH_METHODS = new Map>([ + ["useRouter", new Set(["push", "replace", "back", "forward", "refresh", "prefetch"])], + [ + "useNavigation", + new Set(["navigate", "push", "goBack", "popToTop", "reset", "replace", "dispatch"]), + ], + ["useSearchParams", new Set(["get", "getAll", "has", "set"])], +]); + +// HACK: O(1) lookup. Indexes top-level `const x = useFooBar(...)` +// declarations once per component on enter, so subsequent +// MemberExpression visitors don't re-walk the whole body for every +// access. +const buildHookBindingMap = (componentBody: EsTreeNode): Map => { + const result = new Map(); + if (componentBody?.type !== "BlockStatement") return result; + for (const statement of componentBody.body ?? []) { + if (statement.type !== "VariableDeclaration") continue; + for (const declarator of statement.declarations ?? []) { + if (declarator.id?.type !== "Identifier") continue; + if (declarator.init?.type !== "CallExpression") continue; + const callee = declarator.init.callee; + if (callee?.type !== "Identifier") continue; + result.set(declarator.id.name, callee.name); + } + } + return result; +}; + +// HACK: React Compiler memoizes inside a component based on stable +// reference equality of *destructured* values. `router.push("/x")` +// reads `push` off the hook return on every render, which the compiler +// can't memoize as cleanly as a destructured `const { push } = useRouter()`. +// The destructured form also makes the dependency graph obvious — if +// you only need `push`, the compiler doesn't need to track all of +// `router`. This is a soft signal even without React Compiler enabled +// (it makes intent clearer and reduces accidental capture). +// +// Heuristic: `router.push(...)` (or any of the canonical hook objects) +// where `router` is bound to a `useRouter()` call in the same component. +// We don't fire when the binding is destructured already. +export const reactCompilerDestructureMethod = defineRule({ + create: (context: RuleContext) => { + const hookBindingMapStack: Array> = []; + + const isComponent = (node: EsTreeNode): boolean => { + if (node.type === "FunctionDeclaration") { + return Boolean(node.id?.name && isUppercaseName(node.id.name)); + } + if (node.type === "VariableDeclarator") { + return isComponentAssignment(node); + } + return false; + }; + + // HACK: push UNCONDITIONALLY for every component so push/pop stay + // balanced. A concise-arrow component (`const Foo = () =>
`) + // has no BlockStatement body and therefore no hook bindings, but it + // still triggers the matching `:exit` — without an unconditional + // push, the exit would pop the *outer* component's frame and silently + // drop diagnostics on every member access in the parent. The empty + // Map returned by `buildHookBindingMap` for non-Block bodies is the + // correct semantic for "this component declares zero hook bindings". + const enter = (node: EsTreeNode): void => { + if (!isComponent(node)) return; + const body = node.type === "FunctionDeclaration" ? node.body : node.init?.body; + hookBindingMapStack.push(buildHookBindingMap(body)); + }; + const exit = (node: EsTreeNode): void => { + if (isComponent(node)) hookBindingMapStack.pop(); + }; + + return { + FunctionDeclaration: enter, + "FunctionDeclaration:exit": exit, + VariableDeclarator: enter, + "VariableDeclarator:exit": exit, + MemberExpression(node: EsTreeNode) { + if (hookBindingMapStack.length === 0) return; + if (node.computed) return; + if (node.object?.type !== "Identifier") return; + if (node.property?.type !== "Identifier") return; + + const bindingName = node.object.name; + const methodName = node.property.name; + const hookBindings = hookBindingMapStack[hookBindingMapStack.length - 1]; + const hookSource = hookBindings.get(bindingName); + if (!hookSource) return; + + const allowedMethods = HOOK_OBJECTS_WITH_METHODS.get(hookSource); + if (!allowedMethods || !allowedMethods.has(methodName)) return; + + if (node.parent?.type !== "CallExpression" || node.parent.callee !== node) return; + + context.report({ + node, + message: `Destructure for clarity: \`const { ${methodName} } = ${hookSource}()\` then call \`${methodName}(...)\` directly — easier for React Compiler to memoize and clearer about which methods this component depends on`, + }); + }, + }; + }, +}); diff --git a/packages/react-doctor/src/plugin/rules/architecture/utils/create-deprecated-react-import-rule.ts b/packages/react-doctor/src/plugin/rules/architecture/utils/create-deprecated-react-import-rule.ts new file mode 100644 index 0000000000..72b34fc096 --- /dev/null +++ b/packages/react-doctor/src/plugin/rules/architecture/utils/create-deprecated-react-import-rule.ts @@ -0,0 +1,49 @@ +import type { EsTreeNode } from "../../../utils/es-tree-node.js"; +import type { RuleContext } from "../../../utils/rule-context.js"; +import type { Rule } from "../../../utils/rule.js"; +import type { DeprecatedReactImportRuleOptions } from "./deprecated-react-import-rule-options.js"; + +export const createDeprecatedReactImportRule = ({ + source, + messages, + handleExtraSource, +}: DeprecatedReactImportRuleOptions): Rule => ({ + create: (context: RuleContext) => { + const namespaceBindings = new Set(); + + return { + ImportDeclaration(node: EsTreeNode) { + const sourceValue = node.source?.value; + if (typeof sourceValue !== "string") return; + if (handleExtraSource?.(node, context)) return; + if (sourceValue !== source) return; + + for (const specifier of node.specifiers ?? []) { + if (specifier.type === "ImportSpecifier") { + const importedName = specifier.imported?.name; + if (!importedName) continue; + const message = messages.get(importedName); + if (message) context.report({ node: specifier, message }); + continue; + } + if ( + specifier.type === "ImportDefaultSpecifier" || + specifier.type === "ImportNamespaceSpecifier" + ) { + const localName = specifier.local?.name; + if (localName) namespaceBindings.add(localName); + } + } + }, + MemberExpression(node: EsTreeNode) { + if (namespaceBindings.size === 0) return; + if (node.computed) return; + if (node.object?.type !== "Identifier") return; + if (!namespaceBindings.has(node.object.name)) return; + if (node.property?.type !== "Identifier") return; + const message = messages.get(node.property.name); + if (message) context.report({ node, message }); + }, + }; + }, +}); diff --git a/packages/react-doctor/src/plugin/rules/architecture/utils/deprecated-react-import-rule-options.ts b/packages/react-doctor/src/plugin/rules/architecture/utils/deprecated-react-import-rule-options.ts new file mode 100644 index 0000000000..4e795b5f4d --- /dev/null +++ b/packages/react-doctor/src/plugin/rules/architecture/utils/deprecated-react-import-rule-options.ts @@ -0,0 +1,16 @@ +import type { EsTreeNode } from "../../../utils/es-tree-node.js"; +import type { RuleContext } from "../../../utils/rule-context.js"; + +export interface DeprecatedReactImportRuleOptions { + /** The exact `import "..."` source string this rule watches. */ + source: string; + /** Per-imported-name message dictionary. Exact-match lookup. */ + messages: ReadonlyMap; + /** + * Optional extra ImportDeclaration handler invoked BEFORE the standard + * source check — used by the react-dom rule to flag every import from + * `react-dom/test-utils` (whole entry point gone in React 19). + * Return `true` to mark "handled, skip the standard branch". + */ + handleExtraSource?: (node: EsTreeNode, context: RuleContext) => boolean; +} diff --git a/packages/react-doctor/src/plugin/rules/bundle-size.ts b/packages/react-doctor/src/plugin/rules/bundle-size.ts index e237c26948..37430bfb6d 100644 --- a/packages/react-doctor/src/plugin/rules/bundle-size.ts +++ b/packages/react-doctor/src/plugin/rules/bundle-size.ts @@ -1,153 +1,7 @@ -import { BARREL_INDEX_SUFFIXES, HEAVY_LIBRARIES } from "../constants.js"; -import { findJsxAttribute, hasJsxAttribute } from "../helpers.js"; -import type { EsTreeNode, Rule, RuleContext } from "../types.js"; - -export const noBarrelImport: Rule = { - create: (context: RuleContext) => { - let didReportForFile = false; - - return { - ImportDeclaration(node: EsTreeNode) { - if (didReportForFile) return; - - const source = node.source?.value; - if (typeof source !== "string" || !source.startsWith(".")) return; - - if (BARREL_INDEX_SUFFIXES.some((suffix) => source.endsWith(suffix))) { - didReportForFile = true; - context.report({ - node, - message: - "Import from barrel/index file — import directly from the source module for better tree-shaking", - }); - } - }, - }; - }, -}; - -export const noFullLodashImport: Rule = { - create: (context: RuleContext) => ({ - ImportDeclaration(node: EsTreeNode) { - const source = node.source?.value; - if (source === "lodash" || source === "lodash-es") { - context.report({ - node, - message: "Importing entire lodash library — import from 'lodash/functionName' instead", - }); - } - }, - }), -}; - -export const noMoment: Rule = { - create: (context: RuleContext) => ({ - ImportDeclaration(node: EsTreeNode) { - if (node.source?.value === "moment") { - context.report({ - node, - message: 'moment.js is 300kb+ — use "date-fns" or "dayjs" instead', - }); - } - }, - }), -}; - -export const preferDynamicImport: Rule = { - create: (context: RuleContext) => ({ - ImportDeclaration(node: EsTreeNode) { - const source = node.source?.value; - if (typeof source === "string" && HEAVY_LIBRARIES.has(source)) { - context.report({ - node, - message: `"${source}" is a heavy library — use React.lazy() or next/dynamic for code splitting`, - }); - } - }, - }), -}; - -export const useLazyMotion: Rule = { - create: (context: RuleContext) => ({ - ImportDeclaration(node: EsTreeNode) { - const source = node.source?.value; - if (source !== "framer-motion" && source !== "motion/react") return; - - const hasFullMotionImport = node.specifiers?.some( - (specifier: EsTreeNode) => - specifier.type === "ImportSpecifier" && specifier.imported?.name === "motion", - ); - - if (hasFullMotionImport) { - context.report({ - node, - message: 'Import "m" with LazyMotion instead of "motion" — saves ~30kb in bundle size', - }); - } - }, - }), -}; - -// HACK: bundlers can only tree-shake / split when the import target is a -// statically-analyzable string literal. `import(variable)` or -// `require(variable)` defeats trace targets and forces a fat bundle. -export const noDynamicImportPath: Rule = { - create: (context: RuleContext) => ({ - ImportExpression(node: EsTreeNode) { - const source = node.source; - if (source && source.type !== "Literal" && source.type !== "TemplateLiteral") { - context.report({ - node, - message: - "Dynamic import path is not statically analyzable — use a string literal so the bundler can split this chunk", - }); - return; - } - if (source?.type === "TemplateLiteral" && (source.expressions?.length ?? 0) > 0) { - context.report({ - node, - message: - "Template literal with interpolation in dynamic import — use a string literal so the bundler can split this chunk", - }); - } - }, - CallExpression(node: EsTreeNode) { - if (node.callee?.type !== "Identifier" || node.callee.name !== "require") return; - const arg = node.arguments?.[0]; - if (!arg) return; - if (arg.type !== "Literal" && arg.type !== "TemplateLiteral") { - context.report({ - node, - message: - "Dynamic require() path is not statically analyzable — use a string literal so the bundler can trace this dependency", - }); - return; - } - if (arg.type === "TemplateLiteral" && (arg.expressions?.length ?? 0) > 0) { - context.report({ - node, - message: - "Template literal with interpolation in require() — use a string literal so the bundler can trace this dependency", - }); - } - }, - }), -}; - -export const noUndeferredThirdParty: Rule = { - create: (context: RuleContext) => ({ - JSXOpeningElement(node: EsTreeNode) { - if (node.name?.type !== "JSXIdentifier" || node.name.name !== "script") return; - const attributes = node.attributes ?? []; - if (!findJsxAttribute(attributes, "src")) return; - - if (!hasJsxAttribute(attributes, "defer") && !hasJsxAttribute(attributes, "async")) { - context.report({ - node, - message: - "Synchronous