From fe5e1d590a603dff53516729ec7fe6df7345fec5 Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Sat, 10 Oct 2026 08:12:09 +0100 Subject: [PATCH 1/2] feat!: make exadevConfig take the options object alone `exadevConfig(options, ...userConfigs)` forced a caller who only wanted to append their own blocks to pass an empty options object first, and a block passed in the options slot was typed as options and its keys silently ignored. The factory now takes `options` only and returns the config array; callers append their own blocks by spreading the result inside `defineConfig`. The README, the factory's tests and the tests of each option that previously exercised trailing blocks follow the new shape. The default export is still `exadevConfig()` evaluated once. Migration: `exadevConfig({}, a, b)` becomes `defineConfig(...exadevConfig(), a, b)`, and `exadevConfig({ react: false }, a)` becomes `defineConfig(...exadevConfig({ react: false }), a)`. BREAKING CHANGE: `exadevConfig` no longer accepts config blocks after the options argument. Spread its result inside `defineConfig` and put your own blocks after it. --- README.md | 6 +++--- src/create-config.ts | 4 +--- src/create-config.unit.test.ts | 20 ++++++-------------- src/import-policy.unit.test.ts | 8 ++------ src/pure-modules.unit.test.ts | 8 ++------ src/test-hygiene.unit.test.ts | 8 ++------ 6 files changed, 16 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index 81177d9c..fa607543 100644 --- a/README.md +++ b/README.md @@ -209,7 +209,7 @@ export default tseslint.config( | [Required Markdown headings](#required-markdown-headings) | Off unless given (only a repository can say which documents need which headings); needs the optional peer `@eslint/markdown` | `exadevConfig({ markdownHeadings })` / `markdownHeadingsConfig(options)` | | [Agent skills and plugin marketplaces](#agent-skills-and-plugin-marketplaces) | Auto-detected: on if the project has a `.claude-plugin/marketplace.json`, a `skills//SKILL.md` or a `plugins//skills//SKILL.md` (symbolic links to those directories count); the SKILL.md rules need the optional peer `@eslint/markdown` and the manifest rules `@eslint/json` | `exadevConfig({ agentSkills })` / `agentSkillsConfig(options)` | -Every tri-state option above (`true`/`false`/`undefined`) is passed through the named `exadevConfig(options, ...userConfigs)` factory export: +Every tri-state option above (`true`/`false`/`undefined`) is passed through the named `exadevConfig(options)` factory export: ```ts // eslint.config.ts @@ -227,7 +227,7 @@ export default defineConfig( ); ``` -Trailing arguments are arbitrary flat-config objects, appended in order after everything else — `exadevConfig({}, { rules: { 'no-console': 'warn' } })` is equivalent to spreading the default export plus one more config object. `import exadev from '@exadev/eslint-config'` (the default export) is just `exadevConfig()` called with no arguments. +`exadevConfig` takes the options object alone and returns a config array, so your own blocks go after the spread inside `defineConfig`, as above: `defineConfig(...exadevConfig(), { rules: { 'no-console': 'warn' } })` is the default export plus one more block. Passing a block as a second argument is a type error. `import exadev from '@exadev/eslint-config'` (the default export) is just `exadevConfig()` called with no arguments. ### Optional React and Next.js support @@ -1769,7 +1769,7 @@ pnpm build - [`src/rules/run-tracker.ts`](src/rules/run-tracker.ts) decides which lint carries a finding that belongs to a set of files rather than to one, so `require-compiler-options` reports once per run; it reads run boundaries from the order of the lints, as [Compiler options](#compiler-options) describes. - [`src/rules/tsconfig-attribution.ts`](src/rules/tsconfig-attribution.ts) works out which tsconfig governs a linted file, attributing a program with no tsconfig to the one the file was last linted under while that tsconfig still lists it. - [`src/create-config.ts`](src/create-config.ts) is config assembly's single source of truth. - - `exadevConfig(options, ...userConfigs)` concatenates, in order: `buildGitignoreConfig`, `recommendedTypeChecked`, `jsdocAndTsdoc`, `jsonCanonicalConfig`, `stylisticCommentsConfig`, `buildReactConfig`, `buildNextjsConfig`, `buildPackageJsonKeyOrderConfig` (each tri-state builder fed its matching option), `buildWorkspaceArchitectureConfig` (only when `workspaceArchitecture` is given; there is no auto-detected default), `buildTurboConfig` (only when `turbo` is given, likewise), `buildImportPolicyConfig` (only when `importPolicies` is given, likewise), `buildPureModulesConfig` (only when `pureModules` is given, likewise), `buildTestHygieneConfig` (only when `testHygiene` is given, likewise), `buildMarkdownHeadingsConfig` (only when `markdownHeadings` is given, likewise), `buildAgentSkillsConfig` (tri-state, placed after the headings block so that a SKILL.md stays YAML whatever format `markdownHeadings` sets), `buildToolingWiringConfig` (only when `toolingWiring` is given, likewise), and any trailing user configs. + - `exadevConfig(options)` concatenates, in order: `buildGitignoreConfig`, `recommendedTypeChecked`, `jsdocAndTsdoc`, `jsonCanonicalConfig`, `stylisticCommentsConfig`, `buildReactConfig`, `buildNextjsConfig`, `buildPackageJsonKeyOrderConfig` (each tri-state builder fed its matching option), `buildWorkspaceArchitectureConfig` (only when `workspaceArchitecture` is given; there is no auto-detected default), `buildTurboConfig` (only when `turbo` is given, likewise), `buildImportPolicyConfig` (only when `importPolicies` is given, likewise), `buildPureModulesConfig` (only when `pureModules` is given, likewise), `buildTestHygieneConfig` (only when `testHygiene` is given, likewise), `buildMarkdownHeadingsConfig` (only when `markdownHeadings` is given, likewise), `buildAgentSkillsConfig` (tri-state, placed after the headings block so that a SKILL.md stays YAML whatever format `markdownHeadings` sets), and `buildToolingWiringConfig` (only when `toolingWiring` is given, likewise). - `defaultConfig` is `exadevConfig()` evaluated once, eagerly, at module load. - [`src/robustness-rules.ts`](src/robustness-rules.ts) holds `UNTYPED_ROBUSTNESS_RULES`, the core ESLint rules that need no type information, spread into both `plugin.configs.recommended` and the type-checked block so the two cannot drift apart. - [`src/stylistic-comments.ts`](src/stylistic-comments.ts) builds `stylisticCommentsConfig`: the hand-picked `@stylistic/eslint-plugin` rules plus this package's own `exadev/multiline-comment-style` and `exadev/prefer-doc-comment`, in two blocks (one scoped to every JS/TS file, one scoped to JSX files only for the three JSX-specific rules). See [Stylistic comment, class-member and JSX rules](#stylistic-comment-class-member-and-jsx-rules). diff --git a/src/create-config.ts b/src/create-config.ts index 544f507b..48cf867f 100644 --- a/src/create-config.ts +++ b/src/create-config.ts @@ -1,4 +1,3 @@ -import type { TSESLint } from '@typescript-eslint/utils'; import { buildAgentSkillsConfig, type AgentSkillsOptions } from './agent-skills'; import type { ConfigArrayValue, PublicConfigArray } from './config-types'; import { buildGitignoreConfig } from './gitignore'; @@ -68,7 +67,7 @@ function withWorkspaceGroups(turbo: TurboOptions, workspaceArchitecture: Workspa * * The tri-state per feature threads straight into each builder's own `enabled` option: true forces on (throwing if the underlying peer isn't resolvable), false forces off (skipping resolution entirely), undefined auto-detects (silently empty if unresolvable, or if an equivalent tool, syncpack for packageJsonKeyOrder, or a project's own .gitignore for gitignore, already does the job). One resolution pass per feature; no separate pre-check gate that would resolve twice. */ -export function exadevConfig(options: ExadevConfigOptions = {}, ...userConfigs: readonly TSESLint.FlatConfig.Config[]): PublicConfigArray { +export function exadevConfig(options: ExadevConfigOptions = {}): PublicConfigArray { const gitignore = buildGitignoreConfig({ enabled: options.gitignore }); const built: ConfigArrayValue = [ ...gitignore, @@ -89,7 +88,6 @@ export function exadevConfig(options: ExadevConfigOptions = {}, ...userConfigs: // After markdownHeadings: both blocks set the frontmatter format of the Markdown they select, and a SKILL.md is always YAML, so the later block must be this one. ...buildAgentSkillsConfig(options.agentSkills, { ignores: gitignore.flatMap(({ ignores = [] }) => ignores) }), ...(options.toolingWiring !== undefined ? buildToolingWiringConfig(options.toolingWiring) : []), - ...userConfigs, ]; return toPublicConfigArray(built); diff --git a/src/create-config.unit.test.ts b/src/create-config.unit.test.ts index df4d59e5..8e335446 100644 --- a/src/create-config.unit.test.ts +++ b/src/create-config.unit.test.ts @@ -1,4 +1,3 @@ -import type { TSESLint } from '@typescript-eslint/utils'; import { describe, expect, it } from 'vitest'; import { exadevConfig } from './create-config'; import jsdocAndTsdoc from './jsdoc'; @@ -59,14 +58,12 @@ describe('exadevConfig', () => { expect(hasEnvRule(exadevConfig({ ...base, turboEnv: false }))).toBe(false); }); - it('turbo, when given, wires in turboConfig\'s blocks, before any trailing user configs', () => { + it('turbo, when given, wires in turboConfig\'s blocks', () => { const TURBO_BLOCK_COUNT = 2; - const extra: TSESLint.FlatConfig.Config = { rules: { 'no-console': 'warn' } }; const base = { react: false, nextjs: false, packageJsonKeyOrder: false, gitignore: false } as const; - const withoutTurbo = exadevConfig(base, extra); - const withTurbo = exadevConfig({ ...base, turbo: {} }, extra); + const withoutTurbo = exadevConfig(base); + const withTurbo = exadevConfig({ ...base, turbo: {} }); expect(withTurbo).toHaveLength(withoutTurbo.length + TURBO_BLOCK_COUNT); - expect(withTurbo.at(-1)).toBe(extra); const hasTurboRule = (blocks: typeof withTurbo) => blocks.some((block) => block.rules !== undefined && 'exadev/turbo-task-outputs' in block.rules); expect(hasTurboRule(withTurbo)).toBe(true); expect(hasTurboRule(withoutTurbo)).toBe(false); @@ -102,13 +99,8 @@ describe('exadevConfig', () => { }); }); - it('appends trailing user configs, in order, after everything else', () => { - const extraA: TSESLint.FlatConfig.Config = { rules: { 'no-console': 'warn' } }; - const extraB: TSESLint.FlatConfig.Config = { files: ['**/*.spec.ts'] }; - const result = exadevConfig({}, extraA, extraB); - const SECOND_TO_LAST = -2; - const LAST = -1; - expect(result.at(SECOND_TO_LAST)).toBe(extraA); - expect(result.at(LAST)).toBe(extraB); + it('rejects a config block passed after the options', () => { + // @ts-expect-error a second argument is no longer accepted: callers spread the result and append their own blocks + exadevConfig({}, { rules: {} }); }); }); diff --git a/src/import-policy.unit.test.ts b/src/import-policy.unit.test.ts index 1bf33008..389386d2 100644 --- a/src/import-policy.unit.test.ts +++ b/src/import-policy.unit.test.ts @@ -38,12 +38,8 @@ describe('exadevConfig importPolicies', () => { const base = { react: false, nextjs: false, turboEnv: false, packageJsonKeyOrder: false, gitignore: false } as const; const hasPolicyRule = (blocks: ReturnType) => blocks.some((block) => block.rules !== undefined && 'exadev/import-policy' in block.rules); - it('adds the policy block only when policies are given, before any trailing user config', () => { + it('adds the policy block only when policies are given', () => { expect(hasPolicyRule(exadevConfig(base))).toBe(false); - const extra = { name: 'user' }; - const withPolicies = exadevConfig({ ...base, importPolicies: POLICIES }, extra); - expect(hasPolicyRule(withPolicies)).toBe(true); - expect(withPolicies.at(-1)).toBe(extra); - expect(hasPolicyRule(withPolicies.slice(0, -1))).toBe(true); + expect(hasPolicyRule(exadevConfig({ ...base, importPolicies: POLICIES }))).toBe(true); }); }); diff --git a/src/pure-modules.unit.test.ts b/src/pure-modules.unit.test.ts index 7ddfbc1c..9df5ac99 100644 --- a/src/pure-modules.unit.test.ts +++ b/src/pure-modules.unit.test.ts @@ -105,12 +105,8 @@ describe('exadevConfig pureModules', () => { const base = { react: false, nextjs: false, turboEnv: false, packageJsonKeyOrder: false, gitignore: false } as const; const hasPureRule = (blocks: ReturnType) => blocks.some((block) => block.rules !== undefined && 'exadev/pure-module' in block.rules); - it('adds the pure-module block only when the option is given, before any trailing user config', () => { + it('adds the pure-module block only when the option is given', () => { expect(hasPureRule(exadevConfig(base))).toBe(false); - const extra = { name: 'user' }; - const withPure = exadevConfig({ ...base, pureModules: { files: FILES } }, extra); - expect(hasPureRule(withPure)).toBe(true); - expect(withPure.at(-1)).toBe(extra); - expect(hasPureRule(withPure.slice(0, -1))).toBe(true); + expect(hasPureRule(exadevConfig({ ...base, pureModules: { files: FILES } }))).toBe(true); }); }); diff --git a/src/test-hygiene.unit.test.ts b/src/test-hygiene.unit.test.ts index d661555b..75167bf2 100644 --- a/src/test-hygiene.unit.test.ts +++ b/src/test-hygiene.unit.test.ts @@ -194,12 +194,8 @@ describe('exadevConfig testHygiene', () => { const base = { react: false, nextjs: false, turboEnv: false, packageJsonKeyOrder: false, gitignore: false } as const; const hasHygiene = (blocks: ReturnType) => blocks.some((block) => block.rules !== undefined && 'exadev/non-vacuous-guard' in block.rules); - it('adds the blocks only when the option is given, before any trailing user config', () => { + it('adds the blocks only when the option is given', () => { expect(hasHygiene(exadevConfig(base))).toBe(false); - const extra = { name: 'user' }; - const withHygiene = exadevConfig({ ...base, testHygiene: {} }, extra); - expect(hasHygiene(withHygiene)).toBe(true); - expect(withHygiene.at(-1)).toBe(extra); - expect(hasHygiene(withHygiene.slice(0, -1))).toBe(true); + expect(hasHygiene(exadevConfig({ ...base, testHygiene: {} }))).toBe(true); }); }); From db572e155871695d9a6174a1f3af48853c9cfd3d Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Sat, 10 Oct 2026 08:08:40 +0100 Subject: [PATCH 2/2] feat: report an optional parameter directly before a rest parameter A function whose parameter list ends with an optional parameter followed by a rest parameter forces a caller who wants only the rest values to pass a placeholder for the optional one. The trailing optional run has length one, so `minTrailingOptional` (schema minimum 2) never saw it. `exadev/prefer-options-object-param` now reports that shape under its own message id, `placeholderBeforeRest`, and offers no suggestion because the right shape (the rest values alone, or one object holding the optional value and the rest values) is a design decision. Runs that reach `minTrailingOptional` keep their existing report. --- README.md | 2 +- src/rules/prefer-options-object-param.ts | 22 ++++- .../prefer-options-object-param.unit.test.ts | 83 ++++++++++++++++++- 3 files changed, 100 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index fa607543..cb49517e 100644 --- a/README.md +++ b/README.md @@ -390,7 +390,7 @@ Bundled into `exadevConfig()`'s default output the same way React/Next.js auto-d | `no-enum-number-widening` | | **A bare `number` is accepted anywhere a numeric enum is expected**, without checking it is actually one of the enum's members — only a numeric *literal* gets range-checked by `tsc`. No autofix: the only provably safe fix is a genuine runtime membership check against the enum's own values, which is a behavioural choice a mechanical fix cannot responsibly make. Requires type information — only in the default (type-checked) export, not `plugin.configs.recommended`. | | `no-enum-reverse-lookup-widening` | suggestion | **A numeric enum's reverse lookup can silently type as `string` for an out-of-range index.** Indexing a numeric enum's reverse mapping (`Direction[n]`) with a bare (non-literal) `number`, or with a different enum's member, types as plain `string` for any index, including one outside the enum's actual members, where it genuinely returns `undefined` at runtime — `tsc` does not range-check even a numeric literal index here. When the indexed expression is the init of a variable with an explicit `: string` annotation, a suggestion widens it to `: string \| undefined`, forcing later uses as a bare `string` to surface as real compile errors; every other syntactic position gets a plain report with no fix, and no case gets a full `--fix` autofix. Requires type information — only in the default (type-checked) export, not `plugin.configs.recommended`. | | `prefer-numeric-sort-compare` | suggestion | **`.sort()` on a number array sorts lexicographically by default.** A deliberately narrow addition alongside [`@typescript-eslint/require-array-sort-compare`](https://typescript-eslint.io/rules/require-array-sort-compare/) (which already flags any bare `.sort()`/`.toSorted()` except on a plain string array, with no fix): when the array's element type is definitively `number`, a suggestion offers an ascending compare function (`(a, b) => a - b`), since the default comparator sorts lexicographically (`[1, 2, 10].sort()` becomes `[1, 10, 2]`). Not a full autofix — descending order is a real, if less common, alternative intent. Requires type information — only in the default (type-checked) export, not `plugin.configs.recommended`, since it needs the checker to confirm the array's element type. | -| `prefer-options-object-param` | suggestion | **A run of 2+ trailing optional parameters (`?`-marked or default-valued) should be bundled into one destructured `options` parameter.** Without this, a caller needing only the last optional parameter must still pass `undefined` for every optional parameter before it (the motivating case: a 10-parameter constructor with 8 trailing optional ones). The threshold is configurable (`{ minTrailingOptional }`, default `2`). A suggestion (not a full autofix) rewrites the parameter list and inserts a `const { ... } = options ?? {};` destructure as the body's first statement; call sites are never rewritten, since the signature edit alone turns every stale positional call site into a real compile error. Still reported, but with no suggestion offered, whenever collapsing the run would not be safe/mechanical: a parameter property in the run, a parameter with no simple resolvable name and explicit type of its own (including a destructured parameter, or one typed only through a separately-declared function-type alias), a decorated parameter, a rest parameter anywhere in the full parameter list, no `BlockStatement` body at all (an arrow's expression body, or any declaration-only signature — including an ambient `.d.ts` function, an interface method/construct signature, or an abstract/ambient class method), a `@param` JSDoc tag naming a parameter in the run, or a parameter/function-scope-local variable already named `options`. Complements `max-params` (see "Individual rule tuning" above), which catches the different shape (excessive REQUIRED parameters) this rule is deliberately blind to. Requires no type information — registered in both `plugin.configs.recommended` and the default (type-checked) export, like `prefer-readonly-array-param` above; unlike its own `prefer-readonly-object-param`/`prefer-numeric-sort-compare` siblings, which genuinely need the checker and so cannot be offered outside the default export at all. | +| `prefer-options-object-param` | suggestion | **A run of 2+ trailing optional parameters (`?`-marked or default-valued) should be bundled into one destructured `options` parameter, and an optional parameter should not sit directly before a rest parameter.** Without this, a caller needing only the last optional parameter must still pass `undefined` for every optional parameter before it (the motivating case: a 10-parameter constructor with 8 trailing optional ones). The threshold is configurable (`{ minTrailingOptional }`, default `2`). A suggestion (not a full autofix) rewrites the parameter list and inserts a `const { ... } = options ?? {};` destructure as the body's first statement; call sites are never rewritten, since the signature edit alone turns every stale positional call site into a real compile error. Still reported, but with no suggestion offered, whenever collapsing the run would not be safe/mechanical: a parameter property in the run, a parameter with no simple resolvable name and explicit type of its own (including a destructured parameter, or one typed only through a separately-declared function-type alias), a decorated parameter, a rest parameter anywhere in the full parameter list, no `BlockStatement` body at all (an arrow's expression body, or any declaration-only signature — including an ambient `.d.ts` function, an interface method/construct signature, or an abstract/ambient class method), a `@param` JSDoc tag naming a parameter in the run, or a parameter/function-scope-local variable already named `options`. An optional parameter directly before a rest parameter is reported as well, even when it is the only optional one (`(options = {}, ...blocks)`), because a caller wanting only the rest values must pass a placeholder for it; that report has its own message and no suggestion, since the right shape (the rest values alone, or one object holding the optional value and the rest values) is a design decision. Complements `max-params` (see "Individual rule tuning" above), which catches the different shape (excessive REQUIRED parameters) this rule is deliberately blind to. Requires no type information — registered in both `plugin.configs.recommended` and the default (type-checked) export, like `prefer-readonly-array-param` above; unlike its own `prefer-readonly-object-param`/`prefer-numeric-sort-compare` siblings, which genuinely need the checker and so cannot be offered outside the default export at all. | | `prefer-doc-comment` | ✓ | **A substantial leading comment on an exported declaration should be a doc comment, not a plain `//`/`/* */` comment.** "Substantial" means 2 or more comment lines, or a single line whose text exceeds a configurable length (`{ maxLineLength }`, default `80`). Covers an exported function declaration, an exported function's own signature (a body-less `TSDeclareFunction`, an ordinary overload signature and a genuinely ambient one alike, `declare function foo(): void;` included, since TypeScript never shows either one's own implementation to callers, only the signature itself), an exported `const` declaration, whatever its initialiser (a function expression or arrow function, an ordinary value such as `export const TIMEOUT_MS = 5000;`, or none at all in an ambient `export declare const x: number;`; reported once even when the statement declares several, e.g. `export const h1 = () => {}, h2 = () => {};`), an exported class declaration, ambient (`export declare class`) or ordinary alike (TypeScript strips only an ambient class's own MEMBER bodies, never its declaration shape, so its leading comment documents the identical public contract an ordinary exported class's does), an exported class's public (non-`private`/`protected`/`#`-private) method (covering both a method's own overload signature and its trailing implementation) and public abstract method (`abstract m(): void;`, needing no equivalent overload-implementation exemption of its own, since TypeScript's own grammar never lets an abstract method carry a body at all), an exported class's public function/arrow-valued property (the class-member equivalent of the function-initialised `const` shape above), an exported interface/type-alias declaration, an exported enum declaration (`const enum` alike; never an individual member, the identical reason an interface's members are never individually checked), an exported namespace/module declaration (ambient or ordinary alike, at any nesting depth whose enclosing namespaces are all exported themselves), and a default-exported expression (`export default (): number => 1;`, `export default 42;`; a default-exported `function`/`class` declaration or body-less signature is covered by its own shape above and reported exactly once, never twice); a non-exported declaration is never reported, regardless of its comment. A TypeScript function/method overload set's own body-carrying IMPLEMENTATION signature is never reported either, however substantial its own comment: TypeScript never shows that signature to callers at all, only the separate overload signatures above it (a body-less `TSDeclareFunction` for a function, written without `declare`, or a `MethodDefinition` whose own `value` is `TSEmptyBodyFunctionExpression` for a class method) are ever checked against a call site, and both of those are themselves reported like any other declaration in this rule's scope, as the overload set's own real public contract; a comment directly above the implementation is instead internal reasoning about how the overloads are actually realised, never a public contract. The implementation is detected via ESLint's own scope analysis for a function (the overload signatures and the implementation all share one Variable) and, for a class method, via an EARLIER (source-order), same-`static`-ness, same-key signature sibling: a same-named `static`/instance pair is never one overload set to TypeScript itself, and a same-named signature declared AFTER the implementation belongs to a different, later member, never the one the implementation itself realises, so neither is ever mistaken for the implementation's own overload signature. A trailing comment on the previous line of code (`const x = 1; // note`) is never treated as the next declaration's own leading comment. A directive-shaped line (the full ESLint directive/config-comment family, `eslint-disable`/`eslint-disable-line`/`eslint-disable-next-line`/`eslint-enable`/`eslint`/`eslint-env`/`global`/`globals`/`exported`, matched case-sensitively, exactly as ESLint's own directive parser matches them, each requiring real whitespace or end-of-string immediately after the keyword rather than a bare word boundary, so an ordinary compound word such as `eslint-plugin-x` is never mistaken for one, and, on a `//` Line comment specifically, recognised only as `eslint-disable-line`/`eslint-disable-next-line`, the two labels ESLint itself honours there; every other member of the family is a directive only in Block form, matching what ESLint itself does; `@ts-expect-error`/`@ts-ignore`/`@ts-nocheck`/`@ts-check`, the `ts-*` markers matched with or without their own leading `@`; `TODO`/`FIXME`, matched case-insensitively when all-uppercase or immediately followed by `:`/`(` (`todo:`, `fixme(scope):`), never a `Todo`/`Fixme` opening ordinary prose; `prettier-ignore`; `c8 ignore`/`v8 ignore`/`istanbul ignore`/`node:coverage ignore`/`node:coverage disable`/`node:coverage enable`; `cspell:disable`/`cspell:disable-line`/`cspell:disable-next-line`/`cspell:enable` (never cspell's own `ignore`/`words` directives, which name dictionary words rather than suppressing a stretch of text); `biome-ignore`; `#region`/`#endregion`, an editor folding marker; and a managed-region marker, a comment of exactly `::start` or `::end` such as `content:claude:start`, which a tool locates by its exact line) found anywhere in a `//` run is never itself merged into the fix: only the prose segment directly adjacent to the declaration, the run of lines after the LAST such directive, is ever reported and converted; every earlier segment, and every directive line itself, is left completely untouched, since an earlier segment sits on the far side of a directive from the declaration and documents whatever the directive itself demarcates, never the declaration below it; a directive seated directly above the declaration itself is transparent to this split rather than a boundary of it, since group assembly skips past it before the run is ever collected, so the prose above it is still reported and converted, with the directive preserved untouched in place, still functional, directly above the declaration; a directive found inside an already-consolidated Block comment leaves the whole comment alone, since a partial fix cannot safely be sliced out of one physical comment node. A `///` triple-slash reference/AMD directive (`/// `) is recognised separately, by its own un-stripped comment value rather than this same pattern, and excluded from the fix the same way. An existing `/**`-opening doc comment, or a `/*!` license/banner block, is left alone regardless of length. Autofix rewraps the comment's existing text into a `/** ... */` block, one line per original logical line as this rule's own fixer writes it, preserving each line's own indentation beyond the delimiter's single conventional space (never flattening a nested example or sub-list to its neighbours' level), and unwraps a hand-written starred block's (`/* ... */` with its own leading `* ` per line) marker rather than doubling it against the fixer's own. This describes this rule's own fixer alone, not a guarantee that `exadevConfig()`'s combined output preserves that text verbatim: a bundled sibling rule's own fixer can still rewrite it further in the same `--fix` run, which is exactly why the fix is withheld below for the two such collisions this rule already guards against. Handles the leading comment whether it is a run of `//` lines or already a bare block: a bare block may be hand-written, or produced by the bare-block fixer of `exadev/multiline-comment-style` (bundled alongside this rule) or of a consumer's own `@stylistic/multiline-comment-style`. Always reported when substantial, but the fix withheld, rather than silently skipping the violation, whenever the exact resulting text would contain a literal closing-comment delimiter or fails to parse as valid TSDoc (checked directly against `@microsoft/tsdoc`'s own parser, the same one `eslint-plugin-tsdoc`'s `tsdoc/syntax` validates against). Also withheld when any considered line itself begins with a literal `*` once its own leading whitespace is trimmed (a hand-written markdown bullet, `* item`, or a bare block comment whose own lines are only partially starred): spliced verbatim into this fixer's own template, that reads as a repeated delimiter on a middle line, which eslint-plugin-jsdoc's own `jsdoc/no-multi-asterisks` rule (bundled unconditionally alongside this rule, see [`jsdoc.ts`](src/jsdoc.ts)) would then strip in the very same `--fix` run, silently losing real content in a way neither this rule nor `@microsoft/tsdoc`'s own parser has any way to detect on its own. Also withheld when any considered line carries an unescaped tag-shaped `@word` mention, at the line's own start (a genuine TSDoc block or modifier tag, `@remarks`, `@param foo`, `@internal`, `@typeParam T`, `@virtual`, `@override`, ...) or immediately after whitespace anywhere further into the line (a bare mid-line mention, `tseslint.config() is @deprecated upstream`), the identical shape `jsdoc/escape-inline-tags` itself keys on: a line-start tag withholds because the converted doc comment would be validated by every tag-checking sibling rule in the bundle and read by TypeScript's own JSDoc parser, and this rule cannot enumerate in advance which tag either will next object to or act on. jsdoc.ts gives `jsdoc/check-tag-names` the TSDoc vocabulary (`settings.jsdoc.tagNamePreference` entries for the TSDoc tags JSDoc spells differently, `typed: false`, and `jsdoc/empty-tags` off), so the TSDoc tags that once collided (`@internal`, `@public`, `@readonly`, `@typeParam`, `@virtual`, `@override`) survive `--fix` there, but a converted tag line such as `@deprecated` or `@internal` is still read as a REAL tag on the symbol, changing what it means rather than merely its formatting. Withholding on every tag-shaped line, not merely a blank line adjacent to one, is deliberate, so no further one-off guard is needed each time a new collision is found. Requires no type information. Wired only through `stylisticCommentsConfig` into the default (type-checked) `exadevConfig()` output (see [Stylistic comment, class-member and JSX rules](#stylistic-comment-class-member-and-jsx-rules)), never registered in `plugin.configs.recommended`/`.barrel`, so a consumer using the lighter `extends: [plugin.configs.recommended]` option (see [The lighter option](#the-lighter-option-the-plugin-named-export)) never gets it. | | `multiline-comment-style` | ✓ | **`@stylistic/eslint-plugin`'s [`multiline-comment-style`](https://eslint.style/rules/multiline-comment-style), never merging a directive into a block comment.** Delegates to the upstream rule with the same options, defaults, messages and fixer, but hides every line `prefer-doc-comment` treats as a directive (`TODO`/`FIXME` task markers, `cspell:*`, `biome-ignore`, `#region`/`#endregion` and the rest of that rule's list) from the upstream rule's grouping, so such a line breaks a `//` run the way upstream's own directives do. A managed-region marker (a comment of exactly `::start` or `::end`, such as `content:claude:start`) is treated as a directive too, so no rewrite merges it with other text. The text the fixer writes also has trailing whitespace removed from every line, since upstream leaves padding on blank lines inside a rewritten block. Exists because upstream does not yet treat `TODO`/`FIXME` as directives ([eslint-stylistic/eslint-stylistic#1287](https://github.com/eslint-stylistic/eslint-stylistic/issues/1287)); see [Stylistic comment, class-member and JSX rules](#stylistic-comment-class-member-and-jsx-rules). Requires no type information. Wired at `'bare-block'` only through `stylisticCommentsConfig` into the default `exadevConfig()` output, never in `plugin.configs.recommended`/`.barrel`. | | `package-json-key-order` | ✓ | **Requires `package.json`'s keys to match `syncpack format`'s order.** See [Optional package.json key ordering](#optional-packagejson-key-ordering) — opt-in via `exadevConfig({ packageJsonKeyOrder: true })`, not part of `recommended`/`barrel`. A JSON-language rule (`@eslint/json`'s `json/json`), not a TSESLint one — needs no type information and doesn't apply to any `.ts`/`.js` file. | diff --git a/src/rules/prefer-options-object-param.ts b/src/rules/prefer-options-object-param.ts index 2fbc8fd1..757b8201 100644 --- a/src/rules/prefer-options-object-param.ts +++ b/src/rules/prefer-options-object-param.ts @@ -5,6 +5,8 @@ import { firstTokenOrThrow } from './ts-node-guards'; `hasSuggestions: true`, not `fixable: 'code'` — this is the key call. `fixable` is applied silently by `--fix` with no review gate. no-pointless-reassignment.ts's own history (a far simpler identifier-swap fixer) already shipped real bugs this way (code that didn't parse, a deleted load-bearing type annotation), and this rule's own fixer is riskier still: it rewrites a parameter list AND inserts a new statement into the function body. prefer-numeric-sort-compare.ts already uses `hasSuggestions` in this exact package for the identical reason — a suggestion the developer explicitly reviews and accepts is appropriate; silently rewriting behaviour on every save is not. Call sites are deliberately never rewritten: the signature edit alone turns every stale positional call site into a real TypeScript compile error, which is the correct, sufficient signal for a human/agent to fix each one — ESLint's own fixer can only ever edit the single file it is linting, so it could never safely coordinate an edit to the declaration with edits to call sites scattered across other files in the same pass anyway. + A rest parameter changes the arithmetic of that argument: a caller who wants only the rest values must pass a placeholder for a single optional parameter that precedes it, even though a run of length one never meets `minTrailingOptional` (whose schema minimum stays 2). That shape is reported under its own message id, `placeholderBeforeRest`, with no suggestion, since the right shape (the rest values alone, or one object holding the optional value and the rest values) is a design decision and not a mechanical rewrite. A run that does reach `minTrailingOptional` keeps the existing report, whether or not a rest parameter follows it. + Every param-list/type-text extraction below uses a verbatim `sourceCode.getText()` slice of the parameter's own type-annotation node, never a checker-based reconstruction (a printer, not a source-text echo, that can silently diverge from what was actually written) — this is why the rule needs no type-checker access at all: every check it performs (is this parameter optional, does it already carry an explicit type annotation, is it a parameter property, is it decorated) is answerable from the parameter's own TSESTree shape, and the fixer only ever echoes source text it already has. Because of that, it is registered in BOTH `plugin.configs.recommended` (src/plugin.ts) and the type-checked bundle (src/recommended-type-checked.ts), matching `no-mutable-union-array-param`/`prefer-readonly-array-param`/`test-file-kind`/`max-params` — unlike its own `prefer-*` siblings `prefer-readonly-object-param` and `prefer-numeric-sort-compare`, which genuinely need the checker (to resolve a parameter's own property types, and to confirm an array's element type, respectively) and so cannot be offered in the lighter, non-type-checked bundle at all. Every bail-out below still reports the diagnostic (a caller still deserves to be told about the anti-pattern) but withholds the suggestion (`suggest` omitted from the report) whenever collapsing the run mechanically would be unsafe or lossy: @@ -190,7 +192,7 @@ export function hasOptionsNameCollision(scope: TSESLint.Scope.Scope, exemptIdent // Named explicitly and passed as createRule's own generic arguments rather than left for TS to infer from the `defaultOptions` object literal below — an array literal infers as the wider `{ minTrailingOptional: number }[]` in that position, not the 1-element tuple RuleCreator's own `Options extends readonly unknown[]` constraint expects, which otherwise surfaces as a spurious `noUncheckedIndexedAccess` "possibly undefined" on `create`'s own destructured second parameter. type Options = readonly [{ readonly minTrailingOptional: number }]; -type MessageIds = 'tooManyTrailingOptional' | 'wrapInOptionsObject'; +type MessageIds = 'tooManyTrailingOptional' | 'wrapInOptionsObject' | 'placeholderBeforeRest'; const preferOptionsObjectParam = createRule({ name: 'prefer-options-object-param', @@ -199,7 +201,7 @@ const preferOptionsObjectParam = createRule({ hasSuggestions: true, docs: { description: - "Suggest bundling a run of 2+ trailing optional parameters into a single destructured 'options' parameter — without this, a caller needing only the last optional parameter must still pass 'undefined' for every optional parameter before it.", + "Suggest bundling a run of 2+ trailing optional parameters into a single destructured 'options' parameter — without this, a caller needing only the last optional parameter must still pass 'undefined' for every optional parameter before it. Also report an optional parameter directly before a rest parameter, which a caller wanting only the rest values must fill with a placeholder.", }, schema: [ { @@ -214,15 +216,29 @@ const preferOptionsObjectParam = createRule({ tooManyTrailingOptional: "This {{ kind }} has {{ count }} trailing optional parameters ({{ names }}) — a caller needing only the last one must still pass 'undefined' for every parameter before it. Bundle the trailing optional run into a single destructured 'options' parameter instead.", wrapInOptionsObject: "Bundle the trailing optional parameters into a single 'options' parameter.", + placeholderBeforeRest: + 'This {{ kind }} takes an optional parameter ({{ name }}) immediately before a rest parameter, so a caller passing only the rest values must supply a placeholder for it. Take the rest values alone, or put the optional value and the rest values in one object.', }, defaultOptions: [{ minTrailingOptional: 2 }], }, create(context, [{ minTrailingOptional }]) { const { sourceCode } = context; + // A run below `minTrailingOptional` is ordinarily fine, but an optional parameter directly before a rest parameter forces a caller who wants only the rest values to pass a placeholder, whatever the run's length. No suggestion is offered: the right shape (rest values alone, or one object holding the optional value and the rest values) is a design decision, not a mechanical rewrite. + function reportPlaceholderBeforeRest(node: FunctionLikeWithParams, run: readonly TSESTree.Parameter[]) { + const optionalBeforeRest = run.at(-1); + if (optionalBeforeRest === undefined || node.params.at(-1)?.type !== AST_NODE_TYPES.RestElement) return; + const name = resolveFixableParam(optionalBeforeRest)?.identifierNode.name ?? sourceCode.getText(optionalBeforeRest); + context.report({ node, messageId: 'placeholderBeforeRest', data: { kind: describeFunctionKind(node), name } }); + } + function checkParams(node: FunctionLikeWithParams) { const run = getTrailingOptionalRun(node.params); - if (run.length < minTrailingOptional) return; + if (run.length < minTrailingOptional) { + reportPlaceholderBeforeRest(node, run); + + return; + } const resolved = run.map(resolveFixableParam); const names = run.map((param, index) => resolved[index]?.identifierNode.name ?? sourceCode.getText(param)).join(', '); diff --git a/src/rules/prefer-options-object-param.unit.test.ts b/src/rules/prefer-options-object-param.unit.test.ts index 8eefdd1f..866f1d52 100644 --- a/src/rules/prefer-options-object-param.unit.test.ts +++ b/src/rules/prefer-options-object-param.unit.test.ts @@ -23,11 +23,14 @@ describe('rule metadata', () => { expect(rule.name).toBe('prefer-options-object-param'); expect(rule.meta.docs?.url).toBe('https://github.com/ExaDev/eslint-config/blob/main/src/rules/prefer-options-object-param.ts'); expect(rule.meta.docs?.description).toBe( - "Suggest bundling a run of 2+ trailing optional parameters into a single destructured 'options' parameter — without this, a caller needing only the last optional parameter must still pass 'undefined' for every optional parameter before it.", + "Suggest bundling a run of 2+ trailing optional parameters into a single destructured 'options' parameter — without this, a caller needing only the last optional parameter must still pass 'undefined' for every optional parameter before it. Also report an optional parameter directly before a rest parameter, which a caller wanting only the rest values must fill with a placeholder.", ); expect(rule.meta.messages.tooManyTrailingOptional).toBe( "This {{ kind }} has {{ count }} trailing optional parameters ({{ names }}) — a caller needing only the last one must still pass 'undefined' for every parameter before it. Bundle the trailing optional run into a single destructured 'options' parameter instead.", ); + expect(rule.meta.messages.placeholderBeforeRest).toBe( + 'This {{ kind }} takes an optional parameter ({{ name }}) immediately before a rest parameter, so a caller passing only the rest values must supply a placeholder for it. Take the rest values alone, or put the optional value and the rest values in one object.', + ); expect(rule.meta.messages.wrapInOptionsObject).toBe("Bundle the trailing optional parameters into a single 'options' parameter."); expect(rule.meta.schema).toEqual([ { type: 'object', properties: { minTrailingOptional: { type: 'integer', minimum: 2 } }, additionalProperties: false }, @@ -51,8 +54,15 @@ ruleTester.run('prefer-options-object-param', rule, { 'function f(): void {}', // A bare rest parameter is never itself optional, and there is nothing before it. 'function f(...rest: number[]): void {}', - // Only 1 parameter precedes a trailing rest parameter — still below the default threshold even though the rest parameter's mere presence does not by itself disqualify the run. - 'function f(a: number, b?: number, ...rest: number[]): void {}', + // A required parameter before a rest parameter needs no placeholder: the caller must supply it anyway. + 'function f(a: number, b: number, ...rest: number[]): void {}', + // A rest parameter after required parameters only. + 'class C {\n constructor(a: number, ...rest: number[]) {}\n}', + // A raised threshold still leaves a rest parameter after required parameters alone valid. + { + code: 'function f(a: number, ...rest: number[]): void {}', + options: [{ minTrailingOptional: 3 }], + }, // A custom, higher threshold (3) is honoured — 2 trailing optional parameters is not enough to trip it. { code: 'function f(a: number, b?: number, c?: number): void {}', @@ -60,6 +70,73 @@ ruleTester.run('prefer-options-object-param', rule, { }, ], invalid: [ + // An optional parameter directly before a rest parameter: a caller wanting only the rest values must pass a placeholder. Reported with its own message and no suggestion. + { + code: 'function f(a: number, b?: number, ...rest: number[]): void {}', + errors: [{ messageId: 'placeholderBeforeRest', data: { kind: 'function', name: 'b' }, suggestions: [] }], + }, + // A default-valued optional parameter before a rest parameter. + { + code: 'function f(b: number = 1, ...rest: number[]): void {}', + errors: [{ messageId: 'placeholderBeforeRest', data: { kind: 'function', name: 'b' }, suggestions: [] }], + }, + // A destructured optional options object before a rest parameter: named by its source text, since it has no single bindable name. + { + code: 'function f({ a }: { a?: number } = {}, ...rest: number[]): void {}', + errors: [{ messageId: 'placeholderBeforeRest', data: { kind: 'function', name: '{ a }: { a?: number } = {}' }, suggestions: [] }], + }, + // An arrow function. + { + code: 'const f = (a?: number, ...rest: number[]): void => {};', + errors: [{ messageId: 'placeholderBeforeRest', data: { kind: 'function', name: 'a' }, suggestions: [] }], + }, + // A class method. + { + code: 'class C {\n m(a?: number, ...rest: number[]): void {}\n}', + errors: [{ messageId: 'placeholderBeforeRest', data: { kind: 'method', name: 'a' }, suggestions: [] }], + }, + // A constructor. + { + code: 'class C {\n constructor(a?: number, ...rest: number[]) {}\n}', + errors: [{ messageId: 'placeholderBeforeRest', data: { kind: 'constructor', name: 'a' }, suggestions: [] }], + }, + // An overload set: each declaration-only signature and the implementation are reported on their own. + { + code: 'function f(a?: number, ...rest: number[]): void;\nfunction f(a?: number, ...rest: number[]): void {}', + errors: [ + { messageId: 'placeholderBeforeRest', data: { kind: 'function', name: 'a' }, suggestions: [] }, + { messageId: 'placeholderBeforeRest', data: { kind: 'function', name: 'a' }, suggestions: [] }, + ], + }, + // A declaration-only ambient function. + { + code: 'declare function f(a?: number, ...rest: number[]): void;', + errors: [{ messageId: 'placeholderBeforeRest', data: { kind: 'function', name: 'a' }, suggestions: [] }], + }, + // A standalone function type. + { + code: 'type F = (a?: number, ...rest: number[]) => void;', + errors: [{ messageId: 'placeholderBeforeRest', data: { kind: 'function', name: 'a' }, suggestions: [] }], + }, + // An interface method signature and a construct signature. + { + code: 'interface I {\n m(a?: number, ...rest: number[]): void;\n new (a?: number, ...rest: number[]): I;\n}', + errors: [ + { messageId: 'placeholderBeforeRest', data: { kind: 'method', name: 'a' }, suggestions: [] }, + { messageId: 'placeholderBeforeRest', data: { kind: 'constructor', name: 'a' }, suggestions: [] }, + ], + }, + // A parameter property before a rest parameter. + { + code: 'class C {\n constructor(private a?: number, ...rest: number[]) {}\n}', + errors: [{ messageId: 'placeholderBeforeRest', data: { kind: 'constructor', name: 'private a?: number' }, suggestions: [] }], + }, + // With a raised threshold, a run shorter than it but followed by a rest parameter is reported by the placeholder message, naming the last optional parameter. + { + code: 'function f(a: number, b?: number, c?: number, ...rest: number[]): void {}', + options: [{ minTrailingOptional: 3 }], + errors: [{ messageId: 'placeholderBeforeRest', data: { kind: 'function', name: 'c' }, suggestions: [] }], + }, // The trailing optional run reaches all the way back to the FIRST parameter, index 0, with no leading required parameter at all: proves getTrailingOptionalRun's own backward walk actually reaches and includes index 0, rather than stopping one short of it. { code: 'function f(a?: number, b?: number): void {\n return;\n}',