mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
ci(client): lint:warnings ratchets the ESLint warning counts per rule
eslint.config.mjs keeps most rules at warn so that CI stayed green on a codebase that was never linted, and CI runs eslint without --max-warnings. Warnings therefore only ever accumulated: 1218 today, among them 194 unused variables and 144 missing effect dependencies in app code, the kind behind the stale closures #2610 chased. lint:warnings runs ESLint over the client with its own config and counts the warnings per rule, separately for app code and for tests, so the headroom from fixing a test cannot be spent in app code. Messages that an eslint-disable comment silenced are counted as a third group, so an inline disable is no way past the ratchet either. No count may pass its entry in scripts/eslint-baseline.json, and --update only lowers. Any error, a parse failure included, fails the check too, so it can replace the plain lint:check step in CI instead of running ESLint twice.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
@@ -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') }))
|
||||
@@ -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<LintResult[]>}
|
||||
*/
|
||||
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 <file> 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
|
||||
}
|
||||
@@ -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<typeof result>[], 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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user