diff --git a/.changeset/eslint-plugin-read-audit.md b/.changeset/eslint-plugin-read-audit.md new file mode 100644 index 0000000..fbee532 --- /dev/null +++ b/.changeset/eslint-plugin-read-audit.md @@ -0,0 +1,10 @@ +--- +"@paramour-js/eslint-plugin": minor +--- + +Two new rules complete the bypass audit in both directions, and the existing rule's messages now name the real API. + +- **`no-raw-param-reads`** — the read-side twin of `no-raw-hrefs`. Flags `useSearchParams()` / `useParams()` imported from `next/navigation` (nudging `useSearch(route)` / `useRouteParams(route)` from `@paramour-js/next/app`) and `router.query` on a `next/router` `useRouter()` router, including the direct-call and destructured forms (nudging the `@paramour-js/next/pages` hooks). An `allow` option (`"routerQuery" | "useParams" | "useSearchParams"`) switches a surface off wholesale. +- **`no-href-arithmetic`** — flags string content appended after an `href()` result (`href(route) + "?tab=1"`, `` `${href(route)}/reviews` ``); prefix-only concatenation (`origin + href(route)`) stays legal. The pure hash case is the plugin's first autofix: `href(route) + "#top"` rewrites to `href(route, { hash: "top" })` when the options provably carry no `hash`. +- Both rules ship in `configs.recommended` at `warn`. +- `no-raw-hrefs` messages now say `href(route, …)` — there is no `route.href()` method to point at. diff --git a/docs/content/docs/reference/eslint-plugin.mdx b/docs/content/docs/reference/eslint-plugin.mdx index 17ffb6c..a5f0059 100644 --- a/docs/content/docs/reference/eslint-plugin.mdx +++ b/docs/content/docs/reference/eslint-plugin.mdx @@ -1,16 +1,17 @@ --- title: "@paramour-js/eslint-plugin" -description: The ESLint plugin — find every raw string href paramour never sees. +description: The ESLint plugin — find every place paramour never sees, in both directions. --- `@paramour-js/eslint-plugin` exists because paramour's value proposition — validated params, typed path building, explicit serialization — evaporates silently the moment someone writes `` or -`router.push("/shop?page=2")`. The code compiles, the navigation works, and -the route's codecs simply never run. In a codebase mid-migration this is -the default failure mode, not an edge case: every pre-existing link is a -raw string, and every new one written from habit is too. The plugin's one -rule finds them all. +`useSearchParams()`. The code compiles, the page renders, and the route's +codecs simply never run. In a codebase mid-migration this is the default +failure mode, not an edge case. The plugin audits the bypass in both +directions — raw writes (`no-raw-hrefs`), raw reads +(`no-raw-param-reads`) — and guards the integrity of what `href()` builds +(`no-href-arithmetic`). ## Install @@ -34,7 +35,7 @@ export default [ ]; ``` -Or wire the rule manually: +Or wire the rules manually: ```js title="eslint.config.js" import paramour from "@paramour-js/eslint-plugin"; @@ -43,15 +44,18 @@ export default [ { plugins: { paramour }, rules: { + "paramour/no-href-arithmetic": "warn", "paramour/no-raw-hrefs": "warn", + "paramour/no-raw-param-reads": "warn", }, }, ]; ``` -The preset registers the rule at `warn` on purpose: a raw string href is -*working* code, and this is a nudge toward migration, not a correctness -gate. Once your routes are migrated, promote it in one line: +The preset registers every rule at `warn` on purpose: a raw string href or +a raw param read is *working* code, and this is a nudge toward migration, +not a correctness gate. Once your routes are migrated, promote any rule +individually: ```js rules: { "paramour/no-raw-hrefs": "error" } @@ -79,6 +83,7 @@ that start with `/`, in three Next.js App Router surfaces: ```tsx import Link from "next/link"; import { redirect, useRouter } from "next/navigation"; +import { href } from "paramour"; ; // ✗ flagged redirect("/login"); // ✗ flagged @@ -86,7 +91,7 @@ redirect("/login"); // ✗ flagged const router = useRouter(); router.push("/shop?page=2"); // ✗ flagged -; // ✓ what the rule nudges toward +; // ✓ what the rule nudges toward ``` There is no autofix and no suggestion: a correct fix requires knowing @@ -144,3 +149,155 @@ Recorded so the omissions read as decisions, not oversights: rule is purely syntactic by design: no type information, no `parserOptions.project` requirement, works in any parser setup that produces JSX nodes. + +## no-raw-param-reads + +The read-side twin of `no-raw-hrefs`: reports the Next.js read APIs whose +results paramour's codecs never validate, in two surfaces: + +1. **`useSearchParams()` / `useParams()`** — calls of either hook imported + from `next/navigation`, under any local name, including the namespace + form (`nav.useParams()`). Imports are tracked through scope resolution, + so the same-named hooks from `react-router-dom` (or anywhere else) + never fire. The message nudges the like-for-like replacement: + `useSearch(route)` for `useSearchParams()`, `useRouteParams(route)` for + `useParams()`, both from `@paramour-js/next/app`. +2. **`router.query`** — `.query` access on a router obtained from + `next/router`'s `useRouter()` (the extensionful `next/router.js` + spelling counts too), in the variable form + (`const router = useRouter()`), the direct form + (`useRouter().query`), and the destructured form + (`const { query } = useRouter()`, including renames and nested + patterns). The nudge is `useRouteParams(route)` and `useSearch(route)` + from `@paramour-js/next/pages`. + +```tsx +import { useParams, useSearchParams } from "next/navigation"; +import { useRouteParams, useSearch } from "@paramour-js/next/app"; + +const params = useParams(); // ✗ flagged +const search = useSearchParams(); // ✗ flagged + +const result = useRouteParams(productRoute); // ✓ what the rule nudges toward +const query = useSearch(productRoute); // ✓ +``` + +There is no autofix and no suggestion: a correct fix requires knowing +which route object the component belongs to — not mechanically derivable +from the call site. + +### What is exempt + +Only reads through the tracked imports fire. `useParams` from +`react-router-dom`, a locally defined `useParams`, and type-only imports +never fire. `useRouter().query` where `useRouter` comes from +`next/navigation` never fires either — the App Router's router has no +`query`. Computed access (`router["query"]`) and a router crossing a +function or prop boundary escape detection — the rule is purely +syntactic, same as `no-raw-hrefs`. + +Other `next/navigation` reads (`usePathname`, `useSelectedLayoutSegment`) +are not paramour's territory and are never flagged. + +### Options + +| Option | Type | Default | Description | +| ------- | ----------------------------------------------------- | ------- | ---------------------------------- | +| `allow` | `("routerQuery" \| "useParams" \| "useSearchParams")[]` | `[]` | Surfaces to switch off wholesale. | + +`allow` is the escape hatch for a codebase that deliberately keeps one +surface untyped — for example an app that reads `useParams()` only in a +handful of not-yet-migrated components: + +```js +rules: { + "paramour/no-raw-param-reads": ["warn", { allow: ["useParams"] }], +} +``` + +For a *single* legitimate raw read — say, forwarding ad-hoc `utm_*` +params the app deliberately leaves out of its route definitions — prefer +a targeted disable comment over switching off the whole surface: + +```ts +// eslint-disable-next-line paramour/no-raw-param-reads -- untyped utm_* forwarding +const search = useSearchParams(); +``` + +### Deliberately out of scope (v1) + +Recorded so the omissions read as decisions, not oversights: + +- **Direct `props.searchParams` / `props.params` access** in `page.tsx` / + `layout.tsx` — the noisiest surface and the only one needing filename + awareness; it follows once the hook surfaces prove the false-positive + policy, mirroring how `no-raw-hrefs` sequenced dynamic strings out of + its v1. +- **Routers crossing boundaries** — same accepted cost as `no-raw-hrefs`. +- **Computed access** — `router["query"]` is invisible to a syntactic + rule. + +## no-href-arithmetic + +Reports string arithmetic that appends content *after* an `href()` result +— the exact thing explicit serialization exists to prevent — in two +surfaces: + +1. **`+` concatenation** — `href(route) + "?tab=1"`, including chained + forms (`href(route) + "#a" + x`). The `href` import from `paramour` is + tracked through scope resolution, aliased and namespace forms + included. +2. **Template literals** — content after `${href(route)}`: + `` `${href(route)}?page=2` ``, `` `${href(route)}/${child}` ``. + +Prepending is fine: `origin + href(route)` and +`` `${origin}${href(route)}` `` are the legitimate way to build absolute +URLs for metadata, emails, and redirects, and never fire. + +```ts +import { href } from "paramour"; + +const a = href(route) + "?tab=1"; // ✗ flagged — declare tab in the route's search codecs +const b = `${href(route)}/reviews`; // ✗ flagged +const c = href(route) + "#reviews"; // ✗ flagged, autofixed ↓ + +const fixed = href(route, { hash: "reviews" }); // ✓ what the autofix produces +const url = "https://example.com" + href(route); // ✓ prefixing is legitimate +``` + +This is the plugin's first fixable rule, and the fixer is deliberately +narrow: it rewrites only the pure hash case — a single appended +`"#fragment"` literal on a call whose options provably have no `hash` +(absent, or an object literal without one). The rewrite is exactly +semantics-preserving: `href()` renders `hash` as a trailing `#fragment`, +byte-identical to the concatenation it replaces. `?` suffixes are never +autofixed — the appended query needs a codec key in the route's `search` +config, which no fixer can invent — so the message points at the `search` +option instead. + +### What is exempt + +Prefix-only concatenation and interpolation, as above. `href` from any +other module, shadowed locals, and type-only imports never fire. Tagged +templates (`` sql`${href(route)}#x` ``) are skipped — the tag's semantics +are unknown. + +### Options + +No options in v1. One-off exceptions use a standard +`// eslint-disable-next-line paramour/no-href-arithmetic` comment. + +### Deliberately out of scope (v1) + +Recorded so the omissions read as decisions, not oversights: + +- **Method-style flows** — `href(route).concat("#a")`, + `[href(route), "#a"].join("")`, and `+=` accumulation. +- **Values crossing bindings** — `const base = href(route); base + "#a"` + escapes detection; same purely-syntactic boundary as the other rules. +- **The prefix-template autofix** — `` `${origin}${href(route)}#top` `` + gets the hash message but no fix; the fixer only rewrites expressions + that *are* the call plus a suffix. +- **A `route.href()` method form** — not an omission but a fact: paramour + has no such method. `href()` is a standalone import, which is what makes + the scope-resolved detection precise. diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 7ac94dd..a583e8c 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -1,6 +1,6 @@ # @paramour-js/eslint-plugin -ESLint plugin for [paramour](https://paramour.dev). A raw string href — ``, `router.push("/shop?page=2")`, `redirect("/login")` — compiles, navigates, and silently bypasses everything paramour does: the route's codecs never run, params are never validated, and typos ship. In a codebase mid-migration this is the default failure mode. This plugin finds every unmigrated link. +ESLint plugin for [paramour](https://paramour.dev). A raw string href — `` — or a raw param read — `useSearchParams()`, `router.query` — compiles, renders, and silently bypasses everything paramour does: the route's codecs never run, params are never validated, and typos ship. In a codebase mid-migration this is the default failure mode. This plugin finds every place paramour never sees, in both directions, and guards the integrity of what `href()` builds. Docs: @@ -25,7 +25,7 @@ export default [ ]; ``` -Or wire the rule manually: +Or wire the rules manually: ```js import paramour from "@paramour-js/eslint-plugin"; @@ -34,37 +34,31 @@ export default [ { plugins: { paramour }, rules: { + "paramour/no-href-arithmetic": "warn", "paramour/no-raw-hrefs": "warn", + "paramour/no-raw-param-reads": "warn", }, }, ]; ``` -The preset registers the rule at `warn` — it is a migration nudge, not a correctness gate. Once your routes are migrated, promote it in one line: +The preset registers every rule at `warn` — it is a migration nudge, not a correctness gate. Once your routes are migrated, promote any rule individually: ```js rules: { "paramour/no-raw-hrefs": "error" } ``` -## What it flags +## Rules -`paramour/no-raw-hrefs` reports string literals (and expression-free template literals) starting with `/` in three Next.js App Router surfaces: +- [`no-raw-hrefs`](https://paramour.dev/docs/reference/eslint-plugin#no-raw-hrefs) — raw string paths flowing into ``, `router.push`/`replace`/`prefetch`, and `redirect`/`permanentRedirect`. +- [`no-raw-param-reads`](https://paramour.dev/docs/reference/eslint-plugin#no-raw-param-reads) — raw reads through `useSearchParams()`/`useParams()` from `next/navigation` and `router.query` from `next/router`. +- [`no-href-arithmetic`](https://paramour.dev/docs/reference/eslint-plugin#no-href-arithmetic) — string content appended after an `href()` result; the pure-hash case is autofixed to `href()`'s `hash` option. -1. the `href` attribute of `Link` imported from `next/link` (any local name — imports are tracked, not names matched); -2. the first argument of `push`, `replace`, and `prefetch` on a router obtained from `next/navigation`'s `useRouter()` — including the destructured form `const { push } = useRouter()`; -3. arguments to `redirect` and `permanentRedirect` imported from `next/navigation`. +### no-raw-hrefs -External URLs (`https://…`, protocol-relative `//…`), fragments (`#…`), `mailto:`/`tel:`, relative paths, and empty strings never start with `/` (or are explicitly exempt) and are ignored. +Reports string literals (and expression-free template literals) starting with `/` in three Next.js App Router surfaces: the `href` attribute of `Link` imported from `next/link` (any local name — imports are tracked, not names matched); the first argument of `push`, `replace`, and `prefetch` on a router obtained from `next/navigation`'s `useRouter()` — including the destructured form `const { push } = useRouter()`; and arguments to `redirect` and `permanentRedirect` imported from `next/navigation`. -Deliberately out of scope in v1: dynamic strings (`"/users/" + id`, template literals with expressions), the Pages router (`next/router`), the `UrlObject` href form (`href={{ pathname: "/foo" }}`), `Link` re-exported through a wrapper component, and a router instance passed across function or prop boundaries. - -## Options - -| Option | Type | Default | Description | -| ------------- | ---------- | ------- | ------------------------------------------- | -| `ignorePaths` | `string[]` | `[]` | Path prefixes to exempt during a migration. | - -`ignorePaths` matches path-segment prefixes, not raw substrings: `"/legacy"` exempts `/legacy`, `/legacy/old`, `/legacy?tab=1`, and `/legacy#top`, but **not** `/legacybar`. A trailing slash is ignored (`"/legacy/"` behaves like `"/legacy"`); `"/"` exempts everything. +External URLs (`https://…`, protocol-relative `//…`), fragments (`#…`), `mailto:`/`tel:`, relative paths, and empty strings are ignored. `ignorePaths` (path-segment prefixes, not substrings or globs) exempts sections a migration has not reached yet: ```js rules: { @@ -72,6 +66,14 @@ rules: { } ``` +### no-raw-param-reads + +The read-side twin: reports `useSearchParams()` / `useParams()` imported from `next/navigation` (nudging `useSearch(route)` / `useRouteParams(route)` from `@paramour-js/next/app`) and `router.query` on a `next/router` `useRouter()` router, including `useRouter().query` and destructured forms (nudging the `@paramour-js/next/pages` hooks). The `allow` option (`"routerQuery" | "useParams" | "useSearchParams"`) switches a surface off wholesale; one-off legitimate reads (e.g. untyped `utm_*` forwarding) use a targeted disable comment. + +### no-href-arithmetic + +Reports content appended after an `href()` result — `href(route) + "?tab=1"`, `` `${href(route)}/reviews` `` — which reintroduces unvalidated URL content through the back door. Prefixing is fine (`origin + href(route)` is the legitimate absolute-URL pattern). The pure hash case (`href(route) + "#top"`) is autofixed to `href(route, { hash: "top" })`; `?` suffixes are message-only because the appended query needs a codec key in the route's `search` config. No options. + ## License MIT diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index 4542624..72e4a20 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -28,7 +28,7 @@ "eslint": "^10.6.0", "typescript": "^6.0.3" }, - "description": "ESLint plugin for paramour — flags raw string hrefs in Next.js navigation APIs that bypass paramour's typed href() building", + "description": "ESLint plugin for paramour — flags raw hrefs, raw param reads, and href() arithmetic that bypass paramour's typed routing", "author": "Jason Paff ", "keywords": [ "eslint", diff --git a/packages/eslint-plugin/src/index.ts b/packages/eslint-plugin/src/index.ts index 90d5168..789bfa5 100644 --- a/packages/eslint-plugin/src/index.ts +++ b/packages/eslint-plugin/src/index.ts @@ -1,6 +1,8 @@ import type { TSESLint } from "@typescript-eslint/utils"; +import { noHrefArithmetic } from "./rules/no-href-arithmetic.js"; import { noRawHrefs } from "./rules/no-raw-hrefs.js"; +import { noRawParamReads } from "./rules/no-raw-param-reads.js"; // meta.version is intentionally hardcoded (importing package.json would break // the build's rootDir) — it is debug/cache-key metadata only and may lag the @@ -12,7 +14,9 @@ const plugin = { version: "0.1.0", }, rules: { + "no-href-arithmetic": noHrefArithmetic, "no-raw-hrefs": noRawHrefs, + "no-raw-param-reads": noRawParamReads, }, }; @@ -22,7 +26,9 @@ const recommended: TSESLint.FlatConfig.Config = { paramour: plugin, }, rules: { + "paramour/no-href-arithmetic": "warn", "paramour/no-raw-hrefs": "warn", + "paramour/no-raw-param-reads": "warn", }, }; diff --git a/packages/eslint-plugin/src/rules/no-href-arithmetic.ts b/packages/eslint-plugin/src/rules/no-href-arithmetic.ts new file mode 100644 index 0000000..34f7361 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-href-arithmetic.ts @@ -0,0 +1,269 @@ +import type { TSESLint, TSESTree } from "@typescript-eslint/utils"; + +import { AST_NODE_TYPES } from "@typescript-eslint/utils"; + +import { createRule, DOCS_URL } from "../utils/create-rule.js"; +import { getImportedCallee } from "../utils/imports.js"; +import { getStaticPath } from "../utils/static-path.js"; + +type MessageIds = "hrefConcat" | "hrefHashConcat" | "hrefSearchConcat"; +type Options = []; + +/** + * Collects the operands of a `+` chain in source order: `a + b + c` yields + * [a, b, c]. Parentheses do not materialize in the AST, so grouping folds + * away — which is fine, because only "does anything follow the href() result" + * matters here. + */ +function flattenPlusChain(node: TSESTree.BinaryExpression): TSESTree.Node[] { + const operands: TSESTree.Node[] = []; + const walk = (expression: TSESTree.Node): void => { + if ( + expression.type === AST_NODE_TYPES.BinaryExpression && + expression.operator === "+" + ) { + walk(expression.left); + walk(expression.right); + return; + } + operands.push(expression); + }; + walk(node); + return operands; +} + +export const noHrefArithmetic = createRule({ + create(context) { + const { sourceCode } = context; + + function isHrefCall( + expression: TSESTree.Node, + ): expression is TSESTree.CallExpression { + if (expression.type !== AST_NODE_TYPES.CallExpression) return false; + const binding = getImportedCallee(sourceCode, expression); + return binding?.imported === "href" && binding.source === "paramour"; + } + + /** + * True when the expression's rightmost produced string content is an + * href() result: the call itself, a `+` chain ending in one, or a + * template literal whose final quasi is empty and whose last expression + * ends with one. The empty-final-quasi condition is what makes + * double-reporting between the two visitors impossible: a template with + * content after the href is claimed by the TemplateLiteral visitor and + * never counts as "ending with href" for an enclosing `+` chain. + */ + function endsWithHrefCall(expression: TSESTree.Node): boolean { + if (isHrefCall(expression)) return true; + if ( + expression.type === AST_NODE_TYPES.BinaryExpression && + expression.operator === "+" + ) { + return endsWithHrefCall(expression.right); + } + if (expression.type === AST_NODE_TYPES.TemplateLiteral) { + const lastQuasi = expression.quasis[expression.quasis.length - 1]; + const lastExpression = + expression.expressions[expression.expressions.length - 1]; + if (lastQuasi?.value.cooked !== "") return false; + return lastExpression !== undefined && endsWithHrefCall(lastExpression); + } + return false; + } + + /** + * Builds the hash autofix, or returns null when any bail-out makes the + * rewrite unprovable: the fix must be exactly semantics-preserving — + * href() renders `hash` as a trailing "#", byte-identical to + * the flagged concatenation. + */ + function buildHashFix( + reportNode: TSESTree.Node, + hrefCall: TSESTree.CallExpression, + suffix: string, + ): null | TSESLint.ReportFixFunction { + const fragment = suffix.slice(1); + const args = hrefCall.arguments; + if (args.length === 0 || args.length > 2) return null; + if (args.some((arg) => arg.type === AST_NODE_TYPES.SpreadElement)) + return null; + const optionsArg = args[1]; + if ( + optionsArg !== undefined && + optionsArg.type !== AST_NODE_TYPES.ObjectExpression + ) + return null; + let firstProperty: TSESTree.ObjectLiteralElement | undefined; + if (optionsArg) { + for (const property of optionsArg.properties) { + // A spread could carry a hash; a computed key could be "hash". + if (property.type === AST_NODE_TYPES.SpreadElement) return null; + if (property.computed) return null; + const { key } = property; + if (key.type === AST_NODE_TYPES.Identifier && key.name === "hash") + return null; + if (key.type === AST_NODE_TYPES.Literal && key.value === "hash") + return null; + } + firstProperty = optionsArg.properties[0]; + } + const closingParen = sourceCode.getLastToken(hrefCall); + if (!closingParen) return null; + // A trailing comma in the argument list would leave the insertion + // point ambiguous — rare and Prettier-normalized away, so just bail. + if (sourceCode.getTokenBefore(closingParen)?.value === ",") return null; + // Removing the surrounding ranges would silently delete any comment + // living outside the href call. + const hasOutsideComments = sourceCode + .getCommentsInside(reportNode) + .some( + (comment) => + comment.range[0] < hrefCall.range[0] || + comment.range[1] > hrefCall.range[1], + ); + if (hasOutsideComments) return null; + const hashText = `hash: ${JSON.stringify(fragment)}`; + return (fixer) => { + const edits: TSESLint.RuleFix[] = []; + if (!optionsArg) { + edits.push(fixer.insertTextBefore(closingParen, `, { ${hashText} }`)); + } else if (firstProperty === undefined) { + edits.push(fixer.replaceText(optionsArg, `{ ${hashText} }`)); + } else { + // Inserting first keeps perfectionist-natural key order in the + // common cases: hash sorts before params and search. + edits.push(fixer.insertTextBefore(firstProperty, `${hashText}, `)); + } + if (reportNode.range[0] < hrefCall.range[0]) { + edits.push( + fixer.removeRange([reportNode.range[0], hrefCall.range[0]]), + ); + } + if (hrefCall.range[1] < reportNode.range[1]) { + edits.push( + fixer.removeRange([hrefCall.range[1], reportNode.range[1]]), + ); + } + return edits; + }; + } + + /** + * Classifies the appended content and reports. `suffix` is the static + * string appended directly after the href-ending operand, when there is + * exactly one such trailing piece; `hrefCall` is non-null only when the + * whole reported expression is exactly the call plus that suffix — the + * only shape the fixer rewrites. + */ + function reportConcat( + reportNode: TSESTree.Node, + suffix: null | string, + hrefCall: null | TSESTree.CallExpression, + ): void { + if ( + suffix !== null && + suffix.startsWith("#") && + suffix.length > 1 && + !suffix.includes("?") + ) { + context.report({ + data: { suffix }, + fix: hrefCall ? buildHashFix(reportNode, hrefCall, suffix) : null, + messageId: "hrefHashConcat", + node: reportNode, + }); + return; + } + if (suffix?.startsWith("?")) { + context.report({ + data: { suffix }, + messageId: "hrefSearchConcat", + node: reportNode, + }); + return; + } + context.report({ messageId: "hrefConcat", node: reportNode }); + } + + return { + BinaryExpression(node) { + if (node.operator !== "+") return; + // Topmost-only: nested `+` chains are handled once at the top. + if ( + node.parent.type === AST_NODE_TYPES.BinaryExpression && + node.parent.operator === "+" + ) + return; + const operands = flattenPlusChain(node); + let index = -1; + for (let i = operands.length - 2; i >= 0; i--) { + const operand = operands[i]; + if (operand !== undefined && endsWithHrefCall(operand)) { + index = i; + break; + } + } + if (index === -1) return; + const last = operands[operands.length - 1]; + const suffix = + index === operands.length - 2 && last !== undefined + ? getStaticPath(last) + : null; + const head = operands[index]; + const hrefCall = + operands.length === 2 && head !== undefined && isHrefCall(head) + ? head + : null; + reportConcat(node, suffix, hrefCall); + }, + TemplateLiteral(node) { + // Tagged templates have unknown semantics (sql`…`, css`…`) — skip. + if (node.parent.type === AST_NODE_TYPES.TaggedTemplateExpression) + return; + let index = -1; + for (let i = node.expressions.length - 1; i >= 0; i--) { + const expression = node.expressions[i]; + if (expression !== undefined && endsWithHrefCall(expression)) { + index = i; + break; + } + } + if (index === -1) return; + const trailingExpressions = node.expressions.length - 1 - index; + const hasTrailingText = node.quasis + .slice(index + 1) + .some((quasi) => quasi.value.cooked !== ""); + if (trailingExpressions === 0 && !hasTrailingText) return; + const suffix = + trailingExpressions === 0 + ? (node.quasis[index + 1]?.value.cooked ?? null) + : null; + const only = node.expressions[0]; + const hrefCall = + node.expressions.length === 1 && + only !== undefined && + node.quasis[0]?.value.cooked === "" && + isHrefCall(only) + ? only + : null; + reportConcat(node, suffix, hrefCall); + }, + }; + }, + defaultOptions: [], + meta: { + docs: { + description: + "Disallow appending strings to paramour's href() results; pass search and hash through href()'s options instead", + }, + fixable: "code", + messages: { + hrefConcat: `Appending content to an href() result bypasses paramour's route validation and serialization. Pass it through href()'s options ({ params, search, hash }) instead — ${DOCS_URL}`, + hrefHashConcat: `Appending "{{suffix}}" to an href() result bypasses paramour's route validation. Pass it as href()'s hash option instead — ${DOCS_URL}`, + hrefSearchConcat: `Appending "{{suffix}}" to an href() result bypasses paramour's serialization. Declare the params in the route's search codecs and pass them through href()'s search option instead — ${DOCS_URL}`, + }, + schema: [], + type: "suggestion", + }, + name: "no-href-arithmetic", +}); diff --git a/packages/eslint-plugin/src/rules/no-raw-hrefs.ts b/packages/eslint-plugin/src/rules/no-raw-hrefs.ts index 627260c..d03c0ec 100644 --- a/packages/eslint-plugin/src/rules/no-raw-hrefs.ts +++ b/packages/eslint-plugin/src/rules/no-raw-hrefs.ts @@ -1,81 +1,21 @@ -import type { TSESLint, TSESTree } from "@typescript-eslint/utils"; +import type { TSESTree } from "@typescript-eslint/utils"; -import { - AST_NODE_TYPES, - ASTUtils, - ESLintUtils, -} from "@typescript-eslint/utils"; +import { AST_NODE_TYPES } from "@typescript-eslint/utils"; -interface ImportBinding { - imported: string; - source: string; -} +import { createRule, DOCS_URL } from "../utils/create-rule.js"; +import { + getImportBinding, + isImportedCall, + resolveDef, +} from "../utils/imports.js"; +import { getStaticPath } from "../utils/static-path.js"; type MessageIds = "rawHref" | "rawRedirect" | "rawRouterCall"; type Options = [{ ignorePaths?: string[] }]; -const DOCS_URL = "https://paramour.dev/docs/reference/eslint-plugin"; - const REDIRECT_FUNCTIONS = new Set(["permanentRedirect", "redirect"]); const ROUTER_METHODS = new Set(["prefetch", "push", "replace"]); -const createRule = ESLintUtils.RuleCreator((name) => `${DOCS_URL}#${name}`); - -/** - * Resolves a variable definition to the import specifier it binds, if any. - * Type-only imports are treated as no binding — a value usage of one is - * already a TS error, and flagging it here would be noise on top. - */ -function getImportBinding( - def: TSESLint.Scope.Definition | undefined, -): ImportBinding | null { - if (!def) return null; - const specifier = def.node; - if (specifier.type === AST_NODE_TYPES.ImportDefaultSpecifier) { - if (specifier.parent.importKind === "type") return null; - return { imported: "default", source: specifier.parent.source.value }; - } - if (specifier.type === AST_NODE_TYPES.ImportNamespaceSpecifier) { - if (specifier.parent.importKind === "type") return null; - return { imported: "*", source: specifier.parent.source.value }; - } - if (specifier.type === AST_NODE_TYPES.ImportSpecifier) { - if (specifier.parent.type !== AST_NODE_TYPES.ImportDeclaration) return null; - if ( - specifier.importKind === "type" || - specifier.parent.importKind === "type" - ) - return null; - const { imported } = specifier; - return { - imported: - imported.type === AST_NODE_TYPES.Identifier - ? imported.name - : imported.value, - source: specifier.parent.source.value, - }; - } - return null; -} - -/** - * Extracts the string value of a static path expression: a string literal or - * an expression-free template literal. Dynamic strings return null — out of - * scope for v1. - */ -function getStaticPath(node: TSESTree.Node): null | string { - if (node.type === AST_NODE_TYPES.Literal && typeof node.value === "string") { - return node.value; - } - if ( - node.type === AST_NODE_TYPES.TemplateLiteral && - node.expressions.length === 0 - ) { - return node.quasis[0]?.value.cooked ?? null; - } - return null; -} - /** * Boundary-aware prefix match: "/legacy" exempts "/legacy", "/legacy/old", * "/legacy?tab=1", "/legacy#top" — but not "/legacybar". A trailing slash on @@ -105,36 +45,8 @@ export const noRawHrefs = createRule({ const ignorePaths = options.ignorePaths ?? []; const { sourceCode } = context; - function resolveDef( - node: TSESTree.Node, - name: string, - ): TSESLint.Scope.Definition | undefined { - return ASTUtils.findVariable(sourceCode.getScope(node), name)?.defs[0]; - } - function isUseRouterCall(node: null | TSESTree.Expression): boolean { - if (node?.type !== AST_NODE_TYPES.CallExpression) return false; - const { callee } = node; - if (callee.type === AST_NODE_TYPES.Identifier) { - const binding = getImportBinding(resolveDef(callee, callee.name)); - return ( - binding?.imported === "useRouter" && - binding.source === "next/navigation" - ); - } - // Namespace form: import * as nav from "next/navigation"; nav.useRouter() - if ( - callee.type !== AST_NODE_TYPES.MemberExpression || - callee.computed || - callee.object.type !== AST_NODE_TYPES.Identifier || - callee.property.type !== AST_NODE_TYPES.Identifier || - callee.property.name !== "useRouter" - ) - return false; - const binding = getImportBinding( - resolveDef(callee.object, callee.object.name), - ); - return binding?.imported === "*" && binding.source === "next/navigation"; + return isImportedCall(sourceCode, node, "useRouter", ["next/navigation"]); } function checkPath( @@ -175,7 +87,7 @@ export const noRawHrefs = createRule({ if (!REDIRECT_FUNCTIONS.has(method) && !ROUTER_METHODS.has(method)) return; if (callee.object.type !== AST_NODE_TYPES.Identifier) return; - const def = resolveDef(callee.object, callee.object.name); + const def = resolveDef(sourceCode, callee.object, callee.object.name); const binding = getImportBinding(def); if (binding) { // Surface 3, namespace form: import * as nav from @@ -205,7 +117,7 @@ export const noRawHrefs = createRule({ return; } if (callee.type !== AST_NODE_TYPES.Identifier) return; - const def = resolveDef(callee, callee.name); + const def = resolveDef(sourceCode, callee, callee.name); if (!def) return; const binding = getImportBinding(def); if (binding) { @@ -251,7 +163,9 @@ export const noRawHrefs = createRule({ // matter what is in scope — skip them before paying for scope // resolution; hrefs on intrinsics dominate real JSX. if (/^[a-z]/.test(elementName.name)) return; - const binding = getImportBinding(resolveDef(node, elementName.name)); + const binding = getImportBinding( + resolveDef(sourceCode, node, elementName.name), + ); if (binding?.imported !== "default" || binding.source !== "next/link") return; const { value } = node; @@ -271,9 +185,9 @@ export const noRawHrefs = createRule({ "Disallow raw string paths in Next.js navigation APIs; build hrefs with paramour's typed href() instead", }, messages: { - rawHref: `Raw string href "{{path}}" bypasses paramour's route validation. Build it with the route's href() instead — ${DOCS_URL}`, - rawRedirect: `{{callee}}() called with raw path "{{path}}" bypasses paramour's route validation. Pass the route's href() result instead — ${DOCS_URL}`, - rawRouterCall: `router.{{method}}() called with raw path "{{path}}" bypasses paramour's route validation. Pass the route's href() result instead — ${DOCS_URL}`, + rawHref: `Raw string href "{{path}}" bypasses paramour's route validation. Build it with href(route, …) instead — ${DOCS_URL}`, + rawRedirect: `{{callee}}() called with raw path "{{path}}" bypasses paramour's route validation. Pass an href(route, …) result instead — ${DOCS_URL}`, + rawRouterCall: `router.{{method}}() called with raw path "{{path}}" bypasses paramour's route validation. Pass an href(route, …) result instead — ${DOCS_URL}`, }, schema: [ { diff --git a/packages/eslint-plugin/src/rules/no-raw-param-reads.ts b/packages/eslint-plugin/src/rules/no-raw-param-reads.ts new file mode 100644 index 0000000..63a353a --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-raw-param-reads.ts @@ -0,0 +1,136 @@ +import type { TSESTree } from "@typescript-eslint/utils"; + +import { AST_NODE_TYPES } from "@typescript-eslint/utils"; + +import { createRule, DOCS_URL } from "../utils/create-rule.js"; +import { + getImportedCallee, + isImportedCall, + resolveDef, + sourceMatches, +} from "../utils/imports.js"; + +type AllowEntry = "routerQuery" | "useParams" | "useSearchParams"; +type MessageIds = "rawParamsHook" | "rawRouterQuery"; +type Options = [{ allow?: AllowEntry[] }]; + +// Each raw App Router hook mapped to its like-for-like paramour replacement; +// both names are interpolated into the message. +const APP_HOOK_REPLACEMENTS = new Map([ + ["useParams", "useRouteParams"], + ["useSearchParams", "useSearch"], +]); + +// Bare and extensionful spellings — nodenext-resolution users import +// "next/router.js". +const PAGES_ROUTER_SOURCES = ["next/router", "next/router.js"]; + +export const noRawParamReads = createRule({ + create(context, [options]) { + // Widened to string so unfiltered binding names can be probed directly. + const allow: ReadonlySet = new Set(options.allow ?? []); + const { sourceCode } = context; + + function isPagesUseRouterCall( + node: null | TSESTree.Expression | undefined, + ): boolean { + return isImportedCall( + sourceCode, + node, + "useRouter", + PAGES_ROUTER_SOURCES, + ); + } + + return { + // Surface 1: useParams() / useSearchParams() from next/navigation. The + // call compiles and renders, but the route's codecs never run. + CallExpression(node) { + const binding = getImportedCallee(sourceCode, node); + if (!binding || !sourceMatches(binding.source, "next/navigation")) + return; + const replacement = APP_HOOK_REPLACEMENTS.get(binding.imported); + if (replacement === undefined) return; + if (allow.has(binding.imported)) return; + context.report({ + data: { hook: binding.imported, replacement }, + messageId: "rawParamsHook", + node, + }); + }, + // Surface 2: router.query on a next/router useRouter() router — the + // variable form and the direct useRouter().query form. A router passed + // across function boundaries or through props escapes detection — + // accepted cost of staying syntactic. + MemberExpression(node) { + if (allow.has("routerQuery")) return; + if ( + node.computed || + node.property.type !== AST_NODE_TYPES.Identifier || + node.property.name !== "query" + ) + return; + const { object } = node; + if (object.type === AST_NODE_TYPES.CallExpression) { + if (!isPagesUseRouterCall(object)) return; + context.report({ messageId: "rawRouterQuery", node }); + return; + } + if (object.type !== AST_NODE_TYPES.Identifier) return; + const def = resolveDef(sourceCode, object, object.name); + if (def?.node.type !== AST_NODE_TYPES.VariableDeclarator) return; + if (def.node.id.type !== AST_NODE_TYPES.Identifier) return; + if (!isPagesUseRouterCall(def.node.init)) return; + context.report({ messageId: "rawRouterQuery", node }); + }, + // Surface 2, destructured form: const { query } = useRouter(). Reported + // once on the pattern property — a destructured read has no later call + // site to anchor to, and flagging every reference of the local would + // multiply noise without adding information. Matched on the pattern + // *key*, so const { query: q } and const { query: { id } } fire too. + VariableDeclarator(node) { + if (allow.has("routerQuery")) return; + if (node.id.type !== AST_NODE_TYPES.ObjectPattern) return; + if (!isPagesUseRouterCall(node.init)) return; + for (const property of node.id.properties) { + if ( + property.type !== AST_NODE_TYPES.Property || + property.computed || + property.key.type !== AST_NODE_TYPES.Identifier || + property.key.name !== "query" + ) + continue; + context.report({ messageId: "rawRouterQuery", node: property }); + } + }, + }; + }, + defaultOptions: [{ allow: [] }], + meta: { + docs: { + description: + "Disallow raw Next.js param and search reads that bypass paramour's codecs; use the typed hooks from @paramour-js/next instead", + }, + messages: { + rawParamsHook: `{{hook}}() bypasses paramour's route validation. Use {{replacement}}(route) from "@paramour-js/next/app" instead — ${DOCS_URL}`, + rawRouterQuery: `router.query bypasses paramour's route validation. Use useRouteParams(route) and useSearch(route) from "@paramour-js/next/pages" instead — ${DOCS_URL}`, + }, + schema: [ + { + additionalProperties: false, + properties: { + allow: { + items: { + enum: ["routerQuery", "useParams", "useSearchParams"], + type: "string", + }, + type: "array", + }, + }, + type: "object", + }, + ], + type: "suggestion", + }, + name: "no-raw-param-reads", +}); diff --git a/packages/eslint-plugin/src/utils/create-rule.ts b/packages/eslint-plugin/src/utils/create-rule.ts new file mode 100644 index 0000000..f7ec39e --- /dev/null +++ b/packages/eslint-plugin/src/utils/create-rule.ts @@ -0,0 +1,9 @@ +import { ESLintUtils } from "@typescript-eslint/utils"; + +export const DOCS_URL = "https://paramour.dev/docs/reference/eslint-plugin"; + +// The docs page carries one `## ` section per rule, so the anchor +// pattern below only works while those headings match the rule names exactly. +export const createRule = ESLintUtils.RuleCreator( + (name) => `${DOCS_URL}#${name}`, +); diff --git a/packages/eslint-plugin/src/utils/imports.ts b/packages/eslint-plugin/src/utils/imports.ts new file mode 100644 index 0000000..67b4aad --- /dev/null +++ b/packages/eslint-plugin/src/utils/imports.ts @@ -0,0 +1,109 @@ +import type { TSESLint, TSESTree } from "@typescript-eslint/utils"; + +import { AST_NODE_TYPES, ASTUtils } from "@typescript-eslint/utils"; + +export interface ImportBinding { + imported: string; + source: string; +} + +/** + * Resolves a variable definition to the import specifier it binds, if any. + * Type-only imports are treated as no binding — a value usage of one is + * already a TS error, and flagging it here would be noise on top. + */ +export function getImportBinding( + def: TSESLint.Scope.Definition | undefined, +): ImportBinding | null { + if (!def) return null; + const specifier = def.node; + if (specifier.type === AST_NODE_TYPES.ImportDefaultSpecifier) { + if (specifier.parent.importKind === "type") return null; + return { imported: "default", source: specifier.parent.source.value }; + } + if (specifier.type === AST_NODE_TYPES.ImportNamespaceSpecifier) { + if (specifier.parent.importKind === "type") return null; + return { imported: "*", source: specifier.parent.source.value }; + } + if (specifier.type === AST_NODE_TYPES.ImportSpecifier) { + if (specifier.parent.type !== AST_NODE_TYPES.ImportDeclaration) return null; + if ( + specifier.importKind === "type" || + specifier.parent.importKind === "type" + ) + return null; + const { imported } = specifier; + return { + imported: + imported.type === AST_NODE_TYPES.Identifier + ? imported.name + : imported.value, + source: specifier.parent.source.value, + }; + } + return null; +} + +/** + * Resolves a call's callee to the import it binds: an identifier callee + * (named/aliased/default import) or the non-computed namespace member form + * (ns.name). Returns null for anything else, including a bare call of a + * namespace object itself. + */ +export function getImportedCallee( + sourceCode: TSESLint.SourceCode, + call: TSESTree.CallExpression, +): ImportBinding | null { + const { callee } = call; + if (callee.type === AST_NODE_TYPES.Identifier) { + const binding = getImportBinding( + resolveDef(sourceCode, callee, callee.name), + ); + if (!binding || binding.imported === "*") return null; + return binding; + } + if ( + callee.type !== AST_NODE_TYPES.MemberExpression || + callee.computed || + callee.object.type !== AST_NODE_TYPES.Identifier || + callee.property.type !== AST_NODE_TYPES.Identifier + ) + return null; + const binding = getImportBinding( + resolveDef(sourceCode, callee.object, callee.object.name), + ); + if (binding?.imported !== "*") return null; + return { imported: callee.property.name, source: binding.source }; +} + +/** + * True when the expression is a call of `imported` from one of `sources`, in + * either the identifier or the namespace-member callee form. + */ +export function isImportedCall( + sourceCode: TSESLint.SourceCode, + expr: null | TSESTree.Expression | undefined, + imported: string, + sources: readonly string[], +): boolean { + if (expr?.type !== AST_NODE_TYPES.CallExpression) return false; + const binding = getImportedCallee(sourceCode, expr); + return binding?.imported === imported && sources.includes(binding.source); +} + +/** Finds the first definition of `name` in scope at `node`. */ +export function resolveDef( + sourceCode: TSESLint.SourceCode, + node: TSESTree.Node, + name: string, +): TSESLint.Scope.Definition | undefined { + return ASTUtils.findVariable(sourceCode.getScope(node), name)?.defs[0]; +} + +/** + * Module-source match that also accepts the extensionful spelling nodenext + * resolution users write ("next/router.js" for "next/router"). + */ +export function sourceMatches(actual: string, expected: string): boolean { + return actual === expected || actual === `${expected}.js`; +} diff --git a/packages/eslint-plugin/src/utils/static-path.ts b/packages/eslint-plugin/src/utils/static-path.ts new file mode 100644 index 0000000..b997f68 --- /dev/null +++ b/packages/eslint-plugin/src/utils/static-path.ts @@ -0,0 +1,21 @@ +import type { TSESTree } from "@typescript-eslint/utils"; + +import { AST_NODE_TYPES } from "@typescript-eslint/utils"; + +/** + * Extracts the string value of a static path expression: a string literal or + * an expression-free template literal. Dynamic strings return null — out of + * scope for v1. + */ +export function getStaticPath(node: TSESTree.Node): null | string { + if (node.type === AST_NODE_TYPES.Literal && typeof node.value === "string") { + return node.value; + } + if ( + node.type === AST_NODE_TYPES.TemplateLiteral && + node.expressions.length === 0 + ) { + return node.quasis[0]?.value.cooked ?? null; + } + return null; +} diff --git a/packages/eslint-plugin/test/no-href-arithmetic.test.ts b/packages/eslint-plugin/test/no-href-arithmetic.test.ts new file mode 100644 index 0000000..f078691 --- /dev/null +++ b/packages/eslint-plugin/test/no-href-arithmetic.test.ts @@ -0,0 +1,277 @@ +import { RuleTester } from "@typescript-eslint/rule-tester"; +import { afterAll, describe, it } from "vitest"; + +import { noHrefArithmetic } from "../src/rules/no-href-arithmetic.js"; + +RuleTester.afterAll = afterAll; +RuleTester.describe = describe; +RuleTester.it = it; + +const ruleTester = new RuleTester({ + languageOptions: { parserOptions: { ecmaFeatures: { jsx: true } } }, +}); + +ruleTester.run("no-href-arithmetic", noHrefArithmetic, { + invalid: [ + // Hash suffix — the autofixable shape + { + code: `import { href } from "paramour"; +declare const route: object; +export const url = href(route) + "#top";`, + errors: [{ data: { suffix: "#top" }, messageId: "hrefHashConcat" }], + output: `import { href } from "paramour"; +declare const route: object; +export const url = href(route, { hash: "top" });`, + }, + { + code: `import { href } from "paramour"; +declare const route: object; +export const url = \`\${href(route)}#top\`;`, + errors: [{ data: { suffix: "#top" }, messageId: "hrefHashConcat" }], + output: `import { href } from "paramour"; +declare const route: object; +export const url = href(route, { hash: "top" });`, + }, + // Existing options object: hash is inserted first (perfectionist order) + { + code: `import { href } from "paramour"; +declare const route: object; +export const url = href(route, { params: { id: 1 } }) + "#reviews";`, + errors: [{ data: { suffix: "#reviews" }, messageId: "hrefHashConcat" }], + output: `import { href } from "paramour"; +declare const route: object; +export const url = href(route, { hash: "reviews", params: { id: 1 } });`, + }, + { + code: `import { href } from "paramour"; +declare const route: object; +export const url = href(route, {}) + "#a";`, + errors: [{ data: { suffix: "#a" }, messageId: "hrefHashConcat" }], + output: `import { href } from "paramour"; +declare const route: object; +export const url = href(route, { hash: "a" });`, + }, + // Namespace and aliased import forms fix too + { + code: `import * as pm from "paramour"; +declare const route: object; +export const url = pm.href(route) + "#x";`, + errors: [{ data: { suffix: "#x" }, messageId: "hrefHashConcat" }], + output: `import * as pm from "paramour"; +declare const route: object; +export const url = pm.href(route, { hash: "x" });`, + }, + { + code: `import { href as h } from "paramour"; +declare const route: object; +export const url = h(route) + "#a";`, + errors: [{ data: { suffix: "#a" }, messageId: "hrefHashConcat" }], + output: `import { href as h } from "paramour"; +declare const route: object; +export const url = h(route, { hash: "a" });`, + }, + // Fixes inside JSX expression containers + { + code: `import { href } from "paramour"; +declare const route: object; +export const el = ;`, + errors: [{ data: { suffix: "#top" }, messageId: "hrefHashConcat" }], + output: `import { href } from "paramour"; +declare const route: object; +export const el = ;`, + }, + // Fixes are shape-local: the template fix lands in one pass; the "?b" + // still concatenated afterward surfaces on the next lint pass — standard + // ESLint fixpoint behavior. + { + code: `import { href } from "paramour"; +declare const route: object; +export const url = \`\${href(route)}#a\` + "?b";`, + errors: [{ data: { suffix: "#a" }, messageId: "hrefHashConcat" }], + output: `import { href } from "paramour"; +declare const route: object; +export const url = href(route, { hash: "a" }) + "?b";`, + }, + // Query suffixes teach the search option — never autofixed (the appended + // params need codec keys) + { + code: `import { href } from "paramour"; +declare const route: object; +export const url = href(route) + "?tab=1";`, + errors: [{ data: { suffix: "?tab=1" }, messageId: "hrefSearchConcat" }], + output: null, + }, + { + code: `import { href } from "paramour"; +declare const route: object; +export const url = \`\${href(route)}?page=2\`;`, + errors: [{ data: { suffix: "?page=2" }, messageId: "hrefSearchConcat" }], + output: null, + }, + // Dynamic or multi-part suffixes get the generic message + { + code: `import { href } from "paramour"; +declare const route: object; +declare const suffix: string; +export const url = href(route) + suffix;`, + errors: [{ messageId: "hrefConcat" }], + output: null, + }, + { + code: `import { href } from "paramour"; +declare const route: object; +declare const child: string; +export const url = \`\${href(route)}/\${child}\`;`, + errors: [{ messageId: "hrefConcat" }], + output: null, + }, + { + code: `import { href } from "paramour"; +declare const route: object; +declare const x: string; +export const url = href(route) + "#a" + x;`, + errors: [{ messageId: "hrefConcat" }], + output: null, + }, + { + code: `import { href } from "paramour"; +declare const route: object; +declare const frag: string; +export const url = \`\${href(route)}#\${frag}\`;`, + errors: [{ messageId: "hrefConcat" }], + output: null, + }, + { + code: `import { href } from "paramour"; +declare const a: object; +declare const b: object; +export const url = href(a) + href(b);`, + errors: [{ messageId: "hrefConcat" }], + output: null, + }, + // Hash suffix, but the chain isn't exactly call-plus-literal — no fix + { + code: `import { href } from "paramour"; +declare const route: object; +declare const x: string; +export const url = x + href(route) + "#t";`, + errors: [{ data: { suffix: "#t" }, messageId: "hrefHashConcat" }], + output: null, + }, + { + code: `import { href } from "paramour"; +declare const route: object; +declare const origin: string; +export const url = \`\${origin}\${href(route)}#top\`;`, + errors: [{ data: { suffix: "#top" }, messageId: "hrefHashConcat" }], + output: null, + }, + { + code: `import { href } from "paramour"; +declare const route: object; +export const url = \`\${href(route)}\` + "#top";`, + errors: [{ data: { suffix: "#top" }, messageId: "hrefHashConcat" }], + output: null, + }, + // Fixer bail-outs: existing hash, spread, non-object options, trailing + // comma, comments outside the call + { + code: `import { href } from "paramour"; +declare const route: object; +export const url = href(route, { hash: "a" }) + "#b";`, + errors: [{ data: { suffix: "#b" }, messageId: "hrefHashConcat" }], + output: null, + }, + { + code: `import { href } from "paramour"; +declare const route: object; +declare const opts: object; +export const url = href(route, { ...opts }) + "#b";`, + errors: [{ data: { suffix: "#b" }, messageId: "hrefHashConcat" }], + output: null, + }, + { + code: `import { href } from "paramour"; +declare const route: object; +declare const opts: object; +export const url = href(route, opts) + "#b";`, + errors: [{ data: { suffix: "#b" }, messageId: "hrefHashConcat" }], + output: null, + }, + { + code: `import { href } from "paramour"; +declare const route: object; +export const url = href(route,) + "#a";`, + errors: [{ data: { suffix: "#a" }, messageId: "hrefHashConcat" }], + output: null, + }, + { + code: `import { href } from "paramour"; +declare const route: object; +export const url = href(route) /* keep */ + "#a";`, + errors: [{ data: { suffix: "#a" }, messageId: "hrefHashConcat" }], + output: null, + }, + // A bare "#" appends an empty fragment — generic, nothing to derive + { + code: `import { href } from "paramour"; +declare const route: object; +export const url = href(route) + "#";`, + errors: [{ messageId: "hrefConcat" }], + output: null, + }, + ], + valid: [ + // Prefix-only concatenation is the legitimate absolute-URL pattern + `import { href } from "paramour"; +declare const route: object; +export const url = "https://example.com" + href(route);`, + `import { href } from "paramour"; +declare const route: object; +declare const origin: string; +export const url = \`\${origin}\${href(route)}\`;`, + // Bare results + `import { href } from "paramour"; +declare const route: object; +export const url = href(route);`, + `import { href } from "paramour"; +declare const route: object; +export const url = \`\${href(route)}\`;`, + // href from another module — never fires + `import { href } from "./urls"; +declare const route: object; +export const url = href(route) + "#a";`, + // Shadowed import resolves to the inner binding + `import { href } from "paramour"; +declare const route: object; +export function f() { + const href = (x: object) => String(x); + return href(route) + "#a"; +}`, + // Type-only import — a value usage is already a TS error + `import type { href } from "paramour"; +declare const route: object; +export const url = href(route) + "#a";`, + // Tagged templates have unknown semantics + `import { href } from "paramour"; +declare const route: object; +declare function tag(strings: TemplateStringsArray, ...values: string[]): string; +export const url = tag\`\${href(route)}#top\`;`, + // Flows the rule deliberately does not follow (LP4 boundary costs) + `import { href } from "paramour"; +declare const route: object; +export const url = href(route).concat("#a");`, + `import { href } from "paramour"; +declare const route: object; +export const url = [href(route), "#a"].join("");`, + `import { href } from "paramour"; +declare const route: object; +const base = href(route); +export const url = base + "#a";`, + // Non-concatenation operators + `import { href } from "paramour"; +declare const route: object; +declare const x: string; +export const eq = href(route) === x;`, + ], +}); diff --git a/packages/eslint-plugin/test/no-raw-param-reads.test.ts b/packages/eslint-plugin/test/no-raw-param-reads.test.ts new file mode 100644 index 0000000..8241786 --- /dev/null +++ b/packages/eslint-plugin/test/no-raw-param-reads.test.ts @@ -0,0 +1,254 @@ +import { RuleTester } from "@typescript-eslint/rule-tester"; +import { afterAll, describe, it } from "vitest"; + +import { noRawParamReads } from "../src/rules/no-raw-param-reads.js"; + +RuleTester.afterAll = afterAll; +RuleTester.describe = describe; +RuleTester.it = it; + +const ruleTester = new RuleTester({ + languageOptions: { parserOptions: { ecmaFeatures: { jsx: true } } }, +}); + +ruleTester.run("no-raw-param-reads", noRawParamReads, { + invalid: [ + // Surface 1: App Router hooks + { + code: `import { useSearchParams } from "next/navigation"; +export function C() { + const params = useSearchParams(); + return params.get("q"); +}`, + errors: [ + { + data: { hook: "useSearchParams", replacement: "useSearch" }, + messageId: "rawParamsHook", + }, + ], + }, + { + code: `import { useParams } from "next/navigation"; +export function C() { + const params = useParams(); + return params.id; +}`, + errors: [ + { + data: { hook: "useParams", replacement: "useRouteParams" }, + messageId: "rawParamsHook", + }, + ], + }, + // Aliased import reports the canonical imported name + { + code: `import { useParams as useP } from "next/navigation"; +export const params = useP();`, + errors: [ + { + data: { hook: "useParams", replacement: "useRouteParams" }, + messageId: "rawParamsHook", + }, + ], + }, + // Namespace form + { + code: `import * as nav from "next/navigation"; +export const params = nav.useSearchParams();`, + errors: [ + { + data: { hook: "useSearchParams", replacement: "useSearch" }, + messageId: "rawParamsHook", + }, + ], + }, + // Extensionful nodenext spelling + { + code: `import { useParams } from "next/navigation.js"; +export const params = useParams();`, + errors: [ + { + data: { hook: "useParams", replacement: "useRouteParams" }, + messageId: "rawParamsHook", + }, + ], + }, + // Surface 2: router.query, variable form + { + code: `import { useRouter } from "next/router"; +export function C() { + const router = useRouter(); + return router.query.id; +}`, + errors: [{ messageId: "rawRouterQuery" }], + }, + { + code: `import { useRouter } from "next/router.js"; +export function C() { + const router = useRouter(); + return router.query.id; +}`, + errors: [{ messageId: "rawRouterQuery" }], + }, + // Direct call form + { + code: `import { useRouter } from "next/router"; +export function C() { + return useRouter().query; +}`, + errors: [{ messageId: "rawRouterQuery" }], + }, + // Aliased useRouter + { + code: `import { useRouter as useR } from "next/router"; +export function C() { + const r = useR(); + return r.query; +}`, + errors: [{ messageId: "rawRouterQuery" }], + }, + // Namespace-qualified useRouter() + { + code: `import * as R from "next/router"; +export function C() { + const router = R.useRouter(); + return router.query; +}`, + errors: [{ messageId: "rawRouterQuery" }], + }, + // Surface 2, destructured forms — reported on the pattern property + { + code: `import { useRouter } from "next/router"; +export function C() { + const { query } = useRouter(); + return query.id; +}`, + errors: [{ messageId: "rawRouterQuery" }], + }, + { + code: `import { useRouter } from "next/router"; +export function C() { + const { query: q } = useRouter(); + return q.id; +}`, + errors: [{ messageId: "rawRouterQuery" }], + }, + { + code: `import { useRouter } from "next/router"; +export function C() { + const { query: { id } } = useRouter(); + return id; +}`, + errors: [{ messageId: "rawRouterQuery" }], + }, + // Partial allow: unlisted surfaces still fire + { + code: `import { useSearchParams } from "next/navigation"; +export const params = useSearchParams();`, + errors: [ + { + data: { hook: "useSearchParams", replacement: "useSearch" }, + messageId: "rawParamsHook", + }, + ], + options: [{ allow: ["useParams"] }], + }, + // Multiple violations in one file, with report locations + { + code: `import { useParams } from "next/navigation"; +import { useRouter } from "next/router"; +export function C() { + const params = useParams(); + const router = useRouter(); + return [params.id, router.query.id]; +}`, + errors: [ + { + column: 18, + data: { hook: "useParams", replacement: "useRouteParams" }, + line: 4, + messageId: "rawParamsHook", + }, + { + column: 22, + line: 6, + messageId: "rawRouterQuery", + }, + ], + }, + ], + valid: [ + // Same-name hooks from other modules — the headline non-goal + `import { useParams } from "react-router-dom"; +export const params = useParams();`, + `import { useSearchParams } from "react-router-dom"; +export const params = useSearchParams();`, + // Locally defined and shadowed names never fire + `function useParams() { return {}; } +export const params = useParams();`, + `import { useParams } from "next/navigation"; +export function C() { + const useParams = () => ({}); + return useParams(); +}`, + // Type-only import — a value usage is already a TS error + `import type { useParams } from "next/navigation"; +export const params = useParams();`, + // App Router useRouter() has no .query — the source check keeps this out + `import { useRouter } from "next/navigation"; +export function C() { + const router = useRouter(); + return router.query; +}`, + // Writes are no-raw-hrefs' surface, not this rule's + `import { useRouter } from "next/router"; +export function go() { + const router = useRouter(); + router.push("/a"); +}`, + // Other destructured keys are untyped-read-free + `import { useRouter } from "next/router"; +export function C() { + const { pathname } = useRouter(); + return pathname; +}`, + // Computed access — out of scope for a syntactic rule + `import { useRouter } from "next/router"; +export function C() { + const router = useRouter(); + return router["query"]; +}`, + // query-shaped members on non-routers + `declare const db: { query: (sql: string) => unknown }; +export const rows = db.query("select 1");`, + // Namespace import of another module + `import * as nav from "./params"; +export const params = nav.useParams();`, + // A router crossing a function boundary escapes detection — accepted + // cost of staying syntactic + `import type { NextRouter } from "next/router"; +export function read(router: NextRouter) { + return router.query; +}`, + // allow silences each surface + { + code: `import { useSearchParams } from "next/navigation"; +export const params = useSearchParams();`, + options: [{ allow: ["useSearchParams"] }], + }, + { + code: `import { useParams } from "next/navigation"; +export const params = useParams();`, + options: [{ allow: ["useParams"] }], + }, + { + code: `import { useRouter } from "next/router"; +export function C() { + const router = useRouter(); + const { query } = useRouter(); + return [router.query, query]; +}`, + options: [{ allow: ["routerQuery"] }], + }, + ], +});