diff --git a/.changeset/code-quality-test-discipline.md b/.changeset/code-quality-test-discipline.md new file mode 100644 index 0000000..c4b11aa --- /dev/null +++ b/.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>` or check it with `satisfies`. Options: `mockFactories`, `allowTargets` for types too wide to `Pick` from. diff --git a/packages/eslint-plugin-code-quality/README.md b/packages/eslint-plugin-code-quality/README.md index 23480f0..45f4a0d 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,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 `