fix(scripts): the two ESLint ratchets refuse a population that will not parse (#10123)

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 ` +

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

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 <file> --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<object>, 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<typeof collectFatalMessages>} 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<object[]>}} eslint
+ * @param {string[]} targets
+ * @param {{gate: string, repoRoot?: string, onFatal?: (report: string, fatals: object[]) => never|unknown}} options
+ * @returns {Promise<object[]>} 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;
+}