From 72038565f930a725dcb0d6bcc61c08a631567e94 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 07:31:30 +0000 Subject: [PATCH] fix(scripts): the two ESLint ratchets refuse a population that will not parse (#10123) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both ratchets drive ESLint through its Node API and count the messages that match their own rule. ESLint does not throw on a parse failure — it returns it as a message with no rule id and `fatal: true` — so an unparseable file matched neither filter, contributed zero sites, and the gate printed `✓ … holds` and exited 0: a clean verdict on a file it had never read, while `pnpm lint` failed loudly on the same input. The check lives once, in scripts/eslint-fatal-guard.mjs, and both gates route their run through `lintFilesStrict()` instead of `eslint.lintFiles()`. A parse failure now names the file, the position and the parser's message, and exits 2 — the code both gates already reserve for "refusing to report clean", as distinct from 1 = "the ratchet moved". Site counts are unchanged. The query-options `--self-test` (run by CI ahead of the gate) proves the guard in both directions over real ESLint output, and asserts from source that both gates still route through it — `pnpm check:slot-lookup` has no self-test hook of its own, so that assertion is the wired coverage of its call site. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja --- .../check-query-options-erasure-ratchet.mjs | 94 +++++++- scripts/check-slot-lookup-ratchet.mjs | 20 +- scripts/eslint-fatal-guard.mjs | 214 ++++++++++++++++++ 3 files changed, 322 insertions(+), 6 deletions(-) create mode 100644 scripts/eslint-fatal-guard.mjs diff --git a/scripts/check-query-options-erasure-ratchet.mjs b/scripts/check-query-options-erasure-ratchet.mjs index 5bf66d3341..5b0c85f5d9 100644 --- a/scripts/check-query-options-erasure-ratchet.mjs +++ b/scripts/check-query-options-erasure-ratchet.mjs @@ -71,6 +71,14 @@ // licenses that many new erasures, which is how a ratchet stops meaning // anything. // +// And it REFUSES to report at all (exit 2) when a file in either population +// does not PARSE. ESLint's Node API returns a parse failure as a message +// carrying no rule id, so it matches neither count above: before #10123 such a +// file contributed zero sites and this gate printed `✓ … holds` and exited 0 — +// a clean verdict on a file it had never read, while `pnpm lint` failed loudly +// on the same input. scripts/eslint-fatal-guard.mjs carries the measurement and +// why a fatal is the measurement failing rather than a finding. +// // node scripts/check-query-options-erasure-ratchet.mjs [--update] [--self-test] // // The counts are produced by running ESLint itself over the real config with @@ -89,6 +97,7 @@ import eslintConfig, { QUERY_OPTIONS_TEST_GLOBS, QUERY_OPTIONS_ANY_MESSAGE, } from '../eslint.config.mjs'; +import { checkGuardAdoption, collectFatalMessages, lintFilesStrict } from './eslint-fatal-guard.mjs'; const __dirname = dirname(fileURLToPath(import.meta.url)); const repoRoot = resolve(__dirname, '..'); @@ -119,7 +128,13 @@ async function measure(drop, targets = [LINT_TARGET]) { // purpose, so an eslint-disable comment must not shrink a count here either. allowInlineConfig: false, }); - const results = await eslint.lintFiles(targets); + // Not `eslint.lintFiles`: a parse failure inside the population is the + // measurement failing, not a file with nothing to report, and it matches + // neither count below. The guard names the file and stops (#10123). + const results = await lintFilesStrict(eslint, targets, { + gate: 'check-query-options-erasure-ratchet', + repoRoot, + }); const counts = {}; for (const result of results) { const hits = result.messages.filter((m) => m.ruleId === QUERY_OPTIONS_RULE_ID).length; @@ -358,6 +373,76 @@ async function selfTest() { assert(got === expected, `diffRatchet: ${name} — expected ${expected} error(s), got ${got}`); } + // ── 3. The fatal-parse guard, in both directions (#10123). ─────────────── + // + // ESLint does not throw on a file that will not parse: it returns the failure + // as a message with no rule id, which matches neither count this gate keeps. + // Before the guard such a file contributed zero sites and the gate printed + // `✓ … holds`, so the harm is a QUIET GREEN — a case showing the guard silent + // on today's (parseable) corpus would prove nothing at all. Both directions + // are therefore driven through real ESLint output, and the fixture is a file + // that genuinely does not parse rather than a hand-built message object. + { + const [broken] = await eslint.lintText('export const x = (', { + filePath: 'packages/objectql/src/__selftest_unparseable__.ts', + warnIgnored: false, + }); + assert( + (broken?.messages ?? []).some((m) => m.fatal), + 'ESLint must report an unparseable file as a fatal message — the premise of the guard', + ); + assert( + (broken?.messages ?? []).filter((m) => m.ruleId === QUERY_OPTIONS_RULE_ID).length === 0, + 'and that message must match no counted rule, which is exactly why it needs its own check', + ); + + const fatals = collectFatalMessages([broken], repoRoot); + assert(fatals.length === 1, `the guard must collect the fatal (collected ${fatals.length})`); + assert( + fatals[0]?.file.endsWith('__selftest_unparseable__.ts') && /Parsing error/i.test(fatals[0]?.message ?? ''), + `the collected fatal must name the file and the parse error (got ${JSON.stringify(fatals[0])})`, + ); + + const [parses] = await eslint.lintText('export const x = 1;', { + filePath: 'packages/objectql/src/__selftest_parses__.ts', + warnIgnored: false, + }); + assert( + collectFatalMessages([parses], repoRoot).length === 0, + 'a file that parses must produce no fatal — the guard must not fire on a healthy tree', + ); + + // The call site the gates actually use: it must refuse to hand back results + // for a population it could not measure, and say which file broke. + let reported = null; + const refused = await lintFilesStrict({ lintFiles: async () => [broken] }, [LINT_TARGET], { + gate: 'self-test', + repoRoot, + onFatal: (report) => { reported = report; return 'refused'; }, + }); + assert(refused === 'refused', 'lintFilesStrict must not return results when a file did not parse'); + assert( + (reported ?? '').includes('__selftest_unparseable__.ts') && /Parsing error/i.test(reported ?? ''), + `the failure text must name the file and the parse error (got: ${reported})`, + ); + + let fired = false; + const passed = await lintFilesStrict({ lintFiles: async () => [parses] }, [LINT_TARGET], { + gate: 'self-test', + repoRoot, + onFatal: () => { fired = true; }, + }); + assert( + !fired && Array.isArray(passed) && passed.length === 1, + 'lintFilesStrict must pass the results through when every file parsed', + ); + + // A guard imported once is not a guard still called. This is also the only + // wired coverage of the OTHER gate's call site: `pnpm check:slot-lookup` + // has no --self-test hook, and CI runs this one before the gate itself. + for (const problem of checkGuardAdoption(repoRoot)) assert(false, problem); + } + // A missing config block must ABORT, never report clean. assert(eslintConfig.some(carriesRule), 'the config must carry the query-options rule'); @@ -368,7 +453,8 @@ async function selfTest() { } console.log( `✓ self-test: ${reports.length} reporting shape(s), ${silent.length} silent counterpart(s), ` + - `grandfathering + test-glob channels proved in both directions, ${cases.length} ratchet case(s).`, + `grandfathering + test-glob channels proved in both directions, ${cases.length} ratchet case(s), ` + + `fatal-parse guard proved both ways over real ESLint output, both gates still routed through it.`, ); } @@ -450,8 +536,8 @@ if (errors.length > 0) { console.log( `✓ query-options-erasure ratchet holds: ${sum(nonTest)} unswept non-test site(s) in ` + - `${Object.keys(nonTest).length} file(s), none new. Every other non-test file under ` + - `packages/ is covered by \`pnpm lint\`.`, + `${Object.keys(nonTest).length} file(s), none new, and every file measured parsed. ` + + `Every other non-test file under packages/ is covered by \`pnpm lint\`.`, ); console.log( ` test surface: ${testSites} site(s) in ${Object.keys(testOnly).length} file(s) — at the ` + diff --git a/scripts/check-slot-lookup-ratchet.mjs b/scripts/check-slot-lookup-ratchet.mjs index 14f36fca9d..59b424ab20 100644 --- a/scripts/check-slot-lookup-ratchet.mjs +++ b/scripts/check-slot-lookup-ratchet.mjs @@ -19,6 +19,14 @@ // • a baselined file's count DECREASED or the file is clean/gone (progress!) // — run with --update to ratchet the baseline down and commit it. // +// And it REFUSES to report at all (exit 2) when a file in the population does +// not PARSE. ESLint's Node API returns a parse failure as a message carrying no +// rule id, which matches nothing this script counts, so before #10123 such a +// file contributed zero sites and this gate printed `✓ … holds` and exited 0 — +// a clean verdict on a file it had never read, while `pnpm lint` failed loudly +// on the same input. scripts/eslint-fatal-guard.mjs carries the measurement and +// why a fatal is the measurement failing rather than a finding. +// // node scripts/check-slot-lookup-ratchet.mjs [--update] // // The counts are produced by running ESLint itself with the baseline's @@ -37,6 +45,7 @@ import { fileURLToPath } from 'node:url'; import { ESLint } from 'eslint'; import eslintConfig, { SLOT_LOOKUP_ANY_MESSAGE } from '../eslint.config.mjs'; +import { lintFilesStrict } from './eslint-fatal-guard.mjs'; const __dirname = dirname(fileURLToPath(import.meta.url)); const repoRoot = resolve(__dirname, '..'); @@ -160,7 +169,13 @@ const eslint = new ESLint({ allowInlineConfig: false, }); -const results = await eslint.lintFiles([LINT_TARGET]); +// Not `eslint.lintFiles`: a parse failure inside the population is the +// measurement failing, not a file with nothing to report, and it matches none +// of the filters below. The guard names the file and stops (#10123). +const results = await lintFilesStrict(eslint, [LINT_TARGET], { + gate: 'check-slot-lookup-ratchet', + repoRoot, +}); const current = {}; for (const result of results) { @@ -259,7 +274,8 @@ if (errors.length > 0) { console.log( `✓ slot-lookup ratchet holds: ${totalSites} unswept site(s) in ${totalFiles} file(s), ` + - `none new. Every other file under packages/ is covered by \`pnpm lint\`.`, + `none new, and every file in the population parsed. Every other file under ` + + `packages/ is covered by \`pnpm lint\`.`, ); console.log( monotonicity diff --git a/scripts/eslint-fatal-guard.mjs b/scripts/eslint-fatal-guard.mjs new file mode 100644 index 0000000000..09536ca56d --- /dev/null +++ b/scripts/eslint-fatal-guard.mjs @@ -0,0 +1,214 @@ +// eslint-fatal-guard — a file that will not PARSE must never score clean. +// +// The two ESLint-driven ratchets in this directory +// (check-slot-lookup-ratchet.mjs, check-query-options-erasure-ratchet.mjs) +// measure by running ESLint over `packages/**` through its Node API and +// COUNTING the messages that match their own rule. The counting step is where +// an unparseable file disappears. +// +// ── MEASURED (#10123), re-derived on this tree at 5262124388 ──────────────── +// +// A `packages/**` file holding one syntax error, linted through the Node API +// with this repo's real config: +// +// lintFiles returned normally (did NOT throw) +// messages: [{"ruleId":null,"fatal":true,"severity":2, +// "message":"Parsing error: Expression expected.","line":2,"column":32}] +// errorCount=1 fatalErrorCount=1 +// +// ESLint does not throw on a parse failure — it returns it as an ordinary +// message with NO rule id and `fatal: true`. Neither ratchet's filter can match +// it (one compares `m.message` to its rule's text, the other compares +// `m.ruleId` to its rule's id), so the file contributed ZERO sites and both +// gates printed `✓ … holds` and exited 0 — output byte-identical to a file that +// was measured and is clean. +// +// The same file, through the root `lint` script's spelling +// (`eslint --no-inline-config`), exits 1: +// +// 2:32 error Parsing error: Expression expected +// ✖ 1 problem (1 error, 0 warnings) +// +// So before this guard the two gates and `pnpm lint` DISAGREED about whether an +// unparseable file is a failure — and they disagreed in the direction that +// matters, because each ratchet's own docblock argues it exists precisely +// BECAUSE a green `pnpm lint` proves nothing for the files it covers. A gate +// whose stated rationale is "I see what lint cannot" must not be the one that +// goes quiet first. +// +// ── A fatal is not a finding. It is the measurement failing ───────────────── +// +// This is why the guard aborts rather than counting a fatal as a site: a parse +// failure says the file was never read for sites at all, so neither "0 sites" +// nor "N sites" is a fact about it. Reporting it as a rule hit would be the +// same lie with a different sign. +// +// EXIT CODE 2, deliberately. Both gates already reserve 2 for "refusing to +// report clean when the thing being measured is not what this gate thinks it +// is" (a renamed rule, a rescoped population) and 1 for "the ratchet moved". +// A parse failure belongs to the first family — nothing moved, the measurement +// did not happen — and the exit code is the only part of that distinction a CI +// log preserves for a reader who sees only the step's status. +// +// ── Why `m.fatal`, and not `ruleId === null` ─────────────────────────────── +// +// A null rule id alone is not the signal: ESLint also emits `ruleId: null` +// warnings for other reasons (an explicitly-linted file that config ignores, +// for one), and counting those would make the guard fire on a healthy tree, +// which is how a true gate gets weakened back out. `fatal` is the parse-failure +// flag itself, cross-checked here against the per-result `fatalErrorCount` +// summary so a future ESLint that moves the flag cannot make this silent again +// — the failure mode this whole file exists to close. +// +// ── Why a shared module, and how adoption is kept honest ─────────────────── +// +// Two copies of a guard drift, and a drifted copy is invisible: the gate that +// lost the check keeps printing the same green line. So the check lives once, +// the gates route their run through `lintFilesStrict()` instead of calling +// `eslint.lintFiles()`, and `checkGuardAdoption()` asserts both of those facts +// about every gate in GUARDED_GATES by reading their source. That assertion is +// driven by check-query-options-erasure-ratchet.mjs's `--self-test`, which CI +// runs ahead of the gate itself (`pnpm check:query-options-erasure`); +// `pnpm check:slot-lookup` has no self-test hook of its own, so the coverage of +// ITS call site is the source assertion, not a second wired self-test. +import { readFileSync } from 'node:fs'; +import { relative, resolve } from 'node:path'; +import process from 'node:process'; + +/** "This gate could not measure", as distinct from 1 = "the ratchet moved". */ +export const FATAL_GUARD_EXIT_CODE = 2; + +/** + * The gates that drive ESLint over a population and count what comes back. + * Every one of them must route its run through `lintFilesStrict()`. + */ +export const GUARDED_GATES = [ + 'scripts/check-slot-lookup-ratchet.mjs', + 'scripts/check-query-options-erasure-ratchet.mjs', +]; + +/** + * Every parse failure in an ESLint result set, flattened and repo-relative. + * + * @param {Array<{filePath?: string, messages?: Array, fatalErrorCount?: number}>} results + * @param {string} [repoRoot] absolute root to make paths relative to + * @returns {Array<{file: string, line: number, column: number, message: string}>} + */ +export function collectFatalMessages(results, repoRoot) { + const fatals = []; + for (const result of results ?? []) { + const path = result?.filePath; + const file = !path ? '(unknown file)' + : repoRoot ? relative(repoRoot, path).replace(/\\/g, '/') + : path; + const messages = (result?.messages ?? []).filter((m) => m?.fatal); + for (const m of messages) { + fatals.push({ + file, + line: m.line ?? 0, + column: m.column ?? 0, + message: m.message ?? '(no message)', + }); + } + // The cross-check. `fatalErrorCount` is ESLint's own summary of the same + // fact; if it ever disagrees with the per-message flag, the disagreement is + // reported rather than resolved in favour of silence. + if (messages.length === 0 && (result?.fatalErrorCount ?? 0) > 0) { + fatals.push({ + file, + line: 0, + column: 0, + message: + `ESLint reported fatalErrorCount=${result.fatalErrorCount} but no message ` + + 'carried the fatal flag. Treated as a parse failure: this gate does not ' + + 'report clean for a file it may not have read.', + }); + } + } + return fatals; +} + +/** + * The author-facing failure. Names every file, where it broke and why. + * + * @param {string} gate the gate's name, for the first line + * @param {ReturnType} fatals + * @returns {string} + */ +export function formatFatalReport(gate, fatals) { + const lines = [ + `✗ ${gate}: ${fatals.length} parse failure(s) inside the population this gate measures:`, + '', + ]; + for (const f of fatals) lines.push(` • ${f.file}:${f.line}:${f.column} — ${f.message}`); + lines.push( + '', + 'ESLint returns a parse failure as a message with no rule id, so it matches no', + 'rule this gate counts. A file that does not parse was never read for sites at', + 'all: counting it as zero would report it clean without measuring it, which is', + 'the one thing this gate exists to prevent. Nothing was counted this run.', + '', + 'Fix the parse error (regenerate the file if it is generated), then run this', + 'gate again. `pnpm lint` fails on the same file with the same error.', + ); + return lines.join('\n'); +} + +/** + * `eslint.lintFiles()`, with a parse failure anywhere in the results treated as + * the measurement failing rather than as a file with nothing to report. + * + * @param {{lintFiles: (targets: string[]) => Promise}} eslint + * @param {string[]} targets + * @param {{gate: string, repoRoot?: string, onFatal?: (report: string, fatals: object[]) => never|unknown}} options + * @returns {Promise} the results, when every file parsed + */ +export async function lintFilesStrict(eslint, targets, { gate, repoRoot, onFatal = exitOnFatal } = {}) { + const results = await eslint.lintFiles(targets); + const fatals = collectFatalMessages(results, repoRoot); + if (fatals.length > 0) return onFatal(formatFatalReport(gate ?? 'eslint-fatal-guard', fatals), fatals); + return results; +} + +/** The default handler: print the report and stop. Never returns. */ +function exitOnFatal(report) { + console.error(report); + process.exit(FATAL_GUARD_EXIT_CODE); +} + +/** + * Assert every gate in GUARDED_GATES still routes through this module. + * + * Read from the gates' own source, because the alternative is trusting that a + * guard imported once is a guard still called — and a gate that quietly went + * back to `eslint.lintFiles()` looks, from its output, exactly like one that + * never lost the check. + * + * @param {string} repoRoot + * @returns {string[]} problems, empty when every gate is still guarded + */ +export function checkGuardAdoption(repoRoot) { + const problems = []; + for (const gate of GUARDED_GATES) { + let src; + try { + src = readFileSync(resolve(repoRoot, gate), 'utf8'); + } catch { + problems.push(`${gate}: named by the fatal-parse guard but unreadable — renamed or removed?`); + continue; + } + if (!/eslint-fatal-guard\.mjs/.test(src)) { + problems.push( + `${gate}: does not import scripts/eslint-fatal-guard.mjs. A gate that counts ` + + 'ESLint messages scores an unparseable file as clean without it (#10123).', + ); + } + if (/\.lintFiles\s*\(/.test(src)) { + problems.push( + `${gate}: calls \`.lintFiles(\` directly, so a parse failure in its population ` + + 'is discarded as a message matching no rule. Call lintFilesStrict() instead.', + ); + } + } + return problems; +}