Skip to content

Commit a659896

Browse files
os-steveclaude
andauthored
fix(scripts): the two ESLint ratchets refuse a population that will not parse (#10123) (#10130)
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. Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja Co-authored-by: Claude <noreply@anthropic.com>
1 parent a38408a commit a659896

3 files changed

Lines changed: 322 additions & 6 deletions

File tree

scripts/check-query-options-erasure-ratchet.mjs

Lines changed: 90 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,14 @@
7171
// licenses that many new erasures, which is how a ratchet stops meaning
7272
// anything.
7373
//
74+
// And it REFUSES to report at all (exit 2) when a file in either population
75+
// does not PARSE. ESLint's Node API returns a parse failure as a message
76+
// carrying no rule id, so it matches neither count above: before #10123 such a
77+
// file contributed zero sites and this gate printed `✓ … holds` and exited 0 —
78+
// a clean verdict on a file it had never read, while `pnpm lint` failed loudly
79+
// on the same input. scripts/eslint-fatal-guard.mjs carries the measurement and
80+
// why a fatal is the measurement failing rather than a finding.
81+
//
7482
// node scripts/check-query-options-erasure-ratchet.mjs [--update] [--self-test]
7583
//
7684
// The counts are produced by running ESLint itself over the real config with
@@ -89,6 +97,7 @@ import eslintConfig, {
8997
QUERY_OPTIONS_TEST_GLOBS,
9098
QUERY_OPTIONS_ANY_MESSAGE,
9199
} from '../eslint.config.mjs';
100+
import { checkGuardAdoption, collectFatalMessages, lintFilesStrict } from './eslint-fatal-guard.mjs';
92101

93102
const __dirname = dirname(fileURLToPath(import.meta.url));
94103
const repoRoot = resolve(__dirname, '..');
@@ -119,7 +128,13 @@ async function measure(drop, targets = [LINT_TARGET]) {
119128
// purpose, so an eslint-disable comment must not shrink a count here either.
120129
allowInlineConfig: false,
121130
});
122-
const results = await eslint.lintFiles(targets);
131+
// Not `eslint.lintFiles`: a parse failure inside the population is the
132+
// measurement failing, not a file with nothing to report, and it matches
133+
// neither count below. The guard names the file and stops (#10123).
134+
const results = await lintFilesStrict(eslint, targets, {
135+
gate: 'check-query-options-erasure-ratchet',
136+
repoRoot,
137+
});
123138
const counts = {};
124139
for (const result of results) {
125140
const hits = result.messages.filter((m) => m.ruleId === QUERY_OPTIONS_RULE_ID).length;
@@ -358,6 +373,76 @@ async function selfTest() {
358373
assert(got === expected, `diffRatchet: ${name} — expected ${expected} error(s), got ${got}`);
359374
}
360375

376+
// ── 3. The fatal-parse guard, in both directions (#10123). ───────────────
377+
//
378+
// ESLint does not throw on a file that will not parse: it returns the failure
379+
// as a message with no rule id, which matches neither count this gate keeps.
380+
// Before the guard such a file contributed zero sites and the gate printed
381+
// `✓ … holds`, so the harm is a QUIET GREEN — a case showing the guard silent
382+
// on today's (parseable) corpus would prove nothing at all. Both directions
383+
// are therefore driven through real ESLint output, and the fixture is a file
384+
// that genuinely does not parse rather than a hand-built message object.
385+
{
386+
const [broken] = await eslint.lintText('export const x = (', {
387+
filePath: 'packages/objectql/src/__selftest_unparseable__.ts',
388+
warnIgnored: false,
389+
});
390+
assert(
391+
(broken?.messages ?? []).some((m) => m.fatal),
392+
'ESLint must report an unparseable file as a fatal message — the premise of the guard',
393+
);
394+
assert(
395+
(broken?.messages ?? []).filter((m) => m.ruleId === QUERY_OPTIONS_RULE_ID).length === 0,
396+
'and that message must match no counted rule, which is exactly why it needs its own check',
397+
);
398+
399+
const fatals = collectFatalMessages([broken], repoRoot);
400+
assert(fatals.length === 1, `the guard must collect the fatal (collected ${fatals.length})`);
401+
assert(
402+
fatals[0]?.file.endsWith('__selftest_unparseable__.ts') && /Parsing error/i.test(fatals[0]?.message ?? ''),
403+
`the collected fatal must name the file and the parse error (got ${JSON.stringify(fatals[0])})`,
404+
);
405+
406+
const [parses] = await eslint.lintText('export const x = 1;', {
407+
filePath: 'packages/objectql/src/__selftest_parses__.ts',
408+
warnIgnored: false,
409+
});
410+
assert(
411+
collectFatalMessages([parses], repoRoot).length === 0,
412+
'a file that parses must produce no fatal — the guard must not fire on a healthy tree',
413+
);
414+
415+
// The call site the gates actually use: it must refuse to hand back results
416+
// for a population it could not measure, and say which file broke.
417+
let reported = null;
418+
const refused = await lintFilesStrict({ lintFiles: async () => [broken] }, [LINT_TARGET], {
419+
gate: 'self-test',
420+
repoRoot,
421+
onFatal: (report) => { reported = report; return 'refused'; },
422+
});
423+
assert(refused === 'refused', 'lintFilesStrict must not return results when a file did not parse');
424+
assert(
425+
(reported ?? '').includes('__selftest_unparseable__.ts') && /Parsing error/i.test(reported ?? ''),
426+
`the failure text must name the file and the parse error (got: ${reported})`,
427+
);
428+
429+
let fired = false;
430+
const passed = await lintFilesStrict({ lintFiles: async () => [parses] }, [LINT_TARGET], {
431+
gate: 'self-test',
432+
repoRoot,
433+
onFatal: () => { fired = true; },
434+
});
435+
assert(
436+
!fired && Array.isArray(passed) && passed.length === 1,
437+
'lintFilesStrict must pass the results through when every file parsed',
438+
);
439+
440+
// A guard imported once is not a guard still called. This is also the only
441+
// wired coverage of the OTHER gate's call site: `pnpm check:slot-lookup`
442+
// has no --self-test hook, and CI runs this one before the gate itself.
443+
for (const problem of checkGuardAdoption(repoRoot)) assert(false, problem);
444+
}
445+
361446
// A missing config block must ABORT, never report clean.
362447
assert(eslintConfig.some(carriesRule), 'the config must carry the query-options rule');
363448

@@ -368,7 +453,8 @@ async function selfTest() {
368453
}
369454
console.log(
370455
`✓ self-test: ${reports.length} reporting shape(s), ${silent.length} silent counterpart(s), ` +
371-
`grandfathering + test-glob channels proved in both directions, ${cases.length} ratchet case(s).`,
456+
`grandfathering + test-glob channels proved in both directions, ${cases.length} ratchet case(s), ` +
457+
`fatal-parse guard proved both ways over real ESLint output, both gates still routed through it.`,
372458
);
373459
}
374460

