feat(code-quality): add test-discipline rules

.changeset/code-quality-test-discipline.md

@@ -0,0 +1,17 @@
+---
+'@noctcore/eslint-plugin-code-quality': minor
+---
+
+Four new opt-in test-discipline rules and two stricter existing rules.
+
+**A project that spreads `recommended` sees new errors from two rules it already runs:**
+
+- `no-vacuous-expect` now reports a test whose only assertion is that the render root is present (`soleRenderRootExpect`): `expect(container).not.toBeEmptyDOMElement()`, `expect(container.firstChild).toBeInTheDocument()`, `expect(container.innerHTML).not.toBe('')` and the like, on a `container` or `baseElement` bound from a call (`const { container } = render(...)`, `view.container`, `render(...).container`). Such a test passes for anything that renders, an error fallback included. Assert on a role, a label or a text instead, or set the new `renderRoots` option to `[]` to keep the old behaviour.
+- `skipped-tests-need-tracking` now also reads `node:test` skips: a `skip` or `todo` option whose value is a truthy literal (`{ skip: true }`, `{ skip: 'reason' }`, `{ todo: 1 }`) on `test` / `it` / `describe` / `suite` (and a `t.test` subtest), and `t.skip()` / `t.todo()` (awaited or not) as a statement of the test callback's own body. Only unconditional skips are reported: a computed value (`{ skip: process.platform === 'win32' }`, `{ skip: !ready }`) or a `t.skip(...)` inside an `if` is a platform guard and stays silent. Add an issue URL or `TODO(@owner)` near the skip, as for `.skip(`.
+
+**New rules, left out of `recommended`** (each needs a per-project fact; enabling them is up to you):
+
+- `no-message-only-throw-assertion`: `toThrow()` / `toThrowError()` with no argument, or with only a string, template or regex, and the message snapshots `toThrowErrorMatchingSnapshot()` / `toThrowErrorMatchingInlineSnapshot()`, sync or after `.rejects`. Any error passes those, including a `TypeError` from a broken mock. A class argument, an asymmetric matcher, `.rejects.toMatchObject(...)` and `.not.toThrow()` are fine, and a message-only assertion is accepted when the same test pins the class of the same subject in the same or an enclosing block (a bare `toThrow()` is never excused that way). Options: `throwMatchers`, `allowMessageOnly` (report only the argless form), `trustErrorInstances` (set `false` under Jest, where `toThrow(new X('m'))` and `.rejects.toEqual(new X('m'))` compare only the message; a `.rejects.toMatchObject({ message })` or `toHaveProperty('message')` is never a class pin) and `assertionHelpers`.
+- `no-sleep-in-unit-tests`: a real sleep in a unit test file: `new Promise((r) => setTimeout(r, n))`, `setTimeout` from `timers/promises`, and `promisify(setTimeout)`. A zero or omitted delay (`allowZeroDelay`, on by default) a reject-only timeout guard and a deadline whose kept handle the file passes to `clearTimeout` are not sleeps, and a file that installs fake timers (`fakeTimerMethods`, default `useFakeTimers`) is not checked, since its waits are virtual. Same `testFileSuffixes` / `integrationMarkers` options as `no-real-network-in-unit-tests`.
+- `no-real-clock-in-unit-tests`: `Date.now()`, an argless `new Date()` and `Date()` in a unit test file that never fakes the clock (`useFakeTimers`, `setSystemTime`, a `Date.now` spy, a stubbed or replaced global `Date`, or a mocked module matching `clockModules`). An offset from now (`Date.now() + 60_000`, `new Date().getTime() - 1000`) is allowed.
+- `typed-mock-over-double-cast`: an object literal containing `jest.fn()` / `vi.fn()` cast `as unknown as T` (or through `any` or `never`, in `as` or angle-bracket form). Type it as `jest.Mocked<Pick<T, ...>>` or check it with `satisfies`. Options: `mockFactories`, `allowTargets` for types too wide to `Pick` from.

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,24 @@ 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$'],
+      }],
+      'noctcore-code-quality/no-sleep-in-unit-tests': ['error', {
+        integrationMarkers: ['.integration.', '/e2e/'],
+      }],
+      'noctcore-code-quality/no-real-clock-in-unit-tests': ['error', {
+        clockModules: ['**/common/clock'],
+      }],
+      'noctcore-code-quality/typed-mock-over-double-cast': ['error', {
+        allowTargets: ['PrismaService'],
+      }],
+    },
+  },
 ];
 ```
 
@@ -79,15 +100,19 @@ 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. | ✅ |  |  |  |  |
+| [`no-real-clock-in-unit-tests`](https://noctcore.github.io/eslint-plugins/rules/code-quality/no-real-clock-in-unit-tests/) | Unit tests must not read the real clock (`Date.now()`, `new Date()`) unless the file fakes it; an offset from now (`Date.now() + 60_000`) is allowed. | 🔘 |  |  |  |  |
 | [`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 `<template>.trim() === '' ? fallback : <template>.trim()` patterns. Extract to a named utility so the expression is built once and is unit-testable in one place. | 🔘 |  |  |  |  |
