From 1b60de27f443c27d9babd4e62b77a6cf46e5ddfb Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 8 Oct 2026 12:57:12 +0200 Subject: [PATCH] ci(client): lint:warnings fails an inline rule config comment A block comment such as /* eslint @typescript-eslint/no-explicit-any: off */ turns a rule off for its file without leaving a message behind, neither among the warnings nor among the suppressed ones, so a new file could switch any warning rule off and the ratchet stayed green. eslint-disable comments were already counted under "suppressed"; this was the way around them. lintClient now reads the block comments of every file ESLint linted from the TypeScript parser, so the same text in a string, a template or a regex does not count, and the tally reports each config comment as an error. Nothing in the client uses one today, so the check is zero tolerance. The tests baseline drops by one no-unused-vars warning that is gone. --- client/CLAUDE.md | 2 +- client/scripts/eslint-baseline.json | 2 +- client/scripts/eslint-ratchet.mjs | 5 +- client/scripts/lib/eslint-warnings.mjs | 65 +++++++++++++++++-- .../tests/unit/scripts/eslint-ratchet.test.ts | 46 ++++++++++++- 5 files changed, 111 insertions(+), 9 deletions(-) diff --git a/client/CLAUDE.md b/client/CLAUDE.md index c055d0689..29cf71766 100644 --- a/client/CLAUDE.md +++ b/client/CLAUDE.md @@ -9,7 +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: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; a `/* eslint : ... */` config comment fails outright; --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 (Playwright specs and fixtures under e2e/ included) past 2000, counted wrapped at 120 columns; longer ones may only shrink (CI gate; --update lowers the baseline) diff --git a/client/scripts/eslint-baseline.json b/client/scripts/eslint-baseline.json index 52c569c58..0bc7d0b51 100644 --- a/client/scripts/eslint-baseline.json +++ b/client/scripts/eslint-baseline.json @@ -19,7 +19,7 @@ "@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, + "@typescript-eslint/no-unused-vars": 53, "react-refresh/only-export-components": 1 } } diff --git a/client/scripts/eslint-ratchet.mjs b/client/scripts/eslint-ratchet.mjs index 73183999f..472bdff46 100644 --- a/client/scripts/eslint-ratchet.mjs +++ b/client/scripts/eslint-ratchet.mjs @@ -9,7 +9,10 @@ * 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`. + * Any error fails as well, so the check can stand in for `npm run lint:check`, + * and so does a block comment that configures a rule inline (such as one that + * turns no-explicit-any off): it leaves no message to count, so it would + * otherwise be the free way past the ratchet. * * npm run lint:warnings check against the baseline (CI) * npm run lint:warnings -- --update lower the baseline to today's counts; diff --git a/client/scripts/lib/eslint-warnings.mjs b/client/scripts/lib/eslint-warnings.mjs index 998fa28c9..b72c243e1 100644 --- a/client/scripts/lib/eslint-warnings.mjs +++ b/client/scripts/lib/eslint-warnings.mjs @@ -3,12 +3,14 @@ */ import { ESLint } from 'eslint'; import { join } from 'node:path'; -import { countMap, lowerCounts, readBaseline, TEST_FILE, toKey, writeBaseline } from './ratchet.mjs'; +import ts from 'typescript'; +import { countMap, lowerCounts, readBaseline, readText, 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. + * inline disable must not be the cheap way under the ratchet, and an inline + * rule config, which silences without a trace, fails outright (see tally). */ export const AREAS = ['src', 'tests', 'suppressed']; @@ -30,9 +32,55 @@ export function areaCounts(value) { return null; } +const scriptKind = (file) => + file.endsWith('.tsx') + ? ts.ScriptKind.TSX + : /\.[cm]?ts$/.test(file) + ? ts.ScriptKind.TS + : file.endsWith('.jsx') + ? ts.ScriptKind.JSX + : ts.ScriptKind.JS; + +/** ESLint's label for a block comment that configures rules, as opposed to eslint-disable and the like. */ +const INLINE_CONFIG = /^\s*eslint(?:\s|$)/; + +/** + * The block comments in a file that configure ESLint rules, such as + * `/* eslint no-empty: off *\/`, as `line:column comment`. Such a comment + * switches a rule off for the whole file without a single message reaching + * either the warnings or the suppressed ones, so it would walk past the + * ratchet unseen. The comments come from the TypeScript parser, so the same + * text inside a string or a template does not count. + */ +export function inlineConfigsOf(source, file) { + const sf = ts.createSourceFile(file, source, ts.ScriptTarget.Latest, true, scriptKind(file)); + const seen = new Set(); + const found = []; + const collect = (ranges) => { + for (const range of ranges ?? []) { + if (seen.has(range.pos)) continue; + seen.add(range.pos); + if (range.kind !== ts.SyntaxKind.MultiLineCommentTrivia) continue; + const text = source.slice(range.pos, range.end); + if (!INLINE_CONFIG.test(text.slice(2, -2))) continue; + const { line, character } = sf.getLineAndCharacterOfPosition(range.pos); + found.push(`${line + 1}:${character + 1} ${text.replace(/\s+/g, ' ').slice(0, 80)}`); + } + }; + const visit = (node) => { + collect(ts.getLeadingCommentRanges(source, node.pos)); + collect(ts.getTrailingCommentRanges(source, node.pos)); + for (const child of node.getChildren(sf)) visit(child); + }; + visit(sf); + return found; +} + /** * 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). + * An inline rule config comment (see inlineConfigsOf) is an error too: the + * place to change a rule is eslint.config.mjs. */ export function tally(results, root) { const counts = Object.fromEntries(AREAS.map((area) => [area, {}])); @@ -45,6 +93,8 @@ export function tally(results, root) { }; for (const result of results) { const key = toKey(root, result.filePath); + for (const comment of result.inlineConfigs ?? []) + errors.push(`${key}:${comment} configures a rule inline; set it in eslint.config.mjs (inline-config)`); 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)})`); @@ -59,17 +109,22 @@ export function tally(results, root) { * 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 + * @typedef {{ filePath: string, messages: LintMessage[], suppressedMessages?: LintMessage[], inlineConfigs?: string[] }} LintResult */ /** - * ESLint over the whole client, with its own config, as `npm run lint:check` runs it. + * ESLint over the whole client, with its own config, as `npm run lint:check` + * runs it, each result with the inline rule configs of its file. * * @param {string} root * @returns {Promise} */ export async function lintClient(root) { - return new ESLint({ cwd: root }).lintFiles(['.']); + const results = await new ESLint({ cwd: root }).lintFiles(['.']); + return results.map((result) => ({ + ...result, + inlineConfigs: inlineConfigsOf(readText(result.filePath), result.filePath), + })); } const sum = (map) => Object.values(map).reduce((a, b) => a + b, 0); diff --git a/client/tests/unit/scripts/eslint-ratchet.test.ts b/client/tests/unit/scripts/eslint-ratchet.test.ts index abe16e3bf..c1641409c 100644 --- a/client/tests/unit/scripts/eslint-ratchet.test.ts +++ b/client/tests/unit/scripts/eslint-ratchet.test.ts @@ -1,7 +1,7 @@ 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 { areaCounts, check, inlineConfigsOf, lintClient, tally } from '../../../scripts/lib/eslint-warnings.mjs'; import { RatchetError } from '../../../scripts/lib/ratchet.mjs'; import { ratchetTree, type RatchetTree } from '../../helpers/ratchetFixture'; @@ -128,4 +128,48 @@ describe('lint:warnings', () => { expect(areaCounts({ ...empty, src: { a: -1 } })).toMatch(/in "src"/); await expect(run({ src: {} }, () => [])).rejects.toThrow(RatchetError); }); + + it('ESLINT-009: finds the block comments that configure a rule, and nothing in strings, templates or regexes', () => { + const source = [ + '/* eslint @typescript-eslint/no-explicit-any: off */', + 'const a = "/* eslint no-empty: off */";', + 'const b = `${a} /* eslint no-empty: off */`;', + '/* eslint-disable no-empty */', + '/*eslint no-empty:0*/ const c =
{/* eslint no-empty: 1 */}
;', + 'const r = /\\/\\* eslint no-empty/;', + '// eslint no-empty: off', + '/** eslint is the linter */', + ].join('\n'); + expect(inlineConfigsOf(source, 'src/a.tsx')).toEqual([ + '1:1 /* eslint @typescript-eslint/no-explicit-any: off */', + '5:1 /*eslint no-empty:0*/', + '5:39 /* eslint no-empty: 1 */', + ]); + expect(inlineConfigsOf('export const a = 1 /* eslint no-empty: off */\n', 'src/a.ts')).toHaveLength(1); + expect(inlineConfigsOf('export const a = 1\n', 'src/a.ts')).toEqual([]); + }); + + it('ESLINT-010: an inline rule config fails the check although it leaves no message behind', async () => { + const code = await run(empty, (root) => [ + { ...result(root, 'src/a.ts', []), inlineConfigs: ['1:1 /* eslint no-empty: off */'] }, + ]); + expect(code).toBe(1); + expect(tree.error.join('\n')).toMatch( + /src\/a\.ts:1:1 {2}\/\* eslint no-empty: off \*\/ {2}configures a rule inline/ + ); + }); + + it('ESLINT-011: lintClient reads the inline rule configs of every file it lints', async () => { + tree = ratchetTree({ + 'eslint.config.mjs': "export default [{ files: ['**/*.js'], rules: { 'no-empty': 'warn' } }];\n", + 'src/a.js': '/* eslint no-empty: off */\nif (globalThis.x) {}\n', + 'src/b.js': 'if (globalThis.x) {}\n', + }); + const results = await lintClient(tree.root); + const byKey = Object.fromEntries(results.map((r) => [r.filePath.split('\\').join('/').split('/src/')[1], r])); + expect(byKey['a.js'].messages).toEqual([]); + expect(byKey['a.js'].inlineConfigs).toEqual(['1:1 /* eslint no-empty: off */']); + expect(byKey['b.js'].messages.map((m) => m.ruleId)).toEqual(['no-empty']); + expect(byKey['b.js'].inlineConfigs).toEqual([]); + }); });