Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions README.md

Large diffs are not rendered by default.

4 changes: 1 addition & 3 deletions src/create-config.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand Down
20 changes: 6 additions & 14 deletions src/create-config.unit.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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: {} });
});
});
8 changes: 2 additions & 6 deletions src/import-policy.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof exadevConfig>) => 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);
});
});
8 changes: 2 additions & 6 deletions src/pure-modules.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof exadevConfig>) => 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);
});
});
22 changes: 19 additions & 3 deletions src/rules/prefer-options-object-param.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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<Options, MessageIds>({
name: 'prefer-options-object-param',
Expand All @@ -199,7 +201,7 @@ const preferOptionsObjectParam = createRule<Options, MessageIds>({
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: [
{
Expand All @@ -214,15 +216,29 @@ const preferOptionsObjectParam = createRule<Options, MessageIds>({
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(', ');
Expand Down
Loading