From 762b9d44795ebfe25accc040d6d7e6df4fdeb912 Mon Sep 17 00:00:00 2001 From: Shironex Date: Wed, 30 Sep 2026 12:09:57 +0200 Subject: [PATCH 01/16] feat(code-quality): add no-message-only-throw-assertion Reports toThrow() / toThrowError() with no argument, or with only a string, template or regex, sync or after .rejects: any error passes, so a TypeError from a broken mock satisfies the assertion. A message-only assertion is accepted when the same test pins the class of the same subject. Options: throwMatchers, allowMessageOnly, trustErrorInstances (false under Jest) and assertionHelpers. Opt-in, left out of recommended. --- packages/eslint-plugin-code-quality/README.md | 15 +- .../rules/no-message-only-throw-assertion.md | 145 +++++++++ .../src/configs/recommended.ts | 6 + .../src/rules/index.ts | 3 + .../rules/no-message-only-throw-assertion.ts | 308 ++++++++++++++++++ .../tests/configs/recommended.test.ts | 1 + .../no-message-only-throw-assertion.test.ts | 146 +++++++++ site/scripts/parity.test.ts | 2 +- 8 files changed, 624 insertions(+), 2 deletions(-) create mode 100644 packages/eslint-plugin-code-quality/docs/rules/no-message-only-throw-assertion.md create mode 100644 packages/eslint-plugin-code-quality/src/rules/no-message-only-throw-assertion.ts create mode 100644 packages/eslint-plugin-code-quality/tests/rules/no-message-only-throw-assertion.test.ts diff --git a/packages/eslint-plugin-code-quality/README.md b/packages/eslint-plugin-code-quality/README.md index 23480f0..37fd4c4 100644 --- a/packages/eslint-plugin-code-quality/README.md +++ b/packages/eslint-plugin-code-quality/README.md @@ -46,7 +46,10 @@ export default [ ## Opt-in rules -Two rules are exported but left out of `recommended`: they are house style, not correctness. +Some rules are exported but left out of `recommended`. `interface-prefix-i` and +`no-template-trim-empty-ternary` are house style, not correctness. The test-discipline rules in the +second block need a per-project fact before they are precise, such as which files are unit tests, +so each one takes its options from your project. ```js // eslint.config.js @@ -60,6 +63,15 @@ export default [ 'noctcore-code-quality/no-template-trim-empty-ternary': 'error', }, }, + { + files: ['**/*.{test,spec}.{ts,tsx}'], + languageOptions: { parser: tsParser }, + rules: { + 'noctcore-code-quality/no-message-only-throw-assertion': ['error', { + assertionHelpers: ['^expectRejectsDomainError$'], + }], + }, + }, ]; ``` @@ -79,6 +91,7 @@ export default [ | [`no-elided-code-comments`](https://noctcore.github.io/eslint-plugins/rules/code-quality/no-elided-code-comments/) | Disallow comments that stand in for elided code ('// ... existing code ...', '// rest of the function unchanged', '// your code here'). They are what an agent leaves when it rewrites a file from an abbreviated draft, and the code they replaced has usually been deleted. | โœ… | | | | | | [`no-focused-tests`](https://noctcore.github.io/eslint-plugins/rules/code-quality/no-focused-tests/) | Ban focused tests (it.only / describe.only / test.only, fdescribe / fit / ddescribe) so a focused test never silently lands in CI. | โœ… | | | | | | [`no-historical-comments`](https://noctcore.github.io/eslint-plugins/rules/code-quality/no-historical-comments/) | Disallow comments that frame code relative to what it used to do or to a past incident ('before the fix', 'after the refactor', 'we used to', 'no longer'). Source comments describe the current invariant; history belongs in the commit message or PR description, where it does not rot when the code changes again. | โœ… | | | | | +| [`no-message-only-throw-assertion`](https://noctcore.github.io/eslint-plugins/rules/code-quality/no-message-only-throw-assertion/) | Disallow `toThrow()` with no argument or with only a message: any error passes, including a `TypeError` from a broken mock. Pin the error class. | ๐Ÿ”˜ | | | | | | [`no-narration-comments`](https://noctcore.github.io/eslint-plugins/rules/code-quality/no-narration-comments/) | Disallow narrative comments like 'Here we...', 'Now we...', 'First, we...'. These read as step-by-step prose and add no information a future reader cannot get from the code itself. Often a tell that the comment was generated by an agent describing its own changes. | โœ… | | | | | | [`no-pr-reference-comments`](https://noctcore.github.io/eslint-plugins/rules/code-quality/no-pr-reference-comments/) | Disallow PR/issue references in comments. They belong in commit messages and PR descriptions, where they do not rot when the repo moves, the issue tracker migrates, or the numbering changes. | โœ… | | | | | | [`no-process-exit`](https://noctcore.github.io/eslint-plugins/rules/code-quality/no-process-exit/) | Disallow `process.exit()` outside bootstrap/shutdown paths and standalone CLIs. Application and service code must throw or reject so the lifecycle can shut down gracefully. | โœ… | | | | | diff --git a/packages/eslint-plugin-code-quality/docs/rules/no-message-only-throw-assertion.md b/packages/eslint-plugin-code-quality/docs/rules/no-message-only-throw-assertion.md new file mode 100644 index 0000000..daef80d --- /dev/null +++ b/packages/eslint-plugin-code-quality/docs/rules/no-message-only-throw-assertion.md @@ -0,0 +1,145 @@ +# `noctcore-code-quality/no-message-only-throw-assertion` + +> A throw assertion must pin the error class, not accept any error or only its wording. + + +๐Ÿ”˜ Opt-in: not in `recommended` ยท ๐Ÿ’ญ Type information: not needed + + +## Why + +`expect(fn).toThrow()` passes for any error. When the code under test breaks in a way that throws +something else, most often a `TypeError` from a mock that returns `undefined` where an object was +expected, the assertion still passes and the test stays green on a broken path. + +A message alone is not much better. `toThrow('Employee not found.')` passes for any error class +carrying that sentence. When two refusals share their wording but differ in class (a `NotFoundError` +that says nothing about whether the record exists, and a `ForbiddenError` that confirms it does), the +difference between them is exactly what the test exists to defend, and a message check cannot see it. + +```ts bad filename=src/employees/employee.service.test.ts reports=2 +it('refuses an unknown employee', async () => { + await expect(service.find('e-404')).rejects.toThrow(); +}); + +it('refuses another tenant', async () => { + await expect(service.find('e-other')).rejects.toThrow('Employee not found.'); +}); +``` + +```ts good filename=src/employees/employee.service.test.ts +it('refuses an unknown employee', async () => { + await expect(service.find('e-404')).rejects.toThrow(NotFoundError); +}); + +it('refuses another tenant', async () => { + await expect(service.find('e-other')).rejects.toThrow(NotFoundError); + await expect(service.find('e-other')).rejects.toThrow('Employee not found.'); +}); +``` + +## What it flags + +A `toThrow` / `toThrowError` matcher on an `expect(...)` chain, sync or after `.rejects`, that is not +negated and whose argument: + +- `bareThrow`: is missing: `toThrow()`, `.rejects.toThrowError()`; +- `messageOnlyThrow`: checks only the message: a string, a template literal, a regex literal or a + `RegExp` built with `new RegExp(...)`. + +It reports inside and outside test callbacks, so a shared helper that asserts `.rejects.toThrow()` +is reported where it is written. + +```ts bad filename=src/lib/api-path.test.ts reports=3 +it('rejects an absolute url', () => { + expect(() => toApiPath('https://evil.test')).toThrow('Refusing a non-relative api path'); +}); + +it('rejects an unknown queue', () => { + expect(() => metrics.onModuleInit()).toThrow(new RegExp(`Queue "${missing}" is declared`)); +}); + +export async function expectRefusal(promise: Promise) { + await expect(promise).rejects.toThrow(); +} +``` + +```ts good filename=src/lib/api-path.test.ts +it('rejects an absolute url', () => { + expect(() => toApiPath('https://evil.test')).toThrow(UnsafePathError); +}); + +it('rejects an unknown queue', () => { + expect(() => metrics.onModuleInit()).toThrow( + expect.objectContaining({ name: 'QueueConfigError', queue: missing }), + ); +}); + +export async function expectRefusal(promise: Promise) { + await expect(promise).rejects.toBeInstanceOf(DomainError); +} +``` + +### Class and message asserted separately + +A message-only assertion is accepted when the same test also pins the class of the same subject (the +same source text inside `expect(...)`) with a class-pinning throw matcher, a `.rejects` matcher that +checks the value (`toBeInstanceOf`, `toMatchObject`, `toEqual`, `toStrictEqual`, `toHaveProperty`), +or a configured `assertionHelpers` call whose first argument is that subject. Some refusals differ +only in wording, and there the sentence is the assertion; the class next to it keeps it honest. + +## What it does not flag + +- A class argument (`toThrow(NotFoundError)`, `toThrow(errors.Forbidden)`), an asymmetric matcher + (`toThrow(expect.objectContaining({ code: 'E_LOCKED' }))`) or any other value that is not a string, + template or regex. +- An error instance (`toThrow(new ForbiddenError('No access'))`) while `trustErrorInstances` is on: + Vitest compares it like `toEqual`, so its class name takes part. See Options for Jest. +- `.not.toThrow()` and `.resolves.not.toThrow()`: a negated throw assertion has no class to pin. +- `.rejects.toMatchObject(...)` and the other value matchers on their own. +- A message held in a variable (`toThrow(expectedMessage)`): the rule reads syntax, not values, and + treats it as it would an error class. + +## Options + +| Option | Type | Default | Meaning | +| --- | --- | --- | --- | +| `throwMatchers` | `string[]` | `["toThrow", "toThrowError"]` | Matchers that assert a throw or a rejection. | +| `allowMessageOnly` | `boolean` | `false` | Report only the argless form. For adopting the rule in two steps. | +| `trustErrorInstances` | `boolean` | `true` | Count an error instance argument as pinning the class. Set `false` under Jest, whose `toThrow(new X('m'))` compares only the message. | +| `assertionHelpers` | `string[]` (regex sources) | `[]` | Helpers that pin the error class, such as `expectRejectsDomainError(promise, {...})`. A call on the same subject in the same test accepts a message-only assertion. Matched against `name` or `obj.name`. | + +```js +'noctcore-code-quality/no-message-only-throw-assertion': ['error', { + // Jest compares an error instance by its message only. + trustErrorInstances: false, + assertionHelpers: ['^expect(Throws|Rejects)DomainError$'], +}] +``` + +Under `trustErrorInstances: false` an instance is a message check: + +```ts bad filename=src/auth/guard.spec.ts options={"trustErrorInstances":false} +it('refuses a guest', () => { + expect(() => guard.check(guest)).toThrow(new ForbiddenError('No access')); +}); +``` + +```ts good filename=src/auth/guard.spec.ts options={"trustErrorInstances":false} +it('refuses a guest', () => { + expect(() => guard.check(guest)).toThrow(ForbiddenError); + expect(() => guard.check(guest)).toThrow('No access'); +}); +``` + +## When not to use it + +In a codebase that throws one plain `Error` class everywhere, where the message is the only thing +that tells refusals apart. Start with `allowMessageOnly: true` there: the argless form is still a +defect. + +## Related + +- [`no-swallowed-assertion`](./no-swallowed-assertion.md): an assertion whose failure is caught and + dropped. +- [`no-vacuous-expect`](./no-vacuous-expect.md): assertions that pass for almost any implementation. diff --git a/packages/eslint-plugin-code-quality/src/configs/recommended.ts b/packages/eslint-plugin-code-quality/src/configs/recommended.ts index ee7dd25..f7ede43 100644 --- a/packages/eslint-plugin-code-quality/src/configs/recommended.ts +++ b/packages/eslint-plugin-code-quality/src/configs/recommended.ts @@ -6,6 +6,12 @@ * convention) and `no-template-trim-empty-ternary` (a very specific inline * shape) โ€” are intentionally omitted. They are exported and documented, so a * consumer can enable them explicitly, but they are not on by default. + * + * The test-discipline rules below are omitted because each needs a + * per-project fact before it is precise at `error`: + * - `no-message-only-throw-assertion`: whether an error instance pins its class + * depends on the runner (Vitest compares it, Jest only its message), and a + * project's own class-pinning assertion helpers are configured by name. */ export const recommended = { 'noctcore-code-quality/prefer-early-return': 'error', diff --git a/packages/eslint-plugin-code-quality/src/rules/index.ts b/packages/eslint-plugin-code-quality/src/rules/index.ts index 7998954..363c914 100644 --- a/packages/eslint-plugin-code-quality/src/rules/index.ts +++ b/packages/eslint-plugin-code-quality/src/rules/index.ts @@ -5,6 +5,7 @@ import { noConditionalExpectRule } from './no-conditional-expect'; import { noElidedCodeCommentsRule } from './no-elided-code-comments'; import { noFocusedTestsRule } from './no-focused-tests'; import { noHistoricalCommentsRule } from './no-historical-comments'; +import { noMessageOnlyThrowAssertionRule } from './no-message-only-throw-assertion'; import { noNarrationCommentsRule } from './no-narration-comments'; import { noPrReferenceCommentsRule } from './no-pr-reference-comments'; import { noProcessExitRule } from './no-process-exit'; @@ -34,4 +35,6 @@ export const rules = { // Available but omitted from `recommended` (opinionated / niche). 'interface-prefix-i': interfacePrefixIRule, 'no-template-trim-empty-ternary': noTemplateTrimEmptyTernaryRule, + // Available but omitted from `recommended` (they need per-project facts). + 'no-message-only-throw-assertion': noMessageOnlyThrowAssertionRule, }; diff --git a/packages/eslint-plugin-code-quality/src/rules/no-message-only-throw-assertion.ts b/packages/eslint-plugin-code-quality/src/rules/no-message-only-throw-assertion.ts new file mode 100644 index 0000000..41149b6 --- /dev/null +++ b/packages/eslint-plugin-code-quality/src/rules/no-message-only-throw-assertion.ts @@ -0,0 +1,308 @@ +import { AST_NODE_TYPES, type TSESTree } from '@typescript-eslint/utils'; +import type { JSONSchema4 } from '@typescript-eslint/utils/json-schema'; + +import { createRule } from '../createRule'; + +const RULE_NAME = 'no-message-only-throw-assertion'; + +export interface NoMessageOnlyThrowAssertionOptions { + /** Matchers that assert a throw or a rejection (`toThrow`, `toThrowError`). */ + readonly throwMatchers?: readonly string[]; + /** + * Report only the argless form (`toThrow()`), and accept a string or regex + * argument. For adopting the rule in two steps. + */ + readonly allowMessageOnly?: boolean; + /** + * Treat an error instance argument (`toThrow(new NotFoundError('x'))`) as + * pinning the class. True for Vitest, which compares the instance like + * `toEqual`; set false under Jest, which compares only the message. + */ + readonly trustErrorInstances?: boolean; + /** + * Regex sources (compiled with the `u` flag) for assertion helpers that pin + * the error class, such as `expectRejectsDomainError(promise, {...})`. A + * message-only assertion is accepted when a matching helper is called on the + * same subject in the same test. Matched against `name` for a bare call and + * `obj.name` for a member call on an identifier. + */ + readonly assertionHelpers?: readonly string[]; +} + +type RuleOptions = [NoMessageOnlyThrowAssertionOptions]; +type MessageIds = 'bareThrow' | 'messageOnlyThrow'; + +const DEFAULT_THROW_MATCHERS: readonly string[] = ['toThrow', 'toThrowError']; +const DEFAULT_ALLOW_MESSAGE_ONLY = false; +const DEFAULT_TRUST_ERROR_INSTANCES = true; +const DEFAULT_ASSERTION_HELPERS: readonly string[] = []; + +/** Matchers on a `.rejects` chain that pin the rejection beyond its message. */ +const REJECTS_PINNING_MATCHERS = new Set([ + 'toBeInstanceOf', + 'toMatchObject', + 'toEqual', + 'toStrictEqual', + 'toHaveProperty', +]); + +const TEST_RUNNERS = new Set(['it', 'test']); + +const optionSchema: JSONSchema4 = { + type: 'object', + additionalProperties: false, + properties: { + throwMatchers: { + type: 'array', + items: { type: 'string', minLength: 1 }, + uniqueItems: true, + minItems: 1, + }, + allowMessageOnly: { type: 'boolean' }, + trustErrorInstances: { type: 'boolean' }, + assertionHelpers: { + type: 'array', + items: { type: 'string', minLength: 1 }, + uniqueItems: true, + }, + }, +}; + +type FunctionNode = TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression; + +interface MatcherChain { + /** The `expect(...)` call at the root of the chain. */ + readonly root: TSESTree.CallExpression; + readonly matcher: string; + readonly negated: boolean; + readonly rejects: boolean; +} + +interface Finding { + readonly node: TSESTree.CallExpression; + readonly messageId: MessageIds; + readonly matcher: string; + readonly subject: string; +} + +interface TestFrame { + readonly node: FunctionNode | null; + readonly findings: Finding[]; + /** Source text of every subject whose error class is pinned in this test. */ + readonly pinned: Set; +} + +/** Decompose `expect(x).rejects.not.toThrow(y)` into its parts, or null. */ +function matcherChain(node: TSESTree.CallExpression): MatcherChain | null { + const callee = node.callee; + if ( + callee.type !== AST_NODE_TYPES.MemberExpression || + callee.computed || + callee.property.type !== AST_NODE_TYPES.Identifier + ) { + return null; + } + let negated = false; + let rejects = false; + let current: TSESTree.Expression = callee.object; + while (current.type === AST_NODE_TYPES.MemberExpression) { + if (!current.computed && current.property.type === AST_NODE_TYPES.Identifier) { + if (current.property.name === 'not') { + negated = !negated; + } else if (current.property.name === 'rejects') { + rejects = true; + } + } + current = current.object; + } + if ( + current.type !== AST_NODE_TYPES.CallExpression || + current.callee.type !== AST_NODE_TYPES.Identifier || + current.callee.name !== 'expect' + ) { + return null; + } + return { root: current, matcher: callee.property.name, negated, rejects }; +} + +/** `name` or `obj.name` for a callee, used to match `assertionHelpers`. */ +function calleePath(callee: TSESTree.Expression): string | null { + if (callee.type === AST_NODE_TYPES.Identifier) { + return callee.name; + } + if ( + callee.type === AST_NODE_TYPES.MemberExpression && + !callee.computed && + callee.property.type === AST_NODE_TYPES.Identifier + ) { + const owner = callee.object.type === AST_NODE_TYPES.Identifier ? callee.object.name : ''; + return `${owner}.${callee.property.name}`; + } + return null; +} + +/** Root identifier of a test callee: `it`, `test.concurrent`, `it.each(table)`. */ +function runnerName(callee: TSESTree.Node): string | null { + let current: TSESTree.Node = callee; + for (;;) { + if (current.type === AST_NODE_TYPES.MemberExpression) { + current = current.object; + } else if (current.type === AST_NODE_TYPES.CallExpression) { + current = current.callee; + } else if (current.type === AST_NODE_TYPES.TaggedTemplateExpression) { + current = current.tag; + } else { + break; + } + } + return current.type === AST_NODE_TYPES.Identifier ? current.name : null; +} + +function isTestCallback(node: FunctionNode): boolean { + const parent = node.parent; + if (parent.type !== AST_NODE_TYPES.CallExpression || !parent.arguments.includes(node)) { + return false; + } + const name = runnerName(parent.callee); + return name !== null && TEST_RUNNERS.has(name); +} + +/** True for `new RegExp(...)` / `RegExp(...)`: a pattern, so a message check. */ +function isRegExpConstruction(node: TSESTree.Node): boolean { + return ( + (node.type === AST_NODE_TYPES.NewExpression || node.type === AST_NODE_TYPES.CallExpression) && + node.callee.type === AST_NODE_TYPES.Identifier && + node.callee.name === 'RegExp' + ); +} + +/** True for an argument that checks only the message: a string, a template or a regex. */ +function isMessageArgument(node: TSESTree.Node): boolean { + if (node.type === AST_NODE_TYPES.Literal) { + return typeof node.value === 'string' || 'regex' in node; + } + return node.type === AST_NODE_TYPES.TemplateLiteral || isRegExpConstruction(node); +} + +export const noMessageOnlyThrowAssertionRule = createRule({ + name: RULE_NAME, + meta: { + type: 'problem', + docs: { + description: + 'Disallow `toThrow()` with no argument or with only a message: any error passes, including a `TypeError` from a broken mock. Pin the error class.', + }, + schema: [optionSchema], + messages: { + bareThrow: + '`{{matcher}}()` with no argument passes for any error, including a `TypeError` from a broken mock. Pass the error class, or assert the error with `.rejects.toMatchObject(...)`.', + messageOnlyThrow: + '`{{matcher}}(...)` with only a message passes for any error class with that wording. Pass the error class, or pin it with a second assertion on the same subject.', + }, + }, + defaultOptions: [ + { + throwMatchers: [...DEFAULT_THROW_MATCHERS], + allowMessageOnly: DEFAULT_ALLOW_MESSAGE_ONLY, + trustErrorInstances: DEFAULT_TRUST_ERROR_INSTANCES, + assertionHelpers: [...DEFAULT_ASSERTION_HELPERS], + }, + ], + create(context, [options]) { + const throwMatchers = new Set(options.throwMatchers ?? DEFAULT_THROW_MATCHERS); + const allowMessageOnly = options.allowMessageOnly ?? DEFAULT_ALLOW_MESSAGE_ONLY; + const trustErrorInstances = options.trustErrorInstances ?? DEFAULT_TRUST_ERROR_INSTANCES; + const assertionHelpers = (options.assertionHelpers ?? DEFAULT_ASSERTION_HELPERS).map( + (source) => new RegExp(source, 'u'), + ); + const stack: TestFrame[] = [{ node: null, findings: [], pinned: new Set() }]; + + function textOf(node: TSESTree.Node | undefined): string | null { + return node === undefined ? null : context.sourceCode.getText(node); + } + + /** Report what the frame holds, minus subjects the same test pins elsewhere. */ + function flush(frame: TestFrame): void { + for (const finding of frame.findings) { + if (frame.pinned.has(finding.subject)) { + continue; + } + context.report({ + node: finding.node, + messageId: finding.messageId, + data: { matcher: finding.matcher }, + }); + } + } + + function enter(node: FunctionNode): void { + if (isTestCallback(node)) { + stack.push({ node, findings: [], pinned: new Set() }); + } + } + + function exit(node: FunctionNode): void { + const frame = stack.at(-1); + if (frame?.node === node) { + stack.pop(); + flush(frame); + } + } + + /** Classify a throw matcher's argument: null when it pins the class. */ + function classify(argument: TSESTree.Node | undefined): MessageIds | null { + if (argument === undefined) { + return 'bareThrow'; + } + if (isMessageArgument(argument)) { + return allowMessageOnly ? null : 'messageOnlyThrow'; + } + if (argument.type === AST_NODE_TYPES.NewExpression && !trustErrorInstances) { + return allowMessageOnly ? null : 'messageOnlyThrow'; + } + return null; + } + + return { + ArrowFunctionExpression: enter, + FunctionExpression: enter, + 'ArrowFunctionExpression:exit': exit, + 'FunctionExpression:exit': exit, + CallExpression(node: TSESTree.CallExpression): void { + const frame = stack.at(-1); + if (frame === undefined) { + return; + } + const chain = matcherChain(node); + if (chain === null) { + const path = calleePath(node.callee); + const subject = textOf(node.arguments[0]); + if (path !== null && subject !== null && assertionHelpers.some((re) => re.test(path))) { + frame.pinned.add(subject); + } + return; + } + const subject = textOf(chain.root.arguments[0]); + if (chain.negated || subject === null) { + return; + } + if (throwMatchers.has(chain.matcher)) { + const messageId = classify(node.arguments[0]); + if (messageId === null) { + frame.pinned.add(subject); + } else { + frame.findings.push({ node, messageId, matcher: chain.matcher, subject }); + } + } else if (chain.rejects && REJECTS_PINNING_MATCHERS.has(chain.matcher)) { + frame.pinned.add(subject); + } + }, + 'Program:exit'(): void { + const root = stack[0]; + if (root !== undefined) { + flush(root); + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin-code-quality/tests/configs/recommended.test.ts b/packages/eslint-plugin-code-quality/tests/configs/recommended.test.ts index 6061d48..543adf0 100644 --- a/packages/eslint-plugin-code-quality/tests/configs/recommended.test.ts +++ b/packages/eslint-plugin-code-quality/tests/configs/recommended.test.ts @@ -12,6 +12,7 @@ function severityOf(entry: unknown): unknown { const OMITTED_FROM_PRESETS = [ 'interface-prefix-i', 'no-template-trim-empty-ternary', + 'no-message-only-throw-assertion', ]; describe('presets', () => { diff --git a/packages/eslint-plugin-code-quality/tests/rules/no-message-only-throw-assertion.test.ts b/packages/eslint-plugin-code-quality/tests/rules/no-message-only-throw-assertion.test.ts new file mode 100644 index 0000000..297580e --- /dev/null +++ b/packages/eslint-plugin-code-quality/tests/rules/no-message-only-throw-assertion.test.ts @@ -0,0 +1,146 @@ +import { ruleTester } from '@noctcore/eslint-test-utils'; + +import { noMessageOnlyThrowAssertionRule } from '../../src/rules/no-message-only-throw-assertion'; + +ruleTester.run('no-message-only-throw-assertion', noMessageOnlyThrowAssertionRule, { + valid: [ + // An error class argument pins the type. + { code: "it('refuses', () => { expect(() => parse('x')).toThrow(ValidationError); });" }, + { code: "it('refuses', async () => { await expect(load(1)).rejects.toThrowError(NotFoundError); });" }, + { code: "it('refuses', () => { expect(() => run()).toThrow(errors.Forbidden); });" }, + // A structural rejection assertion instead of a throw matcher. + { + code: "it('refuses', async () => { await expect(load(1)).rejects.toMatchObject({ code: 'NOT_FOUND' }); });", + }, + // An asymmetric matcher argument checks the shape, not only the wording. + { + code: "it('refuses', () => { expect(() => run()).toThrow(expect.objectContaining({ code: 'E_LOCKED' })); });", + }, + // A negated throw assertion says nothing about the class. + { code: "it('stays quiet', () => { expect(() => writePrompt('user-1:a')).not.toThrow(); });" }, + { code: "it('resolves', async () => { await expect(load(1)).resolves.not.toThrow(); });" }, + // An error instance pins the class under Vitest semantics (the default). + { code: "it('refuses', () => { expect(() => run()).toThrow(new ForbiddenError('No access')); });" }, + // Class and message asserted separately on the same subject. + { + code: ` + it('refuses', async () => { + await expect(service.find(id)).rejects.toThrow(NotFoundError); + await expect(service.find(id)).rejects.toThrow('Employee not found.'); + }); + `, + }, + { + code: ` + it('refuses', async () => { + await expect(service.find(id)).rejects.toBeInstanceOf(NotFoundError); + await expect(service.find(id)).rejects.toThrow('Employee not found.'); + }); + `, + }, + // A configured helper that pins the class, called on the same subject. + { + code: ` + it('refuses', async () => { + await expectRejectsDomainError(service.find(id), { type: NotFoundError, appCode: 'EMPLOYEE_NOT_FOUND' }); + await expect(service.find(id)).rejects.toThrow('Employee not found.'); + }); + `, + options: [{ assertionHelpers: ['^expectRejectsDomainError$'] }], + }, + // allowMessageOnly keeps only the argless form in scope. + { + code: "it('refuses', () => { expect(() => toApiPath(url)).toThrow('Refusing a non-relative api path'); });", + options: [{ allowMessageOnly: true }], + }, + // A matcher outside the configured list is not a throw assertion. + { + code: "it('refuses', () => { expect(() => run()).toThrowError(); });", + options: [{ throwMatchers: ['toThrow'] }], + }, + // Not an expect chain. + { code: 'emitter.toThrow();' }, + ], + invalid: [ + // Settly shape: a Prisma rejection asserted with no argument. + { + code: ` + it('blocks the cross-tenant delete', async () => { + await expect(client.firma.delete({ where: { id: firmaA.id } })).rejects.toThrow(); + }); + `, + errors: [{ messageId: 'bareThrow', data: { matcher: 'toThrow' } }], + }, + { + code: "it('throws', () => { expect(() => parse('x')).toThrowError(); });", + errors: [{ messageId: 'bareThrow', data: { matcher: 'toThrowError' } }], + }, + // Settly shape: a string message only. + { + code: "it('refuses', () => { expect(() => toApiPath(url)).toThrow('Refusing a non-relative api path'); });", + errors: [{ messageId: 'messageOnlyThrow', data: { matcher: 'toThrow' } }], + }, + { + code: "it('bubbles', async () => { await expect(run()).rejects.toThrow('boom'); });", + errors: [{ messageId: 'messageOnlyThrow' }], + }, + // A regex, a template and a built RegExp are all message checks. + { + code: "it('refuses', () => { expect(() => run()).toThrow(/not in stock/); });", + errors: [{ messageId: 'messageOnlyThrow' }], + }, + { + code: 'it(\'refuses\', () => { expect(() => run()).toThrow(`Queue ${name} missing`); });', + errors: [{ messageId: 'messageOnlyThrow' }], + }, + { + code: 'it(\'refuses\', () => { expect(() => service.onModuleInit()).toThrow(new RegExp(`Queue "${missing}" is declared`)); });', + errors: [{ messageId: 'messageOnlyThrow' }], + }, + // Under Jest semantics an error instance compares only its message. + { + code: "it('refuses', () => { expect(() => run()).toThrow(new ForbiddenError('No access')); });", + options: [{ trustErrorInstances: false }], + errors: [{ messageId: 'messageOnlyThrow' }], + }, + // allowMessageOnly still reports the argless form. + { + code: "it('throws', async () => { await expect(load()).rejects.toThrow(); });", + options: [{ allowMessageOnly: true }], + errors: [{ messageId: 'bareThrow' }], + }, + // Pinning a DIFFERENT subject does not pair. + { + code: ` + it('refuses', async () => { + await expect(service.find(a)).rejects.toThrow(NotFoundError); + await expect(service.find(b)).rejects.toThrow('Employee not found.'); + }); + `, + errors: [{ messageId: 'messageOnlyThrow', line: 4 }], + }, + // A pin in another test does not pair. + { + code: ` + it('pins', async () => { await expect(service.find(id)).rejects.toThrow(NotFoundError); }); + it('words', async () => { await expect(service.find(id)).rejects.toThrow('Employee not found.'); }); + `, + errors: [{ messageId: 'messageOnlyThrow', line: 3 }], + }, + // An unconfigured helper does not pin. + { + code: ` + it('refuses', async () => { + await expectRejectsDomainError(service.find(id), { type: NotFoundError, appCode: 'EMPLOYEE_NOT_FOUND' }); + await expect(service.find(id)).rejects.toThrow('Employee not found.'); + }); + `, + errors: [{ messageId: 'messageOnlyThrow' }], + }, + // Outside a test callback (a shared helper) it still reports. + { + code: 'export async function expectRefusal(p) { await expect(p).rejects.toThrow(); }', + errors: [{ messageId: 'bareThrow' }], + }, + ], +}); diff --git a/site/scripts/parity.test.ts b/site/scripts/parity.test.ts index 03beb80..61860ae 100644 --- a/site/scripts/parity.test.ts +++ b/site/scripts/parity.test.ts @@ -21,7 +21,7 @@ import { describe, expect, test } from 'bun:test'; import { listRuleDocs, loadInventory } from './inventory'; -const EXPECTED_RULE_COUNT = 113; +const EXPECTED_RULE_COUNT = 114; // Source, not dist: a rule added and not yet built must still fail here. const inventory = await loadInventory('src'); From dcceeeac7a003873b5ed7c965d164af51686f478 Mon Sep 17 00:00:00 2001 From: Shironex Date: Wed, 30 Sep 2026 12:12:01 +0200 Subject: [PATCH 02/16] feat(code-quality): add no-sleep-in-unit-tests Reports a real sleep in a unit test file: a promise whose executor resolves from setTimeout, setTimeout from timers/promises, and promisify(setTimeout). A zero or omitted delay (allowZeroDelay, the act() flush), a reject-only timeout guard, a deadline whose handle is kept for clearTimeout, and any file that installs fake timers are left alone. Opt-in, left out of recommended. Adds a visitor-key walkSome helper to the plugin's ast utils. --- packages/eslint-plugin-code-quality/README.md | 4 + .../docs/rules/no-sleep-in-unit-tests.md | 142 ++++++++ .../src/configs/recommended.ts | 2 + .../src/rules/index.ts | 2 + .../src/rules/no-sleep-in-unit-tests.ts | 321 ++++++++++++++++++ .../src/utils/ast.ts | 37 ++ .../tests/configs/recommended.test.ts | 1 + .../rules/no-sleep-in-unit-tests.test.ts | 186 ++++++++++ site/scripts/parity.test.ts | 2 +- 9 files changed, 696 insertions(+), 1 deletion(-) create mode 100644 packages/eslint-plugin-code-quality/docs/rules/no-sleep-in-unit-tests.md create mode 100644 packages/eslint-plugin-code-quality/src/rules/no-sleep-in-unit-tests.ts create mode 100644 packages/eslint-plugin-code-quality/tests/rules/no-sleep-in-unit-tests.test.ts diff --git a/packages/eslint-plugin-code-quality/README.md b/packages/eslint-plugin-code-quality/README.md index 37fd4c4..8d0ffb7 100644 --- a/packages/eslint-plugin-code-quality/README.md +++ b/packages/eslint-plugin-code-quality/README.md @@ -70,6 +70,9 @@ export default [ 'noctcore-code-quality/no-message-only-throw-assertion': ['error', { assertionHelpers: ['^expectRejectsDomainError$'], }], + 'noctcore-code-quality/no-sleep-in-unit-tests': ['error', { + integrationMarkers: ['.integration.', '/e2e/'], + }], }, }, ]; @@ -96,6 +99,7 @@ export default [ | [`no-pr-reference-comments`](https://noctcore.github.io/eslint-plugins/rules/code-quality/no-pr-reference-comments/) | Disallow PR/issue references in comments. They belong in commit messages and PR descriptions, where they do not rot when the repo moves, the issue tracker migrates, or the numbering changes. | โœ… | | | | | | [`no-process-exit`](https://noctcore.github.io/eslint-plugins/rules/code-quality/no-process-exit/) | Disallow `process.exit()` outside bootstrap/shutdown paths and standalone CLIs. Application and service code must throw or reject so the lifecycle can shut down gracefully. | โœ… | | | | | | [`no-real-network-in-unit-tests`](https://noctcore.github.io/eslint-plugins/rules/code-quality/no-real-network-in-unit-tests/) | Unit tests must not perform real network I/O: mock the HTTP client, or move the test to an integration suite. | โœ… | | | | | +| [`no-sleep-in-unit-tests`](https://noctcore.github.io/eslint-plugins/rules/code-quality/no-sleep-in-unit-tests/) | Unit tests must not sleep on the real clock (`new Promise((r) => setTimeout(r, n))`, `timers/promises`): fake the timers or wait for the condition. | ๐Ÿ”˜ | | | | | | [`no-swallowed-assertion`](https://noctcore.github.io/eslint-plugins/rules/code-quality/no-swallowed-assertion/) | Disallow assertions inside a `try` whose `catch` neither rethrows nor asserts, and `.catch()` handlers that swallow an `expect(...).rejects`/`.resolves` failure: the assertion fails, the error is dropped, and the test passes. | โœ… | | | | | | [`no-template-trim-empty-ternary`](https://noctcore.github.io/eslint-plugins/rules/code-quality/no-template-trim-empty-ternary/) | Disallow inline `