@@ -450,8 +536,8 @@ if (errors.length > 0) {
450536

451537
console.log(
452538
`✓ query-options-erasure ratchet holds: ${sum(nonTest)} unswept non-test site(s) in ` +
453-
`${Object.keys(nonTest).length} file(s), none new. Every other non-test file under ` +
454-
`packages/ is covered by \`pnpm lint\`.`,
539+
`${Object.keys(nonTest).length} file(s), none new, and every file measured parsed. ` +
540+
`Every other non-test file under packages/ is covered by \`pnpm lint\`.`,
455541
);
456542
console.log(
457543
` test surface: ${testSites} site(s) in ${Object.keys(testOnly).length} file(s) — at the ` +

scripts/check-slot-lookup-ratchet.mjs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@
1919
// • a baselined file's count DECREASED or the file is clean/gone (progress!)
2020
// — run with --update to ratchet the baseline down and commit it.
2121
//
22+
// And it REFUSES to report at all (exit 2) when a file in the population does
23+
// not PARSE. ESLint's Node API returns a parse failure as a message carrying no
24+
// rule id, which matches nothing this script counts, so before #10123 such a
25+
// file contributed zero sites and this gate printed `✓ … holds` and exited 0 —
26+
// a clean verdict on a file it had never read, while `pnpm lint` failed loudly
27+
// on the same input. scripts/eslint-fatal-guard.mjs carries the measurement and
28+
// why a fatal is the measurement failing rather than a finding.
29+
//
2230
// node scripts/check-slot-lookup-ratchet.mjs [--update]
2331
//
2432
// The counts are produced by running ESLint itself with the baseline's
@@ -37,6 +45,7 @@ import { fileURLToPath } from 'node:url';
3745
import { ESLint } from 'eslint';
3846

3947
import eslintConfig, { SLOT_LOOKUP_ANY_MESSAGE } from '../eslint.config.mjs';
48+
import { lintFilesStrict } from './eslint-fatal-guard.mjs';
4049

4150
const __dirname = dirname(fileURLToPath(import.meta.url));
4251
const repoRoot = resolve(__dirname, '..');
@@ -160,7 +169,13 @@ const eslint = new ESLint({
160169
allowInlineConfig: false,
161170
});
162171

163-
const results = await eslint.lintFiles([LINT_TARGET]);
172+
// Not `eslint.lintFiles`: a parse failure inside the population is the
173+
// measurement failing, not a file with nothing to report, and it matches none
174+
// of the filters below. The guard names the file and stops (#10123).
175+
const results = await lintFilesStrict(eslint, [LINT_TARGET], {
176+
gate: 'check-slot-lookup-ratchet',
177+
repoRoot,
178+
});
164179

165180
const current = {};
166181
for (const result of results) {
@@ -259,7 +274,8 @@ if (errors.length > 0) {
259274

260275
console.log(
261276
`✓ slot-lookup ratchet holds: ${totalSites} unswept site(s) in ${totalFiles} file(s), ` +
262-
`none new. Every other file under packages/ is covered by \`pnpm lint\`.`,
277+
`none new, and every file in the population parsed. Every other file under ` +
278+
`packages/ is covered by \`pnpm lint\`.`,
263279
);
264280
console.log(
265281
monotonicity

scripts/eslint-fatal-guard.mjs

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
// eslint-fatal-guard — a file that will not PARSE must never score clean.
2+
//
3+
// The two ESLint-driven ratchets in this directory
4+
// (check-slot-lookup-ratchet.mjs, check-query-options-erasure-ratchet.mjs)
5+
// measure by running ESLint over `packages/**` through its Node API and
6+
// COUNTING the messages that match their own rule. The counting step is where
7+
// an unparseable file disappears.
8+
//
9+
// ── MEASURED (#10123), re-derived on this tree at 5262124388 ────────────────
10+
//
11+
// A `packages/**` file holding one syntax error, linted through the Node API
12+
// with this repo's real config:
13+
//
14+
// lintFiles returned normally (did NOT throw)
15+
// messages: [{"ruleId":null,"fatal":true,"severity":2,
16+
// "message":"Parsing error: Expression expected.","line":2,"column":32}]
17+
// errorCount=1 fatalErrorCount=1
18+
//
19+
// ESLint does not throw on a parse failure — it returns it as an ordinary
20+
// message with NO rule id and `fatal: true`. Neither ratchet's filter can match
21+
// it (one compares `m.message` to its rule's text, the other compares
22+
// `m.ruleId` to its rule's id), so the file contributed ZERO sites and both
23+
// gates printed `✓ … holds` and exited 0 — output byte-identical to a file that
24+
// was measured and is clean.
25+
//
26+
// The same file, through the root `lint` script's spelling
27+
// (`eslint <file> --no-inline-config`), exits 1:
28+
//
29+
// 2:32 error Parsing error: Expression expected
30+
// ✖ 1 problem (1 error, 0 warnings)
31+
//
32+
// So before this guard the two gates and `pnpm lint` DISAGREED about whether an
33+
// unparseable file is a failure — and they disagreed in the direction that
34+
// matters, because each ratchet's own docblock argues it exists precisely
35+
// BECAUSE a green `pnpm lint` proves nothing for the files it covers. A gate
36+
// whose stated rationale is "I see what lint cannot" must not be the one that
37+
// goes quiet first.
38+
//
39+
// ── A fatal is not a finding. It is the measurement failing ─────────────────
40+
//
41+
// This is why the guard aborts rather than counting a fatal as a site: a parse
42+
// failure says the file was never read for sites at all, so neither "0 sites"
43+
// nor "N sites" is a fact about it. Reporting it as a rule hit would be the
44+
// same lie with a different sign.
45+
//
46+
// EXIT CODE 2, deliberately. Both gates already reserve 2 for "refusing to
47+
// report clean when the thing being measured is not what this gate thinks it
48+
// is" (a renamed rule, a rescoped population) and 1 for "the ratchet moved".
49+
// A parse failure belongs to the first family — nothing moved, the measurement
50+
// did not happen — and the exit code is the only part of that distinction a CI
51+
// log preserves for a reader who sees only the step's status.
52+
//
53+
// ── Why `m.fatal`, and not `ruleId === null` ───────────────────────────────
54+
//
55+
// A null rule id alone is not the signal: ESLint also emits `ruleId: null`
56+
// warnings for other reasons (an explicitly-linted file that config ignores,
57+
// for one), and counting those would make the guard fire on a healthy tree,
58+
// which is how a true gate gets weakened back out. `fatal` is the parse-failure
59+
// flag itself, cross-checked here against the per-result `fatalErrorCount`
60+
// summary so a future ESLint that moves the flag cannot make this silent again
61+
// — the failure mode this whole file exists to close.
62+
//
63+
// ── Why a shared module, and how adoption is kept honest ───────────────────
64+
//
65+
// Two copies of a guard drift, and a drifted copy is invisible: the gate that
66+
// lost the check keeps printing the same green line. So the check lives once,
67+
// the gates route their run through `lintFilesStrict()` instead of calling
68+
// `eslint.lintFiles()`, and `checkGuardAdoption()` asserts both of those facts
69+
// about every gate in GUARDED_GATES by reading their source. That assertion is
70+
// driven by check-query-options-erasure-ratchet.mjs's `--self-test`, which CI
71+
// runs ahead of the gate itself (`pnpm check:query-options-erasure`);
72+
// `pnpm check:slot-lookup` has no self-test hook of its own, so the coverage of
73+
// ITS call site is the source assertion, not a second wired self-test.
74+
import { readFileSync } from 'node:fs';
75+
import { relative, resolve } from 'node:path';
76+
import process from 'node:process';
77+
78+
/** "This gate could not measure", as distinct from 1 = "the ratchet moved". */
79+
export const FATAL_GUARD_EXIT_CODE = 2;
80+
81+
/**
82+
* The gates that drive ESLint over a population and count what comes back.
83+
* Every one of them must route its run through `lintFilesStrict()`.
84+
*/
85+
export const GUARDED_GATES = [
86+
'scripts/check-slot-lookup-ratchet.mjs',
87+
'scripts/check-query-options-erasure-ratchet.mjs',
88+
];
89+
90+
/**
91+
* Every parse failure in an ESLint result set, flattened and repo-relative.
92+
*
93+
* @param {Array<{filePath?: string, messages?: Array<object>, fatalErrorCount?: number}>} results
94+
* @param {string} [repoRoot] absolute root to make paths relative to
95+
* @returns {Array<{file: string, line: number, column: number, message: string}>}
96+
*/
97+
export function collectFatalMessages(results, repoRoot) {
98+
const fatals = [];
99+
for (const result of results ?? []) {
100+
const path = result?.filePath;
101+
const file = !path ? '(unknown file)'
102+
: repoRoot ? relative(repoRoot, path).replace(/\\/g, '/')
103+
: path;
104+
const messages = (result?.messages ?? []).filter((m) => m?.fatal);
105+
for (const m of messages) {
106+
fatals.push({
107+
file,
108+
line: m.line ?? 0,
109+
column: m.column ?? 0,
110+
message: m.message ?? '(no message)',
111+
});
112+
}
113+
// The cross-check. `fatalErrorCount` is ESLint's own summary of the same
114+
// fact; if it ever disagrees with the per-message flag, the disagreement is
115+
// reported rather than resolved in favour of silence.
116+
if (messages.length === 0 && (result?.fatalErrorCount ?? 0) > 0) {
117+
fatals.push({
118+
file,
119+
line: 0,
120+
column: 0,
121+
message:
122+
`ESLint reported fatalErrorCount=${result.fatalErrorCount} but no message ` +
123+
'carried the fatal flag. Treated as a parse failure: this gate does not ' +
124+
'report clean for a file it may not have read.',
125+
});
126+
}
127+
}
128+
return fatals;
129+
}
130+
131+
/**
132+
* The author-facing failure. Names every file, where it broke and why.
133+
*
134+
* @param {string} gate the gate's name, for the first line
135+
* @param {ReturnType<typeof collectFatalMessages>} fatals
136+
* @returns {string}
137+
*/
138+
export function formatFatalReport(gate, fatals) {
139+
const lines = [
140+
`✗ ${gate}: ${fatals.length} parse failure(s) inside the population this gate measures:`,
141+
'',
142+
];
143+
for (const f of fatals) lines.push(` • ${f.file}:${f.line}:${f.column}${f.message}`);
144+
lines.push(
145+
'',
146+
'ESLint returns a parse failure as a message with no rule id, so it matches no',
147+
'rule this gate counts. A file that does not parse was never read for sites at',
148+
'all: counting it as zero would report it clean without measuring it, which is',
149+
'the one thing this gate exists to prevent. Nothing was counted this run.',
150+
'',
151+
'Fix the parse error (regenerate the file if it is generated), then run this',
152+
'gate again. `pnpm lint` fails on the same file with the same error.',
153+
);
154+
return lines.join('\n');
155+
}
156+
157+
/**
158+
* `eslint.lintFiles()`, with a parse failure anywhere in the results treated as
159+
* the measurement failing rather than as a file with nothing to report.
160+
*
161+
* @param {{lintFiles: (targets: string[]) => Promise<object[]>}} eslint
162+
* @param {string[]} targets
163+
* @param {{gate: string, repoRoot?: string, onFatal?: (report: string, fatals: object[]) => never|unknown}} options
164+
* @returns {Promise<object[]>} the results, when every file parsed
165+
*/
166+
export async function lintFilesStrict(eslint, targets, { gate, repoRoot, onFatal = exitOnFatal } = {}) {
167+
const results = await eslint.lintFiles(targets);
168+
const fatals = collectFatalMessages(results, repoRoot);
169+
if (fatals.length > 0) return onFatal(formatFatalReport(gate ?? 'eslint-fatal-guard', fatals), fatals);
170+
return results;
171+
}
172+
173+
/** The default handler: print the report and stop. Never returns. */
174+
function exitOnFatal(report) {
175+
console.error(report);
176+
process.exit(FATAL_GUARD_EXIT_CODE);
177+
}
178+
179+
/**
180+
* Assert every gate in GUARDED_GATES still routes through this module.
181+
*
182+
* Read from the gates' own source, because the alternative is trusting that a
183+
* guard imported once is a guard still called — and a gate that quietly went
184+
* back to `eslint.lintFiles()` looks, from its output, exactly like one that
185+
* never lost the check.
186+
*
187+
* @param {string} repoRoot
188+
* @returns {string[]} problems, empty when every gate is still guarded
189+
*/
190+
export function checkGuardAdoption(repoRoot) {
191+
const problems = [];
192+
for (const gate of GUARDED_GATES) {
193+
let src;
194+
try {
195+
src = readFileSync(resolve(repoRoot, gate), 'utf8');
196+
} catch {
197+
problems.push(`${gate}: named by the fatal-parse guard but unreadable — renamed or removed?`);
198+
continue;
199+
}
200+
if (!/eslint-fatal-guard\.mjs/.test(src)) {
201+
problems.push(
202+
`${gate}: does not import scripts/eslint-fatal-guard.mjs. A gate that counts ` +
203+
'ESLint messages scores an unparseable file as clean without it (#10123).',
204+
);
205+
}
206+
if (/\.lintFiles\s*\(/.test(src)) {
207+
problems.push(
208+
`${gate}: calls \`.lintFiles(\` directly, so a parse failure in its population ` +
209+
'is discarded as a message matching no rule. Call lintFilesStrict() instead.',
210+
);
211+
}
212+
}
213+
return problems;
214+
}

0 commit comments

Comments
 (0)