From ce14ae703c3ab9a55d7a4d0cb9ba8c205eec8696 Mon Sep 17 00:00:00 2001 From: Nisarg Patel Date: Fri, 15 May 2026 02:06:46 -0700 Subject: [PATCH 1/6] fix(nextjs): stop flagging response.headers and local Map/Set/Headers in GET handlers (#206) Rewrites `nextjs-no-side-effect-in-get-handler` precision so it no longer floods Next.js codebases with false positives from `response.headers.set()` and request-scoped `new Map/Set/Headers/URLSearchParams/FormData(...)` mutations, while still catching real CSRF-relevant writes: drizzle/prisma ORM mutations, module-level mutable state, mutating fetch, and all three forms of `cookies().set/delete()` including aliased `const cs = await cookies(); cs.set(...)`. Also folds in the safe handler-resolution improvements from PR #251 (cron route skip and depth-bounded `const GET = withAuth(handler)` resolution). Supersedes #209, #211, #233, #238. Co-authored-by: Cursor --- .../nextjs-get-side-effect-false-positives.md | 46 ++ .../src/plugin/constants/library.ts | 15 + .../src/plugin/constants/nextjs.ts | 2 + .../src/plugin/constants/thresholds.ts | 1 + .../nextjs-no-side-effect-in-get-handler.ts | 242 +++++++-- .../tanstack-start-get-mutation.ts | 9 +- .../collect-locally-scoped-cookie-bindings.ts | 31 ++ .../collect-locally-scoped-safe-bindings.ts | 16 + .../src/plugin/utils/find-side-effect.ts | 116 +++-- .../utils/is-safe-mutable-receiver-source.ts | 69 +++ .../plugin/utils/is-safe-receiver-chain.ts | 38 ++ .../nextjs-app/src/app/api/admin/route.tsx | 22 + .../src/app/api/cron/refresh/route.tsx | 12 + .../src/app/api/documents/[id]/route.tsx | 43 ++ .../nextjs-get-side-effects.test.ts | 482 ++++++++++++++++++ .../tests/run-oxlint/nextjs.test.ts | 29 ++ 16 files changed, 1093 insertions(+), 80 deletions(-) create mode 100644 .changeset/nextjs-get-side-effect-false-positives.md create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/utils/collect-locally-scoped-cookie-bindings.ts create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/utils/collect-locally-scoped-safe-bindings.ts create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/utils/is-safe-mutable-receiver-source.ts create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/utils/is-safe-receiver-chain.ts create mode 100644 packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/admin/route.tsx create mode 100644 packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/cron/refresh/route.tsx create mode 100644 packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/documents/[id]/route.tsx create mode 100644 packages/react-doctor/tests/regressions/nextjs-get-side-effects.test.ts diff --git a/.changeset/nextjs-get-side-effect-false-positives.md b/.changeset/nextjs-get-side-effect-false-positives.md new file mode 100644 index 000000000..8d45da03e --- /dev/null +++ b/.changeset/nextjs-get-side-effect-false-positives.md @@ -0,0 +1,46 @@ +--- +"react-doctor": patch +"eslint-plugin-react-doctor": patch +"oxlint-plugin-react-doctor": patch +--- + +Fix trust-breaking false positives in `nextjs-no-side-effect-in-get-handler` +(issue #206). The rule used to flag any `.()` call as a server-state mutation, +which flooded one Next.js 14 codebase with 138 false errors — every single +one a `response.headers.set(...)` response-shaping call. + +The rule now skips these shapes, all of which only shape the outbound +response or mutate request-scoped collections: + +- `response.headers.set/append/delete(...)` and any chain ending in + `.headers` (e.g. `NextResponse.json({...}).headers.set(...)`, + `(await fetcher()).headers.append(...)`). +- Locally-constructed `new Map/Set/WeakMap/WeakSet/Headers/URLSearchParams/ +FormData/Response/NextResponse(...)` bindings and any mutation on those + aliases. +- `new URL(...).searchParams.set(...)` and any `.searchParams.*()` chain. +- `headers()` / `(await headers())` from `next/headers` (returns + `ReadonlyHeaders`; any mutation would throw at runtime) and any aliased + `const h = headers(); h.get(...)`. +- Route handlers under `/cron/` or `/jobs/cron/` — Vercel Cron always + invokes GET and is expected to do real work. + +The rule still flags real CSRF-relevant side effects: + +- ORM mutations like drizzle `db.update(table).set({...})`, `prisma.user. +create(...)`, `db.insert(...)`, `repository.upsert(...)`. +- Module-level mutable state (`const cache = new Map()` declared outside + the handler, then `cache.set(...)` inside it). +- `fetch(url, { method: "POST" | "PUT" | "DELETE" | "PATCH" })`. +- `cookies().set/append/delete()` in all forms: direct, `(await cookies()). +set(...)`, and aliased `const cookieStore = await cookies(); cookieStore. +set(...)` — the alias resolution now flags previously-missed handlers. +- Mutating route segments (`/logout`, `/signout`, `/unsubscribe`, `/delete`, + …). + +The rule also gained depth-bounded handler-binding resolution so +`export const GET = withAuth(handler)` and `export const GET = app.get('/x', +handler).get('/y', handler)` get scanned correctly. + +Closes #206. Supersedes PRs #209, #211, #233, and #238. diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/constants/library.ts b/packages/oxlint-plugin-react-doctor/src/plugin/constants/library.ts index c3b60d8ee..a492a5177 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/constants/library.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/constants/library.ts @@ -29,3 +29,18 @@ export const MUTATION_METHOD_NAMES = new Set([ ]); export const MUTATING_HTTP_METHODS = new Set(["POST", "PUT", "DELETE", "PATCH"]); + +export const SAFE_MUTABLE_CONSTRUCTOR_NAMES = new Set([ + "Map", + "Set", + "WeakMap", + "WeakSet", + "Headers", + "URLSearchParams", + "FormData", + "Response", + "NextResponse", +]); + +export const RESPONSE_FACTORY_OBJECTS = new Set(["Response", "NextResponse"]); +export const RESPONSE_FACTORY_METHODS = new Set(["json", "redirect", "next", "rewrite", "error"]); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/constants/nextjs.ts b/packages/oxlint-plugin-react-doctor/src/plugin/constants/nextjs.ts index 3db35f43d..0a0ddd604 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/constants/nextjs.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/constants/nextjs.ts @@ -24,6 +24,8 @@ export const APP_DIRECTORY_PATTERN = /\/app\//; export const ROUTE_HANDLER_FILE_PATTERN = /\/route\.(tsx?|jsx?)$/; +export const CRON_ROUTE_PATTERN = /\/(?:cron|jobs\/cron)(?:\/|$)/i; + export const MUTATING_ROUTE_SEGMENTS = new Set([ "logout", "log-out", diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/constants/thresholds.ts b/packages/oxlint-plugin-react-doctor/src/plugin/constants/thresholds.ts index f13c5ef09..8ff0e8d49 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/constants/thresholds.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/constants/thresholds.ts @@ -7,3 +7,4 @@ export const SEQUENTIAL_AWAIT_THRESHOLD = 3; export const PROPERTY_ACCESS_REPEAT_THRESHOLD = 3; export const BOOLEAN_PROP_THRESHOLD = 4; export const RENDER_PROP_PROLIFERATION_THRESHOLD = 3; +export const GET_HANDLER_BINDING_RESOLUTION_DEPTH = 3; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/nextjs/nextjs-no-side-effect-in-get-handler.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/nextjs/nextjs-no-side-effect-in-get-handler.ts index 29c2a8799..b0c7c971f 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/nextjs/nextjs-no-side-effect-in-get-handler.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/nextjs/nextjs-no-side-effect-in-get-handler.ts @@ -1,11 +1,18 @@ -import { MUTATING_ROUTE_SEGMENTS, ROUTE_HANDLER_FILE_PATTERN } from "../../constants/nextjs.js"; +import { + CRON_ROUTE_PATTERN, + MUTATING_ROUTE_SEGMENTS, + ROUTE_HANDLER_FILE_PATTERN, +} from "../../constants/nextjs.js"; +import { GET_HANDLER_BINDING_RESOLUTION_DEPTH } from "../../constants/thresholds.js"; +import { collectLocallyScopedCookieBindings } from "../../utils/collect-locally-scoped-cookie-bindings.js"; +import { collectLocallyScopedSafeBindings } from "../../utils/collect-locally-scoped-safe-bindings.js"; import { defineRule } from "../../utils/define-rule.js"; import { findSideEffect } from "../../utils/find-side-effect.js"; import type { EsTreeNode } from "../../utils/es-tree-node.js"; +import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js"; +import { isNodeOfType } from "../../utils/is-node-of-type.js"; import type { Rule } from "../../utils/rule.js"; import type { RuleContext } from "../../utils/rule-context.js"; -import { isNodeOfType } from "../../utils/is-node-of-type.js"; -import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js"; const extractMutatingRouteSegment = (filename: string): string | null => { const segments = filename.split("/"); @@ -16,30 +23,173 @@ const extractMutatingRouteSegment = (filename: string): string | null => { return null; }; -const getExportedGetHandlerBody = (node: EsTreeNode): EsTreeNode | null => { - if (!isNodeOfType(node, "ExportNamedDeclaration")) return null; +const buildProgramBindingLookup = ( + programNode: EsTreeNode, +): ((identifierName: string) => EsTreeNode | null) => { + const topLevelBindings = new Map(); + if (!isNodeOfType(programNode, "Program")) return () => null; + + const collectFromStatements = (statements: EsTreeNode[]): void => { + for (const statement of statements) { + if (isNodeOfType(statement, "VariableDeclaration")) { + for (const declarator of statement.declarations ?? []) { + if (!isNodeOfType(declarator.id, "Identifier")) continue; + if (!declarator.init) continue; + topLevelBindings.set(declarator.id.name, declarator.init); + } + continue; + } + if ( + isNodeOfType(statement, "FunctionDeclaration") && + isNodeOfType(statement.id, "Identifier") && + statement.body + ) { + topLevelBindings.set(statement.id.name, statement); + continue; + } + if (isNodeOfType(statement, "ExportNamedDeclaration") && statement.declaration) { + collectFromStatements([statement.declaration]); + } + } + }; + + collectFromStatements(programNode.body ?? []); + return (identifierName: string) => topLevelBindings.get(identifierName) ?? null; +}; + +const isExportedGetHandler = (node: EsTreeNode): boolean => { + if (!isNodeOfType(node, "ExportNamedDeclaration")) return false; const declaration = node.declaration; - if (!declaration) return null; + if (!declaration) return false; if (isNodeOfType(declaration, "FunctionDeclaration") && declaration.id?.name === "GET") { - return declaration.body; + return true; } if (isNodeOfType(declaration, "VariableDeclaration")) { for (const declarator of declaration.declarations ?? []) { + if (isNodeOfType(declarator?.id, "Identifier") && declarator.id.name === "GET") { + return true; + } + } + } + + return false; +}; + +const isGetMethodCall = (callExpression: EsTreeNode): boolean => + isNodeOfType(callExpression, "CallExpression") && + isNodeOfType(callExpression.callee, "MemberExpression") && + isNodeOfType(callExpression.callee.property, "Identifier") && + callExpression.callee.property.name === "get"; + +const isStringLikeNode = (node: EsTreeNode): boolean => + (isNodeOfType(node, "Literal") && typeof node.value === "string") || + isNodeOfType(node, "TemplateLiteral"); + +const getHandlerCallbackBody = ( + callExpression: EsTreeNodeOfType<"CallExpression">, +): EsTreeNode | null => { + const callArguments = callExpression.arguments ?? []; + if (callArguments.length < 2) return null; + const routePatternArgument = callArguments[0]; + if (!isStringLikeNode(routePatternArgument)) return null; + const handlerArgument = callArguments[callArguments.length - 1]; + if ( + (isNodeOfType(handlerArgument, "ArrowFunctionExpression") || + isNodeOfType(handlerArgument, "FunctionExpression")) && + handlerArgument.body + ) { + return handlerArgument.body; + } + return null; +}; + +const collectChainedGetHandlerBodies = (initNode: EsTreeNode): EsTreeNode[] => { + const chainedBodies: EsTreeNode[] = []; + let cursor: EsTreeNode | null | undefined = initNode; + while (cursor && isNodeOfType(cursor, "CallExpression")) { + if (isGetMethodCall(cursor)) { + const body = getHandlerCallbackBody(cursor); + if (body) chainedBodies.push(body); + } + cursor = isNodeOfType(cursor.callee, "MemberExpression") ? cursor.callee.object : null; + } + return chainedBodies; +}; + +const resolveBodiesFromExpression = ( + expression: EsTreeNode, + resolveBinding: (identifierName: string) => EsTreeNode | null, + remainingDepth: number, +): EsTreeNode[] => { + if (remainingDepth <= 0) return []; + + if ( + isNodeOfType(expression, "ArrowFunctionExpression") || + isNodeOfType(expression, "FunctionExpression") || + isNodeOfType(expression, "FunctionDeclaration") + ) { + return expression.body ? [expression.body] : []; + } + + if (isNodeOfType(expression, "CallExpression")) { + for (const callArgument of expression.arguments ?? []) { if ( - isNodeOfType(declarator?.id, "Identifier") && - declarator.id.name === "GET" && - declarator.init && - (isNodeOfType(declarator.init, "ArrowFunctionExpression") || - isNodeOfType(declarator.init, "FunctionExpression")) + isNodeOfType(callArgument, "ArrowFunctionExpression") || + isNodeOfType(callArgument, "FunctionExpression") ) { - return declarator.init.body; + if (callArgument.body) return [callArgument.body]; } + if (!isNodeOfType(callArgument, "Identifier")) continue; + const argumentInit = resolveBinding(callArgument.name); + if (!argumentInit) continue; + const resolvedBodies = resolveBodiesFromExpression( + argumentInit, + resolveBinding, + remainingDepth - 1, + ); + if (resolvedBodies.length > 0) return resolvedBodies; + const chainedBodies = collectChainedGetHandlerBodies(argumentInit); + if (chainedBodies.length > 0) return chainedBodies; } + return []; } - return null; + if (isNodeOfType(expression, "Identifier")) { + const boundInit = resolveBinding(expression.name); + if (!boundInit) return []; + return resolveBodiesFromExpression(boundInit, resolveBinding, remainingDepth - 1); + } + + return []; +}; + +const resolveGetHandlerBodies = ( + exportNode: EsTreeNode, + resolveBinding: (identifierName: string) => EsTreeNode | null, +): EsTreeNode[] => { + if (!isNodeOfType(exportNode, "ExportNamedDeclaration")) return []; + const declaration = exportNode.declaration; + if (!declaration) return []; + + if (isNodeOfType(declaration, "FunctionDeclaration") && declaration.id?.name === "GET") { + return declaration.body ? [declaration.body] : []; + } + + if (!isNodeOfType(declaration, "VariableDeclaration")) return []; + + for (const declarator of declaration.declarations ?? []) { + if (!isNodeOfType(declarator.id, "Identifier") || declarator.id.name !== "GET") continue; + if (!declarator.init) return []; + return resolveBodiesFromExpression( + declarator.init, + resolveBinding, + GET_HANDLER_BINDING_RESOLUTION_DEPTH, + ); + } + + return []; }; export const nextjsNoSideEffectInGetHandler = defineRule({ @@ -49,30 +199,44 @@ export const nextjsNoSideEffectInGetHandler = defineRule({ category: "Security", recommendation: "Move the side effect to a POST handler and use a
or fetch with method POST — GET requests can be triggered by prefetching and are vulnerable to CSRF", - create: (context: RuleContext) => ({ - ExportNamedDeclaration(node: EsTreeNodeOfType<"ExportNamedDeclaration">) { - const filename = context.getFilename?.() ?? ""; - if (!ROUTE_HANDLER_FILE_PATTERN.test(filename)) return; - - const handlerBody = getExportedGetHandlerBody(node); - if (!handlerBody) return; - - const mutatingSegment = extractMutatingRouteSegment(filename); - if (mutatingSegment) { - context.report({ - node, - message: `GET handler on "/${mutatingSegment}" route — use POST to prevent CSRF and unintended prefetch triggers`, - }); - return; - } + create: (context: RuleContext) => { + let resolveBinding: (identifierName: string) => EsTreeNode | null = () => null; - const sideEffect = findSideEffect(handlerBody); - if (sideEffect) { - context.report({ - node, - message: `GET handler has side effects (${sideEffect}) — use POST to prevent CSRF and unintended prefetch triggers`, - }); - } - }, - }), + return { + Program(node: EsTreeNodeOfType<"Program">) { + resolveBinding = buildProgramBindingLookup(node); + }, + ExportNamedDeclaration(node: EsTreeNodeOfType<"ExportNamedDeclaration">) { + const filename = context.getFilename?.() ?? ""; + if (!ROUTE_HANDLER_FILE_PATTERN.test(filename)) return; + if (CRON_ROUTE_PATTERN.test(filename)) return; + if (!isExportedGetHandler(node)) return; + + const mutatingSegment = extractMutatingRouteSegment(filename); + if (mutatingSegment) { + context.report({ + node, + message: `GET handler on "/${mutatingSegment}" route — use POST to prevent CSRF and unintended prefetch triggers`, + }); + return; + } + + const handlerBodies = resolveGetHandlerBodies(node, resolveBinding); + for (const handlerBody of handlerBodies) { + const locallyScopedSafeBindings = collectLocallyScopedSafeBindings(handlerBody); + const locallyScopedCookieBindings = collectLocallyScopedCookieBindings(handlerBody); + const sideEffect = findSideEffect(handlerBody, { + locallyScopedSafeBindings, + locallyScopedCookieBindings, + }); + if (!sideEffect) continue; + context.report({ + node, + message: `GET handler has side effects (${sideEffect}) — use POST to prevent CSRF and unintended prefetch triggers`, + }); + return; + } + }, + }; + }, }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/tanstack-start/tanstack-start-get-mutation.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/tanstack-start/tanstack-start-get-mutation.ts index b210f0458..0ae2160fc 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/tanstack-start/tanstack-start-get-mutation.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/tanstack-start/tanstack-start-get-mutation.ts @@ -1,4 +1,6 @@ import { MUTATING_HTTP_METHODS } from "../../constants/library.js"; +import { collectLocallyScopedCookieBindings } from "../../utils/collect-locally-scoped-cookie-bindings.js"; +import { collectLocallyScopedSafeBindings } from "../../utils/collect-locally-scoped-safe-bindings.js"; import { defineRule } from "../../utils/define-rule.js"; import { findSideEffect } from "../../utils/find-side-effect.js"; import type { Rule } from "../../utils/rule.js"; @@ -34,7 +36,12 @@ export const tanstackStartGetMutation = defineRule({ const handlerFunction = node.arguments?.[0]; if (!handlerFunction) return; - const sideEffect = findSideEffect(handlerFunction); + const locallyScopedSafeBindings = collectLocallyScopedSafeBindings(handlerFunction); + const locallyScopedCookieBindings = collectLocallyScopedCookieBindings(handlerFunction); + const sideEffect = findSideEffect(handlerFunction, { + locallyScopedSafeBindings, + locallyScopedCookieBindings, + }); if (sideEffect) { context.report({ node, diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/utils/collect-locally-scoped-cookie-bindings.ts b/packages/oxlint-plugin-react-doctor/src/plugin/utils/collect-locally-scoped-cookie-bindings.ts new file mode 100644 index 000000000..0eced72d1 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/utils/collect-locally-scoped-cookie-bindings.ts @@ -0,0 +1,31 @@ +import { collectPatternNames } from "./collect-pattern-names.js"; +import type { EsTreeNode } from "./es-tree-node.js"; +import { isNodeOfType } from "./is-node-of-type.js"; +import { walkInsideStatementBlocks } from "./walk-inside-statement-blocks.js"; + +const isCookiesCall = (node: EsTreeNode): boolean => + isNodeOfType(node, "CallExpression") && + isNodeOfType(node.callee, "Identifier") && + node.callee.name === "cookies"; + +// `cookies()` became async in Next.js 15, so users now write +// `const cookieStore = await cookies()`. We unwrap one `await` so both +// the sync and async aliasing forms collapse to the same detection. +const isCookiesInit = (initNode: EsTreeNode): boolean => { + if (isCookiesCall(initNode)) return true; + if (isNodeOfType(initNode, "AwaitExpression") && initNode.argument) { + return isCookiesCall(initNode.argument); + } + return false; +}; + +export const collectLocallyScopedCookieBindings = (handlerBody: EsTreeNode): Set => { + const cookieBindingNames = new Set(); + walkInsideStatementBlocks(handlerBody, (node: EsTreeNode) => { + if (!isNodeOfType(node, "VariableDeclarator")) return; + if (!node.init) return; + if (!isCookiesInit(node.init)) return; + collectPatternNames(node.id, cookieBindingNames); + }); + return cookieBindingNames; +}; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/utils/collect-locally-scoped-safe-bindings.ts b/packages/oxlint-plugin-react-doctor/src/plugin/utils/collect-locally-scoped-safe-bindings.ts new file mode 100644 index 000000000..9a768408c --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/utils/collect-locally-scoped-safe-bindings.ts @@ -0,0 +1,16 @@ +import { collectPatternNames } from "./collect-pattern-names.js"; +import type { EsTreeNode } from "./es-tree-node.js"; +import { isNodeOfType } from "./is-node-of-type.js"; +import { isSafeMutableReceiverSource } from "./is-safe-mutable-receiver-source.js"; +import { walkInsideStatementBlocks } from "./walk-inside-statement-blocks.js"; + +export const collectLocallyScopedSafeBindings = (handlerBody: EsTreeNode): Set => { + const safeBindingNames = new Set(); + walkInsideStatementBlocks(handlerBody, (node: EsTreeNode) => { + if (!isNodeOfType(node, "VariableDeclarator")) return; + if (!node.init) return; + if (!isSafeMutableReceiverSource(node.init)) return; + collectPatternNames(node.id, safeBindingNames); + }); + return safeBindingNames; +}; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/utils/find-side-effect.ts b/packages/oxlint-plugin-react-doctor/src/plugin/utils/find-side-effect.ts index f6b2b34c2..df0a317a5 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/utils/find-side-effect.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/utils/find-side-effect.ts @@ -2,6 +2,16 @@ import { MUTATING_HTTP_METHODS, MUTATION_METHOD_NAMES } from "../constants/libra import type { EsTreeNode } from "./es-tree-node.js"; import { walkAst } from "./walk-ast.js"; import { isNodeOfType } from "./is-node-of-type.js"; +import { isSafeReceiverChain } from "./is-safe-receiver-chain.js"; + +export interface FindSideEffectOptions { + locallyScopedSafeBindings?: Set; + locallyScopedCookieBindings?: Set; +} + +const EMPTY_BINDING_SET = new Set(); + +const COOKIE_MUTATION_METHOD_NAMES = new Set(["set", "append", "delete"]); // HACK: extracted so `findSideEffect` can re-use the EXACT same shape // predicate when it goes hunting for the literal method to render in @@ -17,22 +27,38 @@ const isMutatingMethodProperty = (property: EsTreeNode): boolean => typeof property.value.value === "string" && MUTATING_HTTP_METHODS.has(property.value.value.toUpperCase()); -const isCookiesOrHeadersCall = (node: EsTreeNode, methodName: string): boolean => { - if (!isNodeOfType(node, "CallExpression") || !isNodeOfType(node.callee, "MemberExpression")) - return false; - const { object, property } = node.callee; - if (!isNodeOfType(property, "Identifier") || !MUTATION_METHOD_NAMES.has(property.name)) - return false; - if (!isNodeOfType(object, "CallExpression") || !isNodeOfType(object.callee, "Identifier")) - return false; - return object.callee.name === methodName; +const isDirectCookiesCall = (node: EsTreeNode): boolean => + isNodeOfType(node, "CallExpression") && + isNodeOfType(node.callee, "Identifier") && + node.callee.name === "cookies"; + +const isAwaitedCookiesCall = (node: EsTreeNode): boolean => + isNodeOfType(node, "AwaitExpression") && node.argument + ? isDirectCookiesCall(node.argument) + : false; + +const isCookieReceiver = ( + receiverNode: EsTreeNode, + locallyScopedCookieBindings: Set, +): boolean => { + if (isDirectCookiesCall(receiverNode)) return true; + if (isAwaitedCookiesCall(receiverNode)) return true; + if (isNodeOfType(receiverNode, "Identifier")) { + return locallyScopedCookieBindings.has(receiverNode.name); + } + return false; }; -const isMutatingDbCall = (node: EsTreeNode): boolean => { - if (!isNodeOfType(node, "CallExpression") || !isNodeOfType(node.callee, "MemberExpression")) - return false; - const { property } = node.callee; - return isNodeOfType(property, "Identifier") && MUTATION_METHOD_NAMES.has(property.name); +const getCookieMutationMethodName = ( + node: EsTreeNode, + locallyScopedCookieBindings: Set, +): string | null => { + if (!isNodeOfType(node, "CallExpression")) return null; + if (!isNodeOfType(node.callee, "MemberExpression")) return null; + if (!isNodeOfType(node.callee.property, "Identifier")) return null; + if (!COOKIE_MUTATION_METHOD_NAMES.has(node.callee.property.name)) return null; + if (!isCookieReceiver(node.callee.object, locallyScopedCookieBindings)) return null; + return node.callee.property.name; }; const isMutatingFetchCall = (node: EsTreeNode): boolean => { @@ -43,31 +69,44 @@ const isMutatingFetchCall = (node: EsTreeNode): boolean => { return Boolean(optionsArgument.properties?.some(isMutatingMethodProperty)); }; -const getCookiesOrHeadersMethodName = ( - child: EsTreeNode, - apiName: "cookies" | "headers", -): string | null => { - if (!isCookiesOrHeadersCall(child, apiName)) return null; - if (!isNodeOfType(child, "CallExpression")) return null; - if (!isNodeOfType(child.callee, "MemberExpression")) return null; - if (!isNodeOfType(child.callee.property, "Identifier")) return null; - return child.callee.property.name; +const isMutatingDbCall = (node: EsTreeNode, locallyScopedSafeBindings: Set): boolean => { + if (!isNodeOfType(node, "CallExpression") || !isNodeOfType(node.callee, "MemberExpression")) + return false; + const { property, object } = node.callee; + if (!isNodeOfType(property, "Identifier") || !MUTATION_METHOD_NAMES.has(property.name)) + return false; + if (isSafeReceiverChain(object, locallyScopedSafeBindings)) return false; + return true; }; -export const findSideEffect = (node: EsTreeNode): string | null => { +const getDbCallDescription = (node: EsTreeNode): string => { + if (!isNodeOfType(node, "CallExpression")) return ".unknown()"; + if (!isNodeOfType(node.callee, "MemberExpression")) return ".unknown()"; + if (!isNodeOfType(node.callee.property, "Identifier")) return ".unknown()"; + const methodName = node.callee.property.name; + const rootObjectName = isNodeOfType(node.callee.object, "Identifier") + ? node.callee.object.name + : null; + return rootObjectName ? `${rootObjectName}.${methodName}()` : `.${methodName}()`; +}; + +export const findSideEffect = ( + node: EsTreeNode, + options: FindSideEffectOptions = {}, +): string | null => { + const locallyScopedSafeBindings = options.locallyScopedSafeBindings ?? EMPTY_BINDING_SET; + const locallyScopedCookieBindings = options.locallyScopedCookieBindings ?? EMPTY_BINDING_SET; + let sideEffectDescription: string | null = null; walkAst(node, (child: EsTreeNode) => { if (sideEffectDescription) return; - const cookiesMethodName = getCookiesOrHeadersMethodName(child, "cookies"); - if (cookiesMethodName) { - sideEffectDescription = `cookies().${cookiesMethodName}()`; - return; - } - const headersMethodName = getCookiesOrHeadersMethodName(child, "headers"); - if (headersMethodName) { - sideEffectDescription = `headers().${headersMethodName}()`; + + const cookieMethodName = getCookieMutationMethodName(child, locallyScopedCookieBindings); + if (cookieMethodName) { + sideEffectDescription = `cookies().${cookieMethodName}()`; return; } + if (isMutatingFetchCall(child) && isNodeOfType(child, "CallExpression")) { // HACK: re-use the EXACT predicate `isMutatingFetchCall` already // matched on so we can't pick a non-Literal duplicate `method:` @@ -83,14 +122,11 @@ export const findSideEffect = (node: EsTreeNode): string | null => { ) return; sideEffectDescription = `fetch() with method ${methodProperty.value.value}`; - } else if (isMutatingDbCall(child) && isNodeOfType(child, "CallExpression")) { - if (!isNodeOfType(child.callee, "MemberExpression")) return; - if (!isNodeOfType(child.callee.property, "Identifier")) return; - const methodName = child.callee.property.name; - const objectName = isNodeOfType(child.callee.object, "Identifier") - ? child.callee.object.name - : null; - sideEffectDescription = objectName ? `${objectName}.${methodName}()` : `.${methodName}()`; + return; + } + + if (isMutatingDbCall(child, locallyScopedSafeBindings)) { + sideEffectDescription = getDbCallDescription(child); } }); return sideEffectDescription; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-safe-mutable-receiver-source.ts b/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-safe-mutable-receiver-source.ts new file mode 100644 index 000000000..9fc5c0871 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-safe-mutable-receiver-source.ts @@ -0,0 +1,69 @@ +import { + RESPONSE_FACTORY_METHODS, + RESPONSE_FACTORY_OBJECTS, + SAFE_MUTABLE_CONSTRUCTOR_NAMES, +} from "../constants/library.js"; +import type { EsTreeNode } from "./es-tree-node.js"; +import { isNodeOfType } from "./is-node-of-type.js"; + +const SAFE_INTRINSIC_PROPERTY_NAMES = new Set(["headers", "searchParams"]); + +const unwrapAwait = (node: EsTreeNode): EsTreeNode => + isNodeOfType(node, "AwaitExpression") && node.argument ? node.argument : node; + +const isSafeConstructorNew = (node: EsTreeNode): boolean => + isNodeOfType(node, "NewExpression") && + isNodeOfType(node.callee, "Identifier") && + SAFE_MUTABLE_CONSTRUCTOR_NAMES.has(node.callee.name); + +const isResponseFactoryCall = (node: EsTreeNode): boolean => { + if (!isNodeOfType(node, "CallExpression")) return false; + if (!isNodeOfType(node.callee, "MemberExpression")) return false; + if (!isNodeOfType(node.callee.object, "Identifier")) return false; + if (!isNodeOfType(node.callee.property, "Identifier")) return false; + return ( + RESPONSE_FACTORY_OBJECTS.has(node.callee.object.name) && + RESPONSE_FACTORY_METHODS.has(node.callee.property.name) + ); +}; + +// `headers()` from `next/headers` returns a ReadonlyHeaders — every +// mutating call would throw at runtime, so it can never represent +// server-state mutation. Including aliases (`const h = headers()`) here +// makes them get collected by `collectLocallyScopedSafeBindings` and +// silently skipped by the rule. +const isHeadersFunctionCall = (node: EsTreeNode): boolean => + isNodeOfType(node, "CallExpression") && + isNodeOfType(node.callee, "Identifier") && + node.callee.name === "headers"; + +// HACK: `something.headers` is always a Headers instance, `something.searchParams` +// is always a URLSearchParams. Catching these by property name lets us +// short-circuit without resolving the receiver's actual type — handles +// the issue #206 shape `await v2GET(req, ctx); res.headers.set(...)` where +// we have no way to know that `v2GET` returns a Response. +const isSafeIntrinsicMemberAccess = (node: EsTreeNode): boolean => + isNodeOfType(node, "MemberExpression") && + isNodeOfType(node.property, "Identifier") && + SAFE_INTRINSIC_PROPERTY_NAMES.has(node.property.name); + +export const isSafeMutableReceiverSource = (initNode: EsTreeNode): boolean => { + const unwrapped = unwrapAwait(initNode); + if (isSafeConstructorNew(unwrapped)) return true; + if (isResponseFactoryCall(unwrapped)) return true; + if (isSafeIntrinsicMemberAccess(unwrapped)) return true; + if (isHeadersFunctionCall(unwrapped)) return true; + return false; +}; + +export const isSafeReceiverChainNode = ( + node: EsTreeNode, + locallyScopedSafeBindings: Set, +): boolean => { + if (isSafeConstructorNew(node)) return true; + if (isResponseFactoryCall(node)) return true; + if (isSafeIntrinsicMemberAccess(node)) return true; + if (isHeadersFunctionCall(node)) return true; + if (isNodeOfType(node, "Identifier") && locallyScopedSafeBindings.has(node.name)) return true; + return false; +}; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-safe-receiver-chain.ts b/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-safe-receiver-chain.ts new file mode 100644 index 000000000..69b573f68 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-safe-receiver-chain.ts @@ -0,0 +1,38 @@ +import type { EsTreeNode } from "./es-tree-node.js"; +import { isNodeOfType } from "./is-node-of-type.js"; +import { isSafeReceiverChainNode } from "./is-safe-mutable-receiver-source.js"; + +// HACK: walks down `callee.object` / `argument` / `expression` looking +// for ANY ancestor in the receiver chain that is structurally "safe" +// (a Headers/Map/Set construction, a Response factory, a `.headers` +// access, a locally-scoped safe binding, etc.). Catching at any +// depth lets us cover `NextResponse.json({...}).headers.set(...)`, +// `(await someAsync()).headers.append(...)`, and parenthesized chains +// without a bunch of per-shape predicates. +export const isSafeReceiverChain = ( + receiverNode: EsTreeNode | null | undefined, + locallyScopedSafeBindings: Set, +): boolean => { + let current: EsTreeNode | null | undefined = receiverNode; + while (current) { + if (isSafeReceiverChainNode(current, locallyScopedSafeBindings)) return true; + if (isNodeOfType(current, "MemberExpression")) { + current = current.object; + continue; + } + if (isNodeOfType(current, "ChainExpression")) { + current = current.expression; + continue; + } + if (isNodeOfType(current, "AwaitExpression")) { + current = current.argument; + continue; + } + if (isNodeOfType(current, "TSNonNullExpression") || isNodeOfType(current, "TSAsExpression")) { + current = current.expression; + continue; + } + return false; + } + return false; +}; diff --git a/packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/admin/route.tsx b/packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/admin/route.tsx new file mode 100644 index 000000000..73e77fbdb --- /dev/null +++ b/packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/admin/route.tsx @@ -0,0 +1,22 @@ +import { cookies } from "next/headers"; +import { NextResponse } from "next/server"; + +declare const db: { + update: (table: unknown) => { set: (values: unknown) => { where: (cond: unknown) => unknown } }; + insert: (values: unknown) => unknown; +}; +declare const usersTable: unknown; +declare const eq: (a: unknown, b: unknown) => unknown; + +// Module-level cache — mutating it from a GET handler IS a real +// side effect (server state leaks across requests). +const cache = new Map(); + +export async function GET() { + cache.set("hit", Date.now()); + db.update(usersTable).set({ active: false }).where(eq("id", 1)); + await fetch("/api/notify", { method: "POST", body: "x" }); + const cookieHandler = await cookies(); + cookieHandler.set("session", "token"); + return NextResponse.json({ ok: true }); +} diff --git a/packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/cron/refresh/route.tsx b/packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/cron/refresh/route.tsx new file mode 100644 index 000000000..f36a6eb62 --- /dev/null +++ b/packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/cron/refresh/route.tsx @@ -0,0 +1,12 @@ +import { NextResponse } from "next/server"; + +declare const db: { + insert: (values: unknown) => Promise; +}; + +// Vercel Cron always invokes GET — real side effects are expected +// and the rule must not fire here. +export async function GET() { + await db.insert({ refreshedAt: Date.now() }); + return NextResponse.json({ ok: true }); +} diff --git a/packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/documents/[id]/route.tsx b/packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/documents/[id]/route.tsx new file mode 100644 index 000000000..a78ee46bf --- /dev/null +++ b/packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/documents/[id]/route.tsx @@ -0,0 +1,43 @@ +import { headers } from "next/headers"; +import { NextResponse } from "next/server"; +import type { NextRequest } from "next/server"; + +interface RouteContext { + params: Promise<{ id: string }>; +} + +declare const v2GET: (req: NextRequest, ctx: RouteContext) => Promise; + +// Verbatim repro from issue #206 — used to flood codebases with false +// positives. Response header shaping must not fire the rule. +export async function GET(req: NextRequest, ctx: RouteContext) { + const res = await v2GET(req, ctx); + res.headers.set("X-Deprecated", "Use /api/v2/documents/[id]"); + res.headers.append("Vary", "Cookie"); + res.headers.delete("X-Cache"); + + const customHeaders = new Headers(); + customHeaders.set("Content-Type", "application/json"); + customHeaders.append("X-Trace", "abc"); + customHeaders.delete("X-Internal"); + + const lookup = new Map(); + lookup.set("id", "value"); + + const seen = new Set(); + seen.add("id"); + + const params = new URL(req.url).searchParams; + params.set("limit", "10"); + + const formData = new FormData(); + formData.set("file", "blob"); + + const response = NextResponse.json({ ok: true }); + response.headers.set("X-Trace", "abc"); + + const readonly = await headers(); + readonly.get("user-agent"); + + return NextResponse.json({ ok: true }).headers ? NextResponse.json({ ok: true }) : response; +} diff --git a/packages/react-doctor/tests/regressions/nextjs-get-side-effects.test.ts b/packages/react-doctor/tests/regressions/nextjs-get-side-effects.test.ts new file mode 100644 index 000000000..182c69d65 --- /dev/null +++ b/packages/react-doctor/tests/regressions/nextjs-get-side-effects.test.ts @@ -0,0 +1,482 @@ +/** + * Regression tests for `nextjs-no-side-effect-in-get-handler` — issue #206. + * + * The rule used to treat ANY `.()` call as a server-state mutation. In a + * Next.js 14 codebase that flooded `src/app/api/**\/route.ts` with 138 + * false-positive errors, every single one a `response.headers.set(...)` + * response-shaping call. + * + * This file pins down: + * - 13 false-negative shapes that must produce ZERO hits (the headers / + * local-Map / cron-route / aliased-`headers()` cases), + * - 9 true-positive shapes that MUST still fire (drizzle, module-level + * caches, aliased `cookies()`, mutating fetch), + * - a 138x stress test that mirrors the original issue. + */ + +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterAll, describe, expect, it } from "vite-plus/test"; + +import { runOxlint } from "@react-doctor/core"; +import type { Diagnostic, ProjectInfo } from "@react-doctor/types"; +import { buildTestProject, setupReactProject, writeFile } from "./_helpers.js"; + +const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "rd-nextjs-get-side-effects-")); + +afterAll(() => { + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +const RULE_NAME = "nextjs-no-side-effect-in-get-handler"; + +const buildNextjsProject = (caseId: string): { rootDirectory: string; project: ProjectInfo } => { + const rootDirectory = setupReactProject(tempRoot, caseId); + return { + rootDirectory, + project: buildTestProject({ rootDirectory, framework: "nextjs" }), + }; +}; + +const writeRouteAndLint = async ( + caseId: string, + routePath: string, + routeSource: string, +): Promise => { + const { rootDirectory, project } = buildNextjsProject(caseId); + writeFile(path.join(rootDirectory, routePath), routeSource); + return runOxlint({ rootDirectory, project }); +}; + +const filterRule = (diagnostics: Diagnostic[]): Diagnostic[] => + diagnostics.filter((diagnostic) => diagnostic.rule === RULE_NAME); + +describe("issue #206: nextjs-no-side-effect-in-get-handler false positives", () => { + describe("does NOT fire on", () => { + it("the verbatim repro: `res.headers.set('X-Deprecated', ...)`", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-verbatim", + "src/app/api/documents/[id]/route.ts", + `import type { NextRequest } from "next/server"; + +interface RouteContext { params: Promise<{ id: string }> } +declare const v2GET: (req: NextRequest, ctx: RouteContext) => Promise; + +export async function GET(req: NextRequest, ctx: RouteContext) { + const res = await v2GET(req, ctx); + res.headers.set("X-Deprecated", "Use /api/v2/documents/[id]"); + return res; +} +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + + it("response.headers.append + response.headers.delete", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-append-delete", + "src/app/api/orders/route.ts", + `import { NextResponse } from "next/server"; + +declare const upstream: () => Promise; + +export async function GET() { + const response = await upstream(); + response.headers.append("Vary", "Cookie"); + response.headers.delete("X-Cache"); + return response ?? NextResponse.json({ ok: true }); +} +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + + it("chained NextResponse.json({...}).headers.set(...)", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-chained-nextresponse", + "src/app/api/health/route.ts", + `import { NextResponse } from "next/server"; + +export async function GET() { + const response = NextResponse.json({ ok: true }); + response.headers.set("Cache-Control", "public, max-age=60"); + return response; +} +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + + it("locally-constructed `new Headers()` and `.set/.append/.delete`", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-local-headers", + "src/app/api/headers/route.ts", + `import { NextResponse } from "next/server"; + +export async function GET() { + const customHeaders = new Headers(); + customHeaders.set("x", "1"); + customHeaders.append("y", "2"); + customHeaders.delete("z"); + return new NextResponse(null, { headers: customHeaders }); +} +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + + it("locally-constructed `new Map()` and `new Set()`", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-local-map-set", + "src/app/api/dedupe/route.ts", + `import { NextResponse } from "next/server"; + +export async function GET(req: Request) { + const cache = new Map(); + cache.set(req.url, "hit"); + const seen = new Set(); + seen.add(req.url); + return NextResponse.json({ ok: true }); +} +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + + it("`new URL(...).searchParams.set(...)`", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-search-params", + "src/app/api/search/route.ts", + `import { NextResponse } from "next/server"; + +export async function GET(req: Request) { + const params = new URL(req.url).searchParams; + params.set("limit", "10"); + return NextResponse.json({ params: params.toString() }); +} +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + + it("locally-constructed `new URLSearchParams()` and `.append`", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-urlsearchparams", + "src/app/api/qs/route.ts", + `import { NextResponse } from "next/server"; + +export async function GET() { + const params = new URLSearchParams(); + params.append("q", "next"); + return NextResponse.json({ qs: params.toString() }); +} +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + + it("locally-constructed `new FormData()` and `.set`", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-formdata", + "src/app/api/upload/route.ts", + `import { NextResponse } from "next/server"; + +export async function GET() { + const formData = new FormData(); + formData.set("file", "blob"); + return NextResponse.json({ size: formData.has("file") }); +} +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + + it("`const response = NextResponse.json(...); response.headers.set(...)`", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-response-alias", + "src/app/api/trace/route.ts", + `import { NextResponse } from "next/server"; + +declare const traceId: string; + +export async function GET() { + const response = NextResponse.json({ ok: true }); + response.headers.set("X-Trace", traceId); + return response; +} +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + + it("`const response = new NextResponse(stream); response.headers.set(...)`", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-new-nextresponse", + "src/app/api/stream/route.ts", + `import { NextResponse } from "next/server"; + +declare const stream: ReadableStream; + +export async function GET() { + const response = new NextResponse(stream); + response.headers.set("Content-Type", "text/event-stream"); + return response; +} +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + + it("`headers().set(...)` and `(await headers()).set(...)` (ReadonlyHeaders — would throw, never a server-state write)", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-readonly-headers", + "src/app/api/echo/route.ts", + `import { headers } from "next/headers"; +import { NextResponse } from "next/server"; + +export async function GET() { + headers().set("x", "y"); + (await headers()).set("a", "b"); + return NextResponse.json({ ok: true }); +} +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + + it("aliased read-only `headers()` — `const h = headers(); h.get('user-agent')`", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-headers-alias-read", + "src/app/api/ua/route.ts", + `import { headers } from "next/headers"; +import { NextResponse } from "next/server"; + +export async function GET() { + const h = headers(); + const userAgent = h.get("user-agent"); + return NextResponse.json({ userAgent }); +} +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + + it("cron route handler with a real `db.insert(...)` is skipped via CRON_ROUTE_PATTERN", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-cron-skip", + "src/app/api/cron/refresh/route.ts", + `import { NextResponse } from "next/server"; + +declare const db: { insert: (values: unknown) => Promise }; + +export async function GET() { + await db.insert({ refreshedAt: Date.now() }); + return NextResponse.json({ ok: true }); +} +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + }); + + describe("DOES still fire on", () => { + it("drizzle `db.update(table).set({...}).where(...)` from the @Sparticuz comment", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-drizzle", + "src/app/api/users/route.ts", + `import { NextResponse } from "next/server"; + +declare const db: { update: (t: unknown) => { set: (v: unknown) => { where: (c: unknown) => unknown } } }; +declare const usersTable: unknown; +declare const eq: (a: unknown, b: unknown) => unknown; + +export async function GET() { + db.update(usersTable).set({ active: false }).where(eq("id", 1)); + return NextResponse.json({ ok: true }); +} +`, + ); + const hits = filterRule(diagnostics); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain(".set()"); + }); + + it("`prisma.user.create({...})`", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-prisma-create", + "src/app/api/signup/route.ts", + `import { NextResponse } from "next/server"; + +declare const prisma: { user: { create: (args: unknown) => Promise } }; + +export async function GET() { + await prisma.user.create({ data: { name: "x" } }); + return NextResponse.json({ ok: true }); +} +`, + ); + const hits = filterRule(diagnostics); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain(".create()"); + }); + + it("module-level `const cache = new Map()` mutated from inside the handler", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-module-cache", + "src/app/api/track/route.ts", + `import { NextResponse } from "next/server"; + +const cache = new Map(); + +export async function GET(req: Request) { + cache.set(req.url, Date.now()); + return NextResponse.json({ ok: true }); +} +`, + ); + const hits = filterRule(diagnostics); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("cache.set()"); + }); + + it("mutating `fetch('/api/notify', { method: 'POST' })`", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-fetch-post", + "src/app/api/notify/route.ts", + `import { NextResponse } from "next/server"; + +export async function GET() { + await fetch("/api/notify", { method: "POST", body: "x" }); + return NextResponse.json({ ok: true }); +} +`, + ); + const hits = filterRule(diagnostics); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("fetch() with method POST"); + }); + + it("direct `cookies().set('session', token, ...)`", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-cookies-set", + "src/app/api/login/route.ts", + `import { cookies } from "next/headers"; +import { NextResponse } from "next/server"; + +declare const token: string; + +export async function GET() { + cookies().set("session", token, { httpOnly: true }); + return NextResponse.json({ ok: true }); +} +`, + ); + const hits = filterRule(diagnostics); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("cookies().set()"); + }); + + it("`(await cookies()).delete('session')` (Next.js 15 async form)", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-cookies-await-delete", + "src/app/api/end-session/route.ts", + `import { cookies } from "next/headers"; +import { NextResponse } from "next/server"; + +export async function GET() { + (await cookies()).delete("session"); + return NextResponse.json({ ok: true }); +} +`, + ); + const hits = filterRule(diagnostics); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("cookies().delete()"); + }); + + it("aliased cookies (sync): `const cookieHandler = cookies(); cookieHandler.set('a', 'b')`", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-cookies-alias-sync", + "src/app/api/bookmark/route.ts", + `import { cookies } from "next/headers"; +import { NextResponse } from "next/server"; + +export async function GET() { + const cookieHandler = cookies(); + cookieHandler.set("a", "b"); + return NextResponse.json({ ok: true }); +} +`, + ); + const hits = filterRule(diagnostics); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("cookies().set()"); + }); + + it("aliased cookies (async): `const cs = await cookies(); cs.delete('session')`", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-cookies-alias-async", + "src/app/api/clear/route.ts", + `import { cookies } from "next/headers"; +import { NextResponse } from "next/server"; + +export async function GET() { + const cs = await cookies(); + cs.delete("session"); + return NextResponse.json({ ok: true }); +} +`, + ); + const hits = filterRule(diagnostics); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("cookies().delete()"); + }); + + it("mutating route segment `/logout` still fires on its own", async () => { + const diagnostics = await writeRouteAndLint( + "issue-206-mutating-segment", + "src/app/logout/route.ts", + `import { NextResponse } from "next/server"; + +export async function GET() { + return NextResponse.json({ ok: true }); +} +`, + ); + const hits = filterRule(diagnostics); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("/logout"); + }); + }); + + describe("stress: synthesised 138x response.headers.set codebase", () => { + it("produces 0 hits across 138 route files (was 138 in the original report)", async () => { + const { rootDirectory, project } = buildNextjsProject("issue-206-138x-stress"); + + const ROUTE_COUNT = 138; + for (let routeIndex = 0; routeIndex < ROUTE_COUNT; routeIndex++) { + const routeSource = `import type { NextRequest } from "next/server"; +import { NextResponse } from "next/server"; + +interface RouteContext { params: Promise<{ id: string }> } +declare const v2GET${routeIndex}: (req: NextRequest, ctx: RouteContext) => Promise; + +export async function GET(req: NextRequest, ctx: RouteContext) { + const res = await v2GET${routeIndex}(req, ctx); + res.headers.set("X-Deprecated", "Use /api/v2/route-${routeIndex}"); + return res ?? NextResponse.json({ ok: true }); +} +`; + writeFile( + path.join(rootDirectory, `src/app/api/route-${routeIndex}/route.ts`), + routeSource, + ); + } + + const diagnostics = await runOxlint({ rootDirectory, project }); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + }); +}); diff --git a/packages/react-doctor/tests/run-oxlint/nextjs.test.ts b/packages/react-doctor/tests/run-oxlint/nextjs.test.ts index e5fab5207..3c31841f4 100644 --- a/packages/react-doctor/tests/run-oxlint/nextjs.test.ts +++ b/packages/react-doctor/tests/run-oxlint/nextjs.test.ts @@ -64,6 +64,35 @@ describe("runOxlint", () => { }); }); + describe("nextjs-no-side-effect-in-get-handler scope", () => { + it("does not fire on response.headers.set/append/delete or local Map/Set/Headers/URLSearchParams", () => { + const documentsRouteIssues = nextjsDiagnostics.filter( + (diagnostic) => + diagnostic.rule === "nextjs-no-side-effect-in-get-handler" && + diagnostic.filePath.includes("api/documents"), + ); + expect(documentsRouteIssues).toHaveLength(0); + }); + + it("does not fire on cron route handlers", () => { + const cronRouteIssues = nextjsDiagnostics.filter( + (diagnostic) => + diagnostic.rule === "nextjs-no-side-effect-in-get-handler" && + diagnostic.filePath.includes("api/cron"), + ); + expect(cronRouteIssues).toHaveLength(0); + }); + + it("still flags drizzle, module-level cache, mutating fetch, and aliased cookies in admin route", () => { + const adminRouteIssues = nextjsDiagnostics.filter( + (diagnostic) => + diagnostic.rule === "nextjs-no-side-effect-in-get-handler" && + diagnostic.filePath.includes("api/admin"), + ); + expect(adminRouteIssues.length).toBeGreaterThan(0); + }); + }); + describeRules( "nextjs rules", { From 5d631c96acdc5f5ce0b07cb737410ed6e236c18b Mon Sep 17 00:00:00 2001 From: Nisarg Patel Date: Fri, 15 May 2026 02:17:08 -0700 Subject: [PATCH 2/6] fr --- .../src/plugin/utils/is-headers-property-access.ts | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/utils/is-headers-property-access.ts diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-headers-property-access.ts b/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-headers-property-access.ts new file mode 100644 index 000000000..057d4aef3 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-headers-property-access.ts @@ -0,0 +1,7 @@ +import type { EsTreeNode } from "./es-tree-node.js"; +import { isNodeOfType } from "./is-node-of-type.js"; + +export const isHeadersPropertyAccess = (node: EsTreeNode): boolean => + isNodeOfType(node, "MemberExpression") && + isNodeOfType(node.property, "Identifier") && + node.property.name === "headers"; From a91c466cc1785c638fe1f60eac6305379114b3ce Mon Sep 17 00:00:00 2001 From: Nisarg Patel Date: Fri, 15 May 2026 02:22:12 -0700 Subject: [PATCH 3/6] chore: drop unused is-headers-property-access util MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `isHeadersPropertyAccess` predicate was added during the issue #206 work but never imported — its check (`.headers` member access) is fully covered by `isSafeIntrinsicMemberAccess` in `is-safe-mutable-receiver- source.ts` (which also handles `.searchParams`). Removing the dead file to comply with the workspace "remove unused code and don't repeat yourself" rule. Co-authored-by: Cursor --- .../src/plugin/utils/is-headers-property-access.ts | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/utils/is-headers-property-access.ts diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-headers-property-access.ts b/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-headers-property-access.ts deleted file mode 100644 index 057d4aef3..000000000 --- a/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-headers-property-access.ts +++ /dev/null @@ -1,7 +0,0 @@ -import type { EsTreeNode } from "./es-tree-node.js"; -import { isNodeOfType } from "./is-node-of-type.js"; - -export const isHeadersPropertyAccess = (node: EsTreeNode): boolean => - isNodeOfType(node, "MemberExpression") && - isNodeOfType(node.property, "Identifier") && - node.property.name === "headers"; From eb1e10f27c0bfb891743dd28abdabcaae33f1cd2 Mon Sep 17 00:00:00 2001 From: Nisarg Patel Date: Fri, 15 May 2026 02:23:12 -0700 Subject: [PATCH 4/6] refactor: extract shared is-cookies-call / is-cookies-or-awaited-cookies-call utils `isCookiesCall` in `collect-locally-scoped-cookie-bindings.ts` and `isDirectCookiesCall` in `find-side-effect.ts` were byte-identical, and `isCookiesInit` was structurally the same as `(direct || awaited)` from `find-side-effect.ts`. Extract to single-purpose utils (one per file, per workspace convention) and rewire both consumers so the cookies-detection predicate has one definition. Co-authored-by: Cursor --- .../collect-locally-scoped-cookie-bindings.ts | 19 ++----------------- .../src/plugin/utils/find-side-effect.ts | 16 +++------------- .../src/plugin/utils/is-cookies-call.ts | 7 +++++++ .../is-cookies-or-awaited-cookies-call.ts | 15 +++++++++++++++ 4 files changed, 27 insertions(+), 30 deletions(-) create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/utils/is-cookies-call.ts create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/utils/is-cookies-or-awaited-cookies-call.ts diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/utils/collect-locally-scoped-cookie-bindings.ts b/packages/oxlint-plugin-react-doctor/src/plugin/utils/collect-locally-scoped-cookie-bindings.ts index 0eced72d1..1fe539a20 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/utils/collect-locally-scoped-cookie-bindings.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/utils/collect-locally-scoped-cookie-bindings.ts @@ -1,30 +1,15 @@ import { collectPatternNames } from "./collect-pattern-names.js"; import type { EsTreeNode } from "./es-tree-node.js"; +import { isCookiesOrAwaitedCookiesCall } from "./is-cookies-or-awaited-cookies-call.js"; import { isNodeOfType } from "./is-node-of-type.js"; import { walkInsideStatementBlocks } from "./walk-inside-statement-blocks.js"; -const isCookiesCall = (node: EsTreeNode): boolean => - isNodeOfType(node, "CallExpression") && - isNodeOfType(node.callee, "Identifier") && - node.callee.name === "cookies"; - -// `cookies()` became async in Next.js 15, so users now write -// `const cookieStore = await cookies()`. We unwrap one `await` so both -// the sync and async aliasing forms collapse to the same detection. -const isCookiesInit = (initNode: EsTreeNode): boolean => { - if (isCookiesCall(initNode)) return true; - if (isNodeOfType(initNode, "AwaitExpression") && initNode.argument) { - return isCookiesCall(initNode.argument); - } - return false; -}; - export const collectLocallyScopedCookieBindings = (handlerBody: EsTreeNode): Set => { const cookieBindingNames = new Set(); walkInsideStatementBlocks(handlerBody, (node: EsTreeNode) => { if (!isNodeOfType(node, "VariableDeclarator")) return; if (!node.init) return; - if (!isCookiesInit(node.init)) return; + if (!isCookiesOrAwaitedCookiesCall(node.init)) return; collectPatternNames(node.id, cookieBindingNames); }); return cookieBindingNames; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/utils/find-side-effect.ts b/packages/oxlint-plugin-react-doctor/src/plugin/utils/find-side-effect.ts index df0a317a5..08f402b5b 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/utils/find-side-effect.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/utils/find-side-effect.ts @@ -1,8 +1,9 @@ import { MUTATING_HTTP_METHODS, MUTATION_METHOD_NAMES } from "../constants/library.js"; import type { EsTreeNode } from "./es-tree-node.js"; -import { walkAst } from "./walk-ast.js"; +import { isCookiesOrAwaitedCookiesCall } from "./is-cookies-or-awaited-cookies-call.js"; import { isNodeOfType } from "./is-node-of-type.js"; import { isSafeReceiverChain } from "./is-safe-receiver-chain.js"; +import { walkAst } from "./walk-ast.js"; export interface FindSideEffectOptions { locallyScopedSafeBindings?: Set; @@ -27,22 +28,11 @@ const isMutatingMethodProperty = (property: EsTreeNode): boolean => typeof property.value.value === "string" && MUTATING_HTTP_METHODS.has(property.value.value.toUpperCase()); -const isDirectCookiesCall = (node: EsTreeNode): boolean => - isNodeOfType(node, "CallExpression") && - isNodeOfType(node.callee, "Identifier") && - node.callee.name === "cookies"; - -const isAwaitedCookiesCall = (node: EsTreeNode): boolean => - isNodeOfType(node, "AwaitExpression") && node.argument - ? isDirectCookiesCall(node.argument) - : false; - const isCookieReceiver = ( receiverNode: EsTreeNode, locallyScopedCookieBindings: Set, ): boolean => { - if (isDirectCookiesCall(receiverNode)) return true; - if (isAwaitedCookiesCall(receiverNode)) return true; + if (isCookiesOrAwaitedCookiesCall(receiverNode)) return true; if (isNodeOfType(receiverNode, "Identifier")) { return locallyScopedCookieBindings.has(receiverNode.name); } diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-cookies-call.ts b/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-cookies-call.ts new file mode 100644 index 000000000..5af0fb0a3 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-cookies-call.ts @@ -0,0 +1,7 @@ +import type { EsTreeNode } from "./es-tree-node.js"; +import { isNodeOfType } from "./is-node-of-type.js"; + +export const isCookiesCall = (node: EsTreeNode): boolean => + isNodeOfType(node, "CallExpression") && + isNodeOfType(node.callee, "Identifier") && + node.callee.name === "cookies"; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-cookies-or-awaited-cookies-call.ts b/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-cookies-or-awaited-cookies-call.ts new file mode 100644 index 000000000..19c79c6fc --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-cookies-or-awaited-cookies-call.ts @@ -0,0 +1,15 @@ +import type { EsTreeNode } from "./es-tree-node.js"; +import { isCookiesCall } from "./is-cookies-call.js"; +import { isNodeOfType } from "./is-node-of-type.js"; + +// `cookies()` became async in Next.js 15, so users now write +// `await cookies()` directly OR `const store = await cookies(); …`. +// We unwrap one `await` so both the sync `cookies()` form and the +// async `await cookies()` form collapse to the same detection. +export const isCookiesOrAwaitedCookiesCall = (node: EsTreeNode): boolean => { + if (isCookiesCall(node)) return true; + if (isNodeOfType(node, "AwaitExpression") && node.argument) { + return isCookiesCall(node.argument); + } + return false; +}; From 12a481931856067fa2b5318734bb04398d6f02bd Mon Sep 17 00:00:00 2001 From: Nisarg Patel Date: Fri, 15 May 2026 02:28:20 -0700 Subject: [PATCH 5/6] fix(tanstack-start): pass handler body (not the fn node) to binding collectors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `collectLocallyScopedSafeBindings` / `collectLocallyScopedCookieBindings` use `walkInsideStatementBlocks`, which returns immediately at any function boundary. `tanstack-start-get-mutation` was passing the `.handler(fn)` callback NODE itself, so both binding sets came back empty — every aliased shape inside a TanStack Start handler was misclassified: - `const customHeaders = new Headers(); customHeaders.set(...)` leaked a false positive. - `const cs = cookies(); cs.set(...)` got reported as `cs.set()` instead of the canonical `cookies().set()`. Unwrap to `handlerFunction.body` (mirroring how the Next.js rule passes `handlerBody` after `resolveGetHandlerBodies`) and add four regression cases that pin this down. `findSideEffect` still gets the body too, which is a no-op for it (walkAst descends through functions) but keeps the inputs consistent across both consumers. Co-authored-by: Cursor --- .../tanstack-start-get-mutation.ts | 18 ++- .../tanstack-start-get-mutation.test.ts | 131 ++++++++++++++++++ 2 files changed, 146 insertions(+), 3 deletions(-) create mode 100644 packages/react-doctor/tests/regressions/tanstack-start-get-mutation.test.ts diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/tanstack-start/tanstack-start-get-mutation.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/tanstack-start/tanstack-start-get-mutation.ts index 0ae2160fc..fb5eae397 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/tanstack-start/tanstack-start-get-mutation.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/tanstack-start/tanstack-start-get-mutation.ts @@ -36,9 +36,21 @@ export const tanstackStartGetMutation = defineRule({ const handlerFunction = node.arguments?.[0]; if (!handlerFunction) return; - const locallyScopedSafeBindings = collectLocallyScopedSafeBindings(handlerFunction); - const locallyScopedCookieBindings = collectLocallyScopedCookieBindings(handlerFunction); - const sideEffect = findSideEffect(handlerFunction, { + // HACK: `collectLocallyScoped*Bindings` uses `walkInsideStatementBlocks`, + // which intentionally stops at function boundaries — so we must hand it + // the function's body, not the function itself, or every aliased + // pattern (`const m = new Map(); m.set(...)`) would slip past. + if ( + !isNodeOfType(handlerFunction, "ArrowFunctionExpression") && + !isNodeOfType(handlerFunction, "FunctionExpression") + ) + return; + const handlerBody = handlerFunction.body; + if (!handlerBody) return; + + const locallyScopedSafeBindings = collectLocallyScopedSafeBindings(handlerBody); + const locallyScopedCookieBindings = collectLocallyScopedCookieBindings(handlerBody); + const sideEffect = findSideEffect(handlerBody, { locallyScopedSafeBindings, locallyScopedCookieBindings, }); diff --git a/packages/react-doctor/tests/regressions/tanstack-start-get-mutation.test.ts b/packages/react-doctor/tests/regressions/tanstack-start-get-mutation.test.ts new file mode 100644 index 000000000..7566a0457 --- /dev/null +++ b/packages/react-doctor/tests/regressions/tanstack-start-get-mutation.test.ts @@ -0,0 +1,131 @@ +/** + * Regression tests for `tanstack-start-get-mutation`. + * + * The rule shares `findSideEffect` with `nextjs-no-side-effect-in-get-handler` + * but receives the handler function NODE (the `.handler(() => {...})` + * callback) rather than its body. `collectLocallyScoped*Bindings` use + * `walkInsideStatementBlocks`, which stops at function boundaries — so + * previously every aliased safe / cookie pattern inside a TanStack Start + * handler slipped past the per-binding precision and either: + * - leaked a false-positive (`const customHeaders = new Headers(); customHeaders.set(...)`), + * - or got the wrong diagnostic message (`cs.set()` instead of `cookies().set()`). + * + * These cases pin down the unwrap fix. + */ + +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterAll, describe, expect, it } from "vite-plus/test"; + +import { runOxlint } from "@react-doctor/core"; +import type { Diagnostic, ProjectInfo } from "@react-doctor/types"; +import { buildTestProject, setupReactProject, writeFile } from "./_helpers.js"; + +const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "rd-tanstack-get-mutation-")); + +afterAll(() => { + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +const RULE_NAME = "tanstack-start-get-mutation"; + +// HACK: `tanstackStartGetMutation` matches any `.handler(fn)` call +// whose chain is rooted in `createServerFn` (see `walkServerFnChain`). +// The fixture stubs `createServerFn` so we don't need the real package +// installed; the AST shape is what the rule walks. +const CREATE_SERVER_FN_STUB = `const chainable: any = new Proxy( + {}, + { get: (_target, _prop) => (..._args: any[]) => chainable }, +); +const createServerFn = (_options?: any) => chainable; +`; + +const buildTanstackProject = ( + caseId: string, +): { rootDirectory: string; project: ProjectInfo } => { + const rootDirectory = setupReactProject(tempRoot, caseId); + return { + rootDirectory, + project: buildTestProject({ rootDirectory, framework: "tanstack-start" }), + }; +}; + +const writeRouteAndLint = async ( + caseId: string, + filePath: string, + source: string, +): Promise => { + const { rootDirectory, project } = buildTanstackProject(caseId); + writeFile(path.join(rootDirectory, filePath), `${CREATE_SERVER_FN_STUB}\n${source}`); + return runOxlint({ rootDirectory, project }); +}; + +const filterRule = (diagnostics: Diagnostic[]): Diagnostic[] => + diagnostics.filter((diagnostic) => diagnostic.rule === RULE_NAME); + +describe("tanstack-start-get-mutation: handler-body binding precision", () => { + it("does NOT flag `const customHeaders = new Headers(); customHeaders.set(...)` inside .handler()", async () => { + const diagnostics = await writeRouteAndLint( + "tanstack-aliased-headers", + "src/routes/headers-fn.tsx", + `export const headersFn = createServerFn().handler(async () => { + const customHeaders = new Headers(); + customHeaders.set("x-trace", "abc"); + customHeaders.append("x-source", "test"); + return { ok: true }; +}); +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + + it("does NOT flag `const cache = new Map(); cache.set(...)` inside .handler()", async () => { + const diagnostics = await writeRouteAndLint( + "tanstack-aliased-map", + "src/routes/map-fn.tsx", + `export const mapFn = createServerFn().handler(async () => { + const cache = new Map(); + cache.set("hit", Date.now()); + return { ok: true }; +}); +`, + ); + expect(filterRule(diagnostics)).toHaveLength(0); + }); + + it("flags aliased cookies (`const cs = cookies(); cs.set('x','y')`) as `cookies().set()`, not `cs.set()`", async () => { + const diagnostics = await writeRouteAndLint( + "tanstack-aliased-cookies", + "src/routes/cookies-fn.tsx", + `declare const cookies: () => { set: (a: string, b: string) => void; delete: (a: string) => void }; + +export const cookiesFn = createServerFn().handler(async () => { + const cs = cookies(); + cs.set("flag", "1"); + return { ok: true }; +}); +`, + ); + const hits = filterRule(diagnostics); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("cookies().set()"); + }); + + it("still flags real mutations on non-safe receivers inside .handler() (regression control)", async () => { + const diagnostics = await writeRouteAndLint( + "tanstack-real-mutation", + "src/routes/delete-fn.tsx", + `declare const db: { users: { delete: (id: string) => Promise } }; + +export const deleteFn = createServerFn().handler(async () => { + await db.users.delete("123"); + return { ok: true }; +}); +`, + ); + const hits = filterRule(diagnostics); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain(".delete()"); + }); +}); From bd9778a37a88f4d42ccc2269b5b6cad18a01f015 Mon Sep 17 00:00:00 2001 From: Nisarg Patel Date: Fri, 15 May 2026 17:59:03 -0700 Subject: [PATCH 6/6] chore: drop changeset (not used in this repo) Co-authored-by: Cursor --- .../nextjs-get-side-effect-false-positives.md | 46 ------------------- 1 file changed, 46 deletions(-) delete mode 100644 .changeset/nextjs-get-side-effect-false-positives.md diff --git a/.changeset/nextjs-get-side-effect-false-positives.md b/.changeset/nextjs-get-side-effect-false-positives.md deleted file mode 100644 index 8d45da03e..000000000 --- a/.changeset/nextjs-get-side-effect-false-positives.md +++ /dev/null @@ -1,46 +0,0 @@ ---- -"react-doctor": patch -"eslint-plugin-react-doctor": patch -"oxlint-plugin-react-doctor": patch ---- - -Fix trust-breaking false positives in `nextjs-no-side-effect-in-get-handler` -(issue #206). The rule used to flag any `.()` call as a server-state mutation, -which flooded one Next.js 14 codebase with 138 false errors — every single -one a `response.headers.set(...)` response-shaping call. - -The rule now skips these shapes, all of which only shape the outbound -response or mutate request-scoped collections: - -- `response.headers.set/append/delete(...)` and any chain ending in - `.headers` (e.g. `NextResponse.json({...}).headers.set(...)`, - `(await fetcher()).headers.append(...)`). -- Locally-constructed `new Map/Set/WeakMap/WeakSet/Headers/URLSearchParams/ -FormData/Response/NextResponse(...)` bindings and any mutation on those - aliases. -- `new URL(...).searchParams.set(...)` and any `.searchParams.*()` chain. -- `headers()` / `(await headers())` from `next/headers` (returns - `ReadonlyHeaders`; any mutation would throw at runtime) and any aliased - `const h = headers(); h.get(...)`. -- Route handlers under `/cron/` or `/jobs/cron/` — Vercel Cron always - invokes GET and is expected to do real work. - -The rule still flags real CSRF-relevant side effects: - -- ORM mutations like drizzle `db.update(table).set({...})`, `prisma.user. -create(...)`, `db.insert(...)`, `repository.upsert(...)`. -- Module-level mutable state (`const cache = new Map()` declared outside - the handler, then `cache.set(...)` inside it). -- `fetch(url, { method: "POST" | "PUT" | "DELETE" | "PATCH" })`. -- `cookies().set/append/delete()` in all forms: direct, `(await cookies()). -set(...)`, and aliased `const cookieStore = await cookies(); cookieStore. -set(...)` — the alias resolution now flags previously-missed handlers. -- Mutating route segments (`/logout`, `/signout`, `/unsubscribe`, `/delete`, - …). - -The rule also gained depth-bounded handler-binding resolution so -`export const GET = withAuth(handler)` and `export const GET = app.get('/x', -handler).get('/y', handler)` get scanned correctly. - -Closes #206. Supersedes PRs #209, #211, #233, and #238.