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([]); + }); });