diff --git a/client/CLAUDE.md b/client/CLAUDE.md index 9849cce40..2c0a88345 100644 --- a/client/CLAUDE.md +++ b/client/CLAUDE.md @@ -9,6 +9,7 @@ npm run dev # Vite dev server; proxies the API/ws/uploads/MCP/OAut npm run build # prebuild generates PWA icons, then vite build npm run typecheck # tsc --noEmit (CI) npm run lint # eslint . (lint:check in CI) +npm run lint:warnings # eslint, failing on any error and on any warning count per rule above scripts/eslint-baseline.json (app code, tests and eslint-disable'd messages counted apart; --update lowers it) npm run lint:pages # enforce the Page pattern (CI gate) npm run lint:rtl # physical left/right sides and rtl-lint-disable comments may only shrink per file (CI gate; --list shows them, --update lowers both baselines) npm run lint:size # no source file or stylesheet past 1000 lines, no test past 2000, counted wrapped at 120 columns; longer ones may only shrink (CI gate; --update lowers the baseline) diff --git a/client/package.json b/client/package.json index 74f5ac217..643d5e044 100644 --- a/client/package.json +++ b/client/package.json @@ -17,6 +17,7 @@ "test:coverage": "vitest run --coverage", "lint": "eslint .", "lint:check": "eslint .", + "lint:warnings": "node scripts/eslint-ratchet.mjs", "lint:pages": "node scripts/check-page-pattern.mjs", "lint:rtl": "node scripts/rtl-lint.mjs", "lint:size": "node scripts/size-lint.mjs", diff --git a/client/scripts/eslint-baseline.json b/client/scripts/eslint-baseline.json new file mode 100644 index 000000000..52c569c58 --- /dev/null +++ b/client/scripts/eslint-baseline.json @@ -0,0 +1,25 @@ +{ + "src": { + "@typescript-eslint/no-explicit-any": 174, + "@typescript-eslint/no-unused-expressions": 4, + "@typescript-eslint/no-unused-vars": 194, + "no-empty": 34, + "no-useless-assignment": 7, + "no-useless-escape": 1, + "preserve-caught-error": 53, + "react-hooks/exhaustive-deps": 144, + "react-refresh/only-export-components": 48, + "unused-disable-directive": 7 + }, + "suppressed": { + "@typescript-eslint/no-explicit-any": 50, + "react-hooks/exhaustive-deps": 64 + }, + "tests": { + "@typescript-eslint/no-explicit-any": 494, + "@typescript-eslint/no-this-alias": 1, + "@typescript-eslint/no-unsafe-function-type": 2, + "@typescript-eslint/no-unused-vars": 54, + "react-refresh/only-export-components": 1 + } +} diff --git a/client/scripts/eslint-ratchet.mjs b/client/scripts/eslint-ratchet.mjs new file mode 100644 index 000000000..57f9fce32 --- /dev/null +++ b/client/scripts/eslint-ratchet.mjs @@ -0,0 +1,23 @@ +#!/usr/bin/env node +/* + * lint:warnings: ESLint warnings may only go down. + * + * Most rules in eslint.config.mjs are warnings, set that way to keep CI green + * on code that was never linted, and a warning never fails a build. So they + * piled up: unused variables hiding dead props, missing effect dependencies + * of the kind that has already shown stale money. This runs ESLint over the + * client and counts the warnings per rule, separately for app code and + * tests, and the messages an eslint-disable comment silenced as a third + * group. Each count may not pass its entry in scripts/eslint-baseline.json. + * Any error fails as well, so the check can stand in for `npm run lint:check`. + * + * npm run lint:warnings check against the baseline (CI) + * npm run lint:warnings -- --update lower the baseline to today's counts; + * it never raises an entry + */ +import { fileURLToPath } from 'node:url' +import { check } from './lib/eslint-warnings.mjs' +import { runCli } from './lib/ratchet.mjs' + +const root = fileURLToPath(new URL('..', import.meta.url)) +await runCli('lint:warnings', (args) => check({ root, update: args.includes('--update') })) diff --git a/client/scripts/lib/eslint-warnings.mjs b/client/scripts/lib/eslint-warnings.mjs new file mode 100644 index 000000000..f62d663f8 --- /dev/null +++ b/client/scripts/lib/eslint-warnings.mjs @@ -0,0 +1,122 @@ +/* + * The tally and the comparison behind lint:warnings (scripts/eslint-ratchet.mjs). + */ +import { join } from 'node:path' +import { ESLint } from 'eslint' +import { countMap, lowerCounts, readBaseline, TEST_FILE, toKey, writeBaseline } from './ratchet.mjs' + +/** + * Where a warning is counted: in the app code, in the tests, or, for a + * message an eslint-disable comment silenced, among the suppressed ones. An + * inline disable must not be the cheap way under the ratchet. + */ +export const AREAS = ['src', 'tests', 'suppressed'] + +const areaOf = (key) => (key.startsWith('tests/') || TEST_FILE.test(key) ? 'tests' : 'src') + +const ruleOf = (message) => + message.ruleId ?? (/eslint-disable/.test(message.message) ? 'unused-disable-directive' : '(no rule)') + +/** A baseline: each area an object of rule → positive count. */ +export function areaCounts(value) { + if (!value || typeof value !== 'object' || Array.isArray(value)) return 'it is not an object of area → rule counts' + for (const area of Object.keys(value)) if (!AREAS.includes(area)) return `"${area}" is not one of ${AREAS.join(', ')}` + for (const area of AREAS) { + if (!(area in value)) return `the "${area}" area is missing` + const problem = countMap(value[area]) + if (problem) return `in "${area}", ${problem}` + } + return null +} + +/** + * The warnings per area and rule in ESLint's results, the files behind each + * count, and every error (an error, a parse failure included, always fails). + */ +export function tally(results, root) { + const counts = Object.fromEntries(AREAS.map((area) => [area, {}])) + const files = Object.fromEntries(AREAS.map((area) => [area, {}])) + const errors = [] + const add = (area, rule, key) => { + counts[area][rule] = (counts[area][rule] ?? 0) + 1 + const where = (files[area][rule] ??= {}) + where[key] = (where[key] ?? 0) + 1 + } + for (const result of results) { + const key = toKey(root, result.filePath) + for (const message of result.messages) { + if (message.fatal || message.severity === 2) errors.push(`${key}:${message.line ?? 0}:${message.column ?? 0} ${message.message} (${ruleOf(message)})`) + else add(areaOf(key), ruleOf(message), key) + } + for (const message of result.suppressedMessages ?? []) add('suppressed', ruleOf(message), key) + } + return { counts, files, errors } +} + +/** + * The part of an ESLint result the tally reads. + * + * @typedef {{ ruleId: string | null, severity: number, message: string, fatal?: boolean, line?: number, column?: number }} LintMessage + * @typedef {{ filePath: string, messages: LintMessage[], suppressedMessages?: LintMessage[] }} LintResult + */ + +/** + * ESLint over the whole client, with its own config, as `npm run lint:check` runs it. + * + * @param {string} root + * @returns {Promise} + */ +export async function lintClient(root) { + return new ESLint({ cwd: root }).lintFiles(['.']) +} + +const sum = (map) => Object.values(map).reduce((a, b) => a + b, 0) + +/** + * Runs ESLint (or lint, in a test) and compares the warnings with the + * baseline at baselinePath. Returns the exit code. + */ +export async function check({ + root, + baselinePath = join(root, 'scripts/eslint-baseline.json'), + update = false, + lint = lintClient, + log = console.log, + error = console.error, +}) { + let baseline = readBaseline(baselinePath, areaCounts) + const { counts, files, errors } = tally(await lint(root), root) + + if (update) { + baseline = Object.fromEntries(AREAS.map((area) => [area, lowerCounts(baseline[area], counts[area])])) + writeBaseline(baselinePath, baseline) + } + + for (const line of errors) error(`FAIL ${line}`) + let grown = 0 + let lowerable = 0 + for (const area of AREAS) { + for (const [rule, n] of Object.entries(counts[area])) { + const allowed = baseline[area][rule] ?? 0 + if (n <= allowed) continue + grown++ + const top = Object.entries(files[area][rule]) + .sort(([, a], [, b]) => b - a) + .slice(0, 5) + .map(([key, k]) => `${key} (${k})`) + error(`FAIL ${area}: ${n} ${rule} warning(s), baseline ${allowed}. Most in: ${top.join(', ')}`) + } + for (const [rule, allowed] of Object.entries(baseline[area])) if ((counts[area][rule] ?? 0) < allowed) lowerable++ + } + if (grown) { + error( + 'Fix the new warning rather than silencing it: an eslint-disable comment is counted under "suppressed". ' + + 'Run npx eslint to see each one.', + ) + } + if (lowerable && !update) log(`${lowerable} rule count(s) fell below the baseline: run with --update to lower it.`) + log( + `eslint: ${errors.length} error(s); warnings ${AREAS.map((area) => `${area} ${sum(counts[area])}/${sum(baseline[area])}`).join(', ')} (now/baseline)`, + ) + return errors.length || grown ? 1 : 0 +} diff --git a/client/tests/unit/scripts/eslint-ratchet.test.ts b/client/tests/unit/scripts/eslint-ratchet.test.ts new file mode 100644 index 000000000..abe16e3bf --- /dev/null +++ b/client/tests/unit/scripts/eslint-ratchet.test.ts @@ -0,0 +1,131 @@ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { areaCounts, check, tally } from '../../../scripts/lib/eslint-warnings.mjs'; +import { RatchetError } from '../../../scripts/lib/ratchet.mjs'; +import { ratchetTree, type RatchetTree } from '../../helpers/ratchetFixture'; + +let tree: RatchetTree; +afterEach(() => tree?.remove()); + +const BASELINE = 'scripts/eslint-baseline.json'; + +interface Message { + ruleId: string | null; + severity: number; + message?: string; + fatal?: boolean; + line?: number; + column?: number; +} + +const warn = (ruleId: string | null, message = 'm'): Message => ({ ruleId, severity: 1, message }); + +function result(root: string, key: string, messages: Message[], suppressed: Message[] = []) { + return { + filePath: join(root, key), + messages: messages.map((m) => ({ message: 'm', ...m })), + suppressedMessages: suppressed.map((m) => ({ message: 'm', ...m })), + }; +} + +const empty = { src: {}, tests: {}, suppressed: {} }; + +async function run(baseline: object, results: (root: string) => ReturnType[], update = false) { + tree = ratchetTree({ [BASELINE]: JSON.stringify(baseline) }); + return check({ + root: tree.root, + baselinePath: tree.path(BASELINE), + update, + lint: async (root: string) => results(root), + ...tree.out, + }); +} + +describe('lint:warnings', () => { + it('ESLINT-001: counts warnings per rule for app code, tests and suppressed messages', () => { + const root = join('/r'); + const { counts, files, errors } = tally( + [ + result( + root, + 'src/a.tsx', + [warn('no-empty'), warn('no-empty'), warn(null, 'Unused eslint-disable directive')], + [warn('react-hooks/exhaustive-deps')] + ), + result(root, 'src/a.test.tsx', [warn('@typescript-eslint/no-explicit-any')]), + result(root, 'tests/unit/b.ts', [warn('@typescript-eslint/no-explicit-any')]), + ], + root + ); + expect(counts).toEqual({ + src: { 'no-empty': 2, 'unused-disable-directive': 1 }, + tests: { '@typescript-eslint/no-explicit-any': 2 }, + suppressed: { 'react-hooks/exhaustive-deps': 1 }, + }); + expect(files.src['no-empty']).toEqual({ 'src/a.tsx': 2 }); + expect(errors).toEqual([]); + }); + + it('ESLINT-002: passes when no count passes its entry', async () => { + const code = await run({ ...empty, src: { 'no-empty': 2 } }, (root) => [ + result(root, 'src/a.ts', [warn('no-empty')]), + ]); + expect(code).toBe(0); + expect(tree.log.join('\n')).toMatch(/fell below the baseline/); + }); + + it('ESLINT-003: fails a new warning and names the files behind it', async () => { + const code = await run({ ...empty, src: { 'no-empty': 1 } }, (root) => [ + result(root, 'src/a.ts', [warn('no-empty')]), + result(root, 'src/b.ts', [warn('no-empty')]), + ]); + expect(code).toBe(1); + expect(tree.error.join('\n')).toMatch( + /src: 2 no-empty warning\(s\), baseline 1\. Most in: src\/a\.ts \(1\), src\/b\.ts \(1\)/ + ); + }); + + it('ESLINT-004: an eslint-disable comment does not get a warning past the ratchet', async () => { + const code = await run(empty, (root) => [result(root, 'src/a.ts', [], [warn('react-hooks/exhaustive-deps')])]); + expect(code).toBe(1); + expect(tree.error.join('\n')).toMatch(/suppressed: 1 react-hooks\/exhaustive-deps warning/); + }); + + it('ESLINT-005: app code cannot spend the headroom the tests left', async () => { + const code = await run({ ...empty, tests: { 'no-empty': 5 } }, (root) => [ + result(root, 'src/a.ts', [warn('no-empty')]), + ]); + expect(code).toBe(1); + }); + + it('ESLINT-006: any error fails, a parse failure included', async () => { + const code = await run(empty, (root) => [ + result(root, 'src/a.ts', [ + { ruleId: null, severity: 2, fatal: true, message: 'Parsing error', line: 3, column: 1 }, + ]), + ]); + expect(code).toBe(1); + expect(tree.error.join('\n')).toMatch(/src\/a\.ts:3:1 {2}Parsing error/); + }); + + it('ESLINT-007: --update lowers and drops counts and never raises one', async () => { + await run( + { src: { 'no-empty': 3, 'no-useless-escape': 1 }, tests: {}, suppressed: {} }, + (root) => [result(root, 'src/a.ts', [warn('no-empty'), warn('no-unused-vars')])], + true + ); + expect(JSON.parse(readFileSync(tree.path(BASELINE), 'utf8'))).toEqual({ + src: { 'no-empty': 1 }, + suppressed: {}, + tests: {}, + }); + }); + + it('ESLINT-008: a baseline without all three areas, or with a stray one, stops the check', async () => { + expect(areaCounts({ src: {}, tests: {} })).toMatch(/"suppressed" area is missing/); + expect(areaCounts({ ...empty, other: {} })).toMatch(/"other" is not one of/); + expect(areaCounts({ ...empty, src: { a: -1 } })).toMatch(/in "src"/); + await expect(run({ src: {} }, () => [])).rejects.toThrow(RatchetError); + }); +});