-| [`no-vacuous-expect`](https://noctcore.github.io/eslint-plugins/rules/code-quality/no-vacuous-expect/) | Disallow vacuous expects (`typeof` checks, literal tautologies, a sole `toBeDefined`/`toBeTruthy`): a test must assert behaviour that a real regression would break. | ✅ |  |  |  |  |
+| [`no-vacuous-expect`](https://noctcore.github.io/eslint-plugins/rules/code-quality/no-vacuous-expect/) | Disallow vacuous expects (`typeof` checks, literal tautologies, a sole `toBeDefined`/`toBeTruthy`, a sole presence check on the render root): a test must assert behaviour that a real regression would break. | ✅ |  |  |  |  |
 | [`prefer-early-return`](https://noctcore.github.io/eslint-plugins/rules/code-quality/prefer-early-return/) | Prefer guard clauses (early return) over wrapping the whole function body in a multi-statement `if` without an `else`. | ✅ |  |  |  |  |
-| [`skipped-tests-need-tracking`](https://noctcore.github.io/eslint-plugins/rules/code-quality/skipped-tests-need-tracking/) | Skipped tests (`.skip` / `.fixme` / `xit` / `xdescribe`) must carry a tracking marker (an issue URL or `TODO(@owner)`) on or above the line, so the debt has an owner instead of rotting silently. | ✅ |  |  |  |  |
+| [`skipped-tests-need-tracking`](https://noctcore.github.io/eslint-plugins/rules/code-quality/skipped-tests-need-tracking/) | Skipped tests (`.skip` / `.fixme` / `xit` / `xdescribe`, and unconditional `node:test` `{ skip }` / `{ todo }` / `t.skip()`) must carry a tracking marker (an issue URL or `TODO(@owner)`) on or above the line, so the debt has an owner instead of rotting silently. | ✅ |  |  |  |  |
+| [`typed-mock-over-double-cast`](https://noctcore.github.io/eslint-plugins/rules/code-quality/typed-mock-over-double-cast/) | Disallow an object literal of `jest.fn()` / `vi.fn()` mocks cast `as unknown as T`: the double cast switches type checking off, so a mock of a renamed or removed method keeps passing. | 🔘 |  |  |  |  |
 <!-- end generated rules -->
 
 ## Severity policy

packages/eslint-plugin-code-quality/docs/rules/no-message-only-throw-assertion.md

@@ -0,0 +1,198 @@
+# `noctcore-code-quality/no-message-only-throw-assertion`
+
+> A throw assertion must pin the error class, not accept any error or only its wording.
+
+<!-- begin generated rule header -->
+🔘 Opt-in: not in `recommended` · 💭 Type information: not needed
+<!-- end generated rule header -->
+
+## 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, a
+  `RegExp` built with `new RegExp(...)`, or `expect.objectContaining({ message: ... })`. A
+  snapshot of the error (`toThrowErrorMatchingSnapshot()`, `toThrowErrorMatchingInlineSnapshot()`)
+  records only its message, so it is a message check too, whatever its argument.
+
+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<unknown>) {
+  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<unknown>) {
+  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 more than the message, or a configured `assertionHelpers` call whose first argument is that
+subject. The `.rejects` matchers that count:
+
+- `toBeInstanceOf(X)`;
+- `toMatchObject(...)`, unless its argument is an object with no key but `message` (`{ message: 'm' }`
+  pins nothing else), or an error instance while `trustErrorInstances` is off;
+- `toHaveProperty(path, ...)`, unless the path is `'message'` or `['message']`;
+- `toEqual(...)` / `toStrictEqual(...)`, unless the argument is an error instance while
+  `trustErrorInstances` is off: Jest's `equals()` compares two errors by their message alone. Some refusals differ
+only in wording, and there the sentence is the assertion; the class next to it keeps it honest.
+
+The pin must run whenever the message check runs: it has to sit in the same block as the message
+check or in a block that encloses it. A class pinned in one branch of an `if` does not excuse a
+message check in the other branch, or one after the `if`. A bare `toThrow()` is never excused by a
+pin: a later call on the same subject is usually a different scenario, and any error satisfies it.
+
+```ts bad filename=src/employees/employee.service.test.ts reports=2
+it('refuses', async () => {
+  if (strict) {
+    await expect(service.find('e-other')).rejects.toThrow(NotFoundError);
+  } else {
+    await expect(service.find('e-other')).rejects.toThrow('Employee not found.');
+  }
+  repository.find.mockResolvedValue({ deleted: true });
+  await expect(service.find('e-other')).rejects.toThrow();
+});
+```
+
+```ts good filename=src/employees/employee.service.test.ts
+it('refuses', async () => {
+  await expect(service.find('e-other')).rejects.toThrow(NotFoundError);
+  if (!strict) {
+    await expect(service.find('e-other')).rejects.toThrow('Employee not found.');
+  }
+  repository.find.mockResolvedValue({ deleted: true });
+  await expect(service.find('e-other')).rejects.toThrow(GoneError);
+});
+```
+
+## 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", "toThrowErrorMatchingSnapshot", "toThrowErrorMatchingInlineSnapshot"]` | Matchers that assert a throw or a rejection. A name ending in `MatchingSnapshot` or `MatchingInlineSnapshot` is always a message check. |
+| `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'))` and `.rejects.toEqual(new X('m'))` compare 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');
+});
+```
+
+The same goes for a `.rejects` equality check used as the pin:
+
+```ts bad filename=src/auth/guard.spec.ts options={"trustErrorInstances":false}
+it('refuses a guest', async () => {
+  await expect(guard.load(guest)).rejects.toEqual(new ForbiddenError('No access'));
+  await expect(guard.load(guest)).rejects.toThrow('No access');
+});
+```
+
+```ts good filename=src/auth/guard.spec.ts options={"trustErrorInstances":false}
+it('refuses a guest', async () => {
+  await expect(guard.load(guest)).rejects.toMatchObject({ name: 'ForbiddenError', code: 'E_FORBIDDEN' });
+  await expect(guard.load(guest)).rejects.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.