From 46dad29f407b35b29b1429791c52ac372c4aaa3f Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Fri, 21 Aug 2026 06:34:18 +0000 Subject: [PATCH] fix(scripts): guard the lintText half of the fatal-parse adoption check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `checkGuardAdoption()` asked whether `lintFilesStrict(` appears. A gate that KEPT one guarded call and measured a second population through `eslint.lintText()` answered yes to all three of its questions while that second population went entirely unguarded — ESLint returns a parse failure as a message with no rule id either way, so the count silently drops it. Measured through the checker as #10598 left it: import + `eslint.lintText(...)`, no strict call -> 1 problem (not armed) import + strict call + `eslint.lintText(...)` -> 0 problems <- the hole so the lintText-ONLY shape was already caught by #10598's positive assertion; the MIXED shape was not. And it was not hypothetical: this gate's own self-test counted through a bare `lintText()`. The `argument 1, object literal` report fixture with its closing paren removed gave hits()=0 with fatalErrorCount=1 — and hits()===0 is exactly what the ten `silent` cases assert, so a fixture that stopped parsing read as proof the rule is quiet. A blanket `.lintText(` ban was not available: the gate legitimately lints text to establish what raw ESLint does with a file that will not parse, which is ground truth the guard is built on and would be circular through the guard. Source text cannot tell that call from a measurement — which result gets COUNTED is data flow. So the check does not guess. It bans the BARE spelling and the gate declares which kind each call is: `lintTextStrict()` when the result is counted, `lintTextUnguarded({ why })` when it is not. - `lintTextStrict()` — the lintText twin of `lintFilesStrict()`, proved both ways at runtime, with the guard's own options kept out of what ESLint sees. - `lintTextUnguarded({ why })` — behaviour: none. It exists to be typed, and throws on an undeclared call so the escape hatch cannot be a rubber stamp. - The armed test now accepts EITHER strict entry point: a gate whose whole population is text never calls `lintFilesStrict(`, and reporting it unguarded would be a false positive of this change's own making. - 5 new adoption fixtures, including the reproduction and the two negative controls. FIXTURE_COUNT is now `+`-spelled for the same reason #10598's fixtures are: `stripComments` keeps string literals, so a contiguous `.lintText(` in this file's own fixtures would report this gate as unguarded. Fixes #10599 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt --- .../check-query-options-erasure-ratchet.mjs | 130 ++++++++++++++++-- scripts/eslint-fatal-guard.mjs | 125 ++++++++++++++++- 2 files changed, 244 insertions(+), 11 deletions(-) diff --git a/scripts/check-query-options-erasure-ratchet.mjs b/scripts/check-query-options-erasure-ratchet.mjs index 0242f50abd..5210d3a885 100644 --- a/scripts/check-query-options-erasure-ratchet.mjs +++ b/scripts/check-query-options-erasure-ratchet.mjs @@ -102,6 +102,8 @@ import { collectFatalMessages, guardAdoptionProblems, lintFilesStrict, + lintTextStrict, + lintTextUnguarded, } from './eslint-fatal-guard.mjs'; import { HEADROOM_CANARY_FILE, @@ -288,12 +290,16 @@ function baselineKeysAddedSinceMergeBase(baselineKeys) { const FIXTURE_CALL_STRICT = 'const results = await lintFilesStrict' + '(eslint, [TARGET], { gate: G });'; const FIXTURE_CALL_RAW = 'const results = await eslint.lintFiles' + '([TARGET]);'; const FIXTURE_IMPORT = "import { lintFilesStrict } from './eslint-fatal-guard.mjs';"; +const FIXTURE_IMPORT_TEXT = "import { lintTextStrict } from './eslint-fatal-guard.mjs';"; const FIXTURE_PROSE = '// on the same input. scripts/eslint-fatal-guard.mjs carries the measurement and'; -const FIXTURE_COUNT = 'const sites = (await eslint.lintText(code)).messages.filter(matches).length;'; +const FIXTURE_COUNT = 'const sites = (await eslint.lintText' + '(code)).messages.filter(matches).length;'; +const FIXTURE_CALL_TEXT_STRICT = 'const [r] = await lintTextStrict' + '(eslint, code, { gate: G });'; +const FIXTURE_CALL_TEXT_DECLARED = "const [r] = await lintTextUnguarded" + "(eslint, code, { why: 'ground truth' });"; const NO_IMPORT = 'does not import scripts/eslint-fatal-guard.mjs'; const NOT_ARMED = 'Importing the guard does not arm it'; const RAW_CALL = 'directly, so a parse failure in its population'; +const BARE_TEXT = 'A counted lintText result discards a parse failure'; /** * The adoption check in both directions, over sources written here. @@ -311,10 +317,13 @@ const GUARD_ADOPTION_CASES = [ // The measured reproduction: the real import line deleted, the docblock left // exactly as it was. Against the raw text this came back CLEAN and the // self-test printed "both gates still routed through it". - ['a docblock mention is not an import', [FIXTURE_PROSE, FIXTURE_COUNT], [NO_IMPORT]], + // (FIXTURE_COUNT is a bare `lintText` count, so it now trips #10599's test + // too — the fixture really does carry both defects, and a case that under- + // states what its own source does is a case nobody can re-derive.) + ['a docblock mention is not an import', [FIXTURE_PROSE, FIXTURE_COUNT], [NO_IMPORT, BARE_TEXT]], // "A guard imported once is not a guard still called" — the docblock's own // thesis, which nothing used to assert. - ['imports the guard and never calls it', [FIXTURE_PROSE, FIXTURE_IMPORT, FIXTURE_COUNT], [NOT_ARMED]], + ['imports the guard and never calls it', [FIXTURE_PROSE, FIXTURE_IMPORT, FIXTURE_COUNT], [NOT_ARMED, BARE_TEXT]], // The same sentence one step further: commenting the call out leaves the // identifier in the text. ['a commented-out call is not a call', [FIXTURE_IMPORT, '// ' + FIXTURE_CALL_STRICT], [NOT_ARMED]], @@ -326,6 +335,31 @@ const GUARD_ADOPTION_CASES = [ // fabricates a finding out of prose (#9367), and this check must not. ['a commented-out raw call is not a raw call', [FIXTURE_IMPORT, FIXTURE_CALL_STRICT, '// was: ' + FIXTURE_CALL_RAW], []], + + // ── The lintText half (#10599) ───────────────────────────────────────── + // + // THE REPRODUCTION. Against the checker as #10458 left it this source came + // back with ZERO problems: it imports the guard, it still calls + // lintFilesStrict() for the first population, and it never touches + // `.lintFiles(` — so all three tests passed while the SECOND population, + // counted out of `eslint.lintText()`, went entirely unguarded. + ['a guarded call plus a second population measured through lintText', + [FIXTURE_IMPORT, FIXTURE_CALL_STRICT, FIXTURE_COUNT], [BARE_TEXT]], + // The negative control that makes the ban a rule rather than a spelling + // preference: the counted call routed through the guard is CLEAN. + ['a counted lintText routed through the guard is guarded', + [FIXTURE_IMPORT, FIXTURE_CALL_STRICT, FIXTURE_CALL_TEXT_STRICT], []], + // And the declared non-measurement — the shape this gate's own ground-truth + // fixtures use, which must not be a finding or the ban is unusable. + ['a declared non-measurement is not a finding', + [FIXTURE_IMPORT, FIXTURE_CALL_STRICT, FIXTURE_CALL_TEXT_DECLARED], []], + // A gate whose whole population is text is ARMED without ever calling + // lintFilesStrict(). Reporting that one unguarded would be a false positive + // of this card's own making. + ['armed through lintTextStrict alone', [FIXTURE_IMPORT_TEXT, FIXTURE_CALL_TEXT_STRICT], []], + // The mask, on the new test too: prose about a bare call is not a bare call. + ['a commented-out lintText is not a lintText', + [FIXTURE_IMPORT, FIXTURE_CALL_STRICT, '// was: ' + FIXTURE_COUNT], []], ]; async function selfTest() { @@ -344,8 +378,24 @@ async function selfTest() { baseConfig: measuringConfig(drop), allowInlineConfig: false, }); + // Counted, therefore guarded (#10599). A fixture that stops parsing yields + // zero messages matching the rule — which is precisely what the `silent` + // cases below assert — so an unguarded count here reads a TYPO as proof the + // rule is correctly quiet. Measured on this tree, the `argument 1, object + // literal` report fixture with its closing paren removed: + // + // parses → hits()=1, fatalErrorCount=0 + // typo → hits()=0, fatalErrorCount=1, "Parsing error: ')' expected." + // + // The fatal is now a self-test FAILURE naming the fixture, not a zero. const hits = async (code, filePath = 'packages/objectql/src/__selftest__.ts') => { - const [result] = await eslint.lintText(code, { filePath, warnIgnored: false }); + const [result] = await lintTextStrict(eslint, code, { + filePath, + warnIgnored: false, + gate: 'self-test fixture', + repoRoot, + onFatal: (report) => { failures.push(report); return []; }, + }); return (result?.messages ?? []).filter((m) => m.ruleId === QUERY_OPTIONS_RULE_ID).length; }; @@ -416,9 +466,13 @@ async function selfTest() { baseConfig: eslintConfig, allowInlineConfig: false, }); - const [result] = await blocking.lintText(code, { + // Counted, and the assertion below is `=== 0` — same reason as hits(). + const [result] = await lintTextStrict(blocking, code, { filePath: 'packages/objectql/src/__selftest__.test.ts', warnIgnored: false, + gate: 'self-test fixture (blocking config)', + repoRoot, + onFatal: (report) => { failures.push(report); return []; }, }); const blocked = (result?.messages ?? []).filter((m) => m.ruleId === QUERY_OPTIONS_RULE_ID).length; assert(blocked === 0, 'a *.test.ts path must NOT be blocked by the rule (first cut is non-test)'); @@ -460,9 +514,11 @@ async function selfTest() { // 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 = (', { + const [broken] = await lintTextUnguarded(eslint, 'export const x = (', { filePath: 'packages/objectql/src/__selftest_unparseable__.ts', warnIgnored: false, + why: 'ground truth: what raw ESLint returns for a file that will not parse. ' + + 'Routing this through the guard would prove the guard with the guard.', }); assert( (broken?.messages ?? []).some((m) => m.fatal), @@ -480,9 +536,11 @@ async function selfTest() { `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;', { + const [parses] = await lintTextUnguarded(eslint, 'export const x = 1;', { filePath: 'packages/objectql/src/__selftest_parses__.ts', warnIgnored: false, + why: 'ground truth, the other direction: the guard must stay silent on this one, ' + + 'so it has to reach collectFatalMessages() unfiltered.', }); assert( collectFatalMessages([parses], repoRoot).length === 0, @@ -514,6 +572,59 @@ async function selfTest() { 'lintFilesStrict must pass the results through when every file parsed', ); + // The lintText twin, both ways (#10599). The source ban below is only + // worth having if the spelling it forces actually guards, and a + // pass-through that never checked would satisfy every source test in this + // file while doing nothing — the exact shape #10123 was. + { + let textReported = null; + let forwarded = null; + const stubBroken = { lintText: async (_code, options) => { forwarded = options; return [broken]; } }; + const textRefused = await lintTextStrict(stubBroken, 'irrelevant', { + filePath: 'packages/objectql/src/__selftest_unparseable__.ts', + warnIgnored: false, + gate: 'self-test', + repoRoot, + onFatal: (report) => { textReported = report; return 'refused'; }, + }); + assert(textRefused === 'refused', 'lintTextStrict must not return results when the text did not parse'); + assert( + (textReported ?? '').includes('__selftest_unparseable__.ts') && /Parsing error/i.test(textReported ?? ''), + `lintTextStrict's failure text must name the file and the parse error (got: ${textReported})`, + ); + // The guard's own options must not reach ESLint: it rejects unknown keys, + // so a leak here is a crash at every call site, not a silent oddity. + assert( + forwarded !== null + && Object.keys(forwarded).sort().join(',') === 'filePath,warnIgnored', + `lintTextStrict must forward only lintText's own options (forwarded ${JSON.stringify(forwarded)})`, + ); + + let textFired = false; + const stubClean = { lintText: async () => [parses] }; + const textPassed = await lintTextStrict(stubClean, 'irrelevant', { + gate: 'self-test', + repoRoot, + onFatal: () => { textFired = true; }, + }); + assert( + !textFired && Array.isArray(textPassed) && textPassed.length === 1, + 'lintTextStrict must pass the results through when the text parsed', + ); + + // And the declaration is a declaration: an undeclared escape is a throw, + // not a quiet pass. An escape hatch nobody has to name is just the bare + // call with extra steps. + let threw = null; + try { + await lintTextUnguarded(stubClean, 'irrelevant', { filePath: 'x.ts' }); + } catch (err) { threw = err; } + assert( + threw instanceof TypeError && /requires `why`/.test(threw?.message ?? ''), + `lintTextUnguarded must refuse an undeclared call (threw: ${threw?.message ?? 'nothing'})`, + ); + } + // A guard imported once is not a guard still called — proved in both // directions over the fixtures above, because the live-tree call that // follows can only ever confirm the direction this tree is already in. @@ -613,8 +724,9 @@ 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), ` + - `fatal-parse guard proved both ways over real ESLint output, both gates still routed through it ` + - `(adoption proved both ways over ${GUARD_ADOPTION_CASES.length} synthetic gate source(s)), ` + + `fatal-parse guard proved both ways over real ESLint output, every counted lint call in both ` + + `gates routed through it (adoption proved both ways over ${GUARD_ADOPTION_CASES.length} synthetic ` + + `gate source(s), files AND text), ` + `and ${HEADROOM_CANARY_FILE} parses at --stack-size=${PARSER_STACK_SIZE_KB} through this gate's own channel.`, ); } diff --git a/scripts/eslint-fatal-guard.mjs b/scripts/eslint-fatal-guard.mjs index da1bbcc67c..c4d80bfed6 100644 --- a/scripts/eslint-fatal-guard.mjs +++ b/scripts/eslint-fatal-guard.mjs @@ -91,6 +91,38 @@ // to "comment or code", #9367) rather than raw; and "the name appears" was // never the claim — `lintFilesStrict(` must actually be CALLED, because a // guard imported once is not a guard still called. +// +// ── MEASURED (#10599): the adoption test names a CALL, not a MEASUREMENT ─── +// +// Those two tests ask whether `lintFilesStrict(` appears. A gate that KEPT one +// guarded call and measured a second population through `eslint.lintText()` +// answers yes to all three questions while the second population goes +// unguarded. Both fixtures run through the checker as it stood after #10458: +// +// import + `eslint.lintText(...)`, no strict call → 1 problem (not armed) +// import + strict call + `eslint.lintText(...)` → 0 problems ← the hole +// +// So the "measures only through lintText" shape was already caught, and the +// MIXED shape was not. This is not hypothetical plumbing: this gate's own +// self-test counted through a bare `lintText()`. The same reporting fixture +// with one character removed, through that helper: +// +// parses → hits()=1 fatalErrorCount=0 +// `as any;` (missing paren) → hits()=0 fatalErrorCount=1 +// "Parsing error: ')' expected." +// +// hits()=0 is what the ten `silent` assertions require, so a fixture that +// stopped parsing would have read as PROOF THAT THE RULE IS QUIET about it. +// The guard's own failure mode, inside the self-test that asserts the guard. +// +// A blanket `/\.lintText\s*\(/` ban was not available: this gate legitimately +// lints text to establish what raw ESLint does with a file that will not parse +// — ground truth the guard is built on, which routing through the guard would +// make circular. Source text cannot tell that call from a measurement; which +// result gets COUNTED is data flow. So the check does not guess. It bans the +// BARE spelling and the gate declares which kind each call is — +// `lintTextStrict()` when the result is counted, `lintTextUnguarded({ why })` +// when it is not. import { readFileSync } from 'node:fs'; import { relative, resolve } from 'node:path'; import process from 'node:process'; @@ -192,6 +224,68 @@ export async function lintFilesStrict(eslint, targets, { gate, repoRoot, onFatal return results; } +/** + * `eslint.lintText()`, fatal-checked exactly as `lintFilesStrict()` is. + * + * The guard's claim is about MEASUREMENTS, not about one method name: a gate + * that counts the messages coming back from `lintText()` drops a parse failure + * for the same reason `lintFiles()` did — ESLint returns it as a message with + * no rule id, matching no rule the gate counts. Anything whose result is + * COUNTED belongs here. + * + * Every option other than the guard's own is forwarded to `eslint.lintText()`, + * so a call site reads like the bare one it replaces. + * + * @param {{lintText: (code: string, options?: object) => Promise}} eslint + * @param {string} code + * @param {{gate: string, repoRoot?: string, onFatal?: (report: string, fatals: object[]) => never|unknown}} options + * @returns {Promise} the results, when the text parsed + */ +export async function lintTextStrict(eslint, code, { gate, repoRoot, onFatal = exitOnFatal, ...textOptions } = {}) { + const results = await eslint.lintText(code, textOptions); + const fatals = collectFatalMessages(results, repoRoot); + if (fatals.length > 0) return onFatal(formatFatalReport(gate ?? 'eslint-fatal-guard', fatals), fatals); + return results; +} + +/** + * `eslint.lintText()`, DECLARED as not a measurement. Behaviour: none added. + * + * This exists to be written down, not to do anything. `checkGuardAdoption()` + * cannot tell a gate MEASURING a population through `lintText()` from a + * self-test EXERCISING the linter — which result gets counted is a data-flow + * fact, and the check reads source text. Rather than guess with a heuristic + * that fires on the next author who writes a legitimate one, the distinction + * moves to where it is decidable: the author states it, at the call site, in + * code that survives comment stripping. A bare `.lintText(` in a guarded gate + * is then a finding with no judgement call left in it. + * + * The legitimate use is a call whose result is GROUND TRUTH FOR the guard + * rather than input to a count — the self-test fixture that must not parse, + * and its parses-cleanly control. Routing those through `lintTextStrict()` + * would be circular: they exist to establish the raw ESLint behaviour the + * guard is built on, so they must see it raw. + * + * It is an escape hatch and it is meant to be one: an author CAN route a real + * measurement through it. What it buys is that doing so takes typing the word + * `Unguarded` and a reason next to the call, where a reviewer reads it, + * instead of the silence that made #10123 and #10458 possible. + * + * @param {{lintText: (code: string, options?: object) => Promise}} eslint + * @param {string} code + * @param {{why: string}} options `why` is required; every other key goes to `lintText` + * @returns {Promise} whatever ESLint returned, fatals and all + */ +export async function lintTextUnguarded(eslint, code, { why, ...textOptions } = {}) { + if (typeof why !== 'string' || why.trim() === '') { + throw new TypeError( + 'lintTextUnguarded() requires `why`: the reason this lint result is not a measurement. ' + + 'If it IS counted, call lintTextStrict() instead (#10599).', + ); + } + return eslint.lintText(code, textOptions); +} + /** The default handler: print the report and stop. Never returns. */ function exitOnFatal(report) { console.error(report); @@ -251,17 +345,44 @@ export function guardAdoptionProblems(gate, source) { `${gate}: does not import scripts/eslint-fatal-guard.mjs. A gate that counts ` + 'ESLint messages scores an unparseable file as clean without it (#10123).', ); - } else if (!/lintFilesStrict\s*\(/.test(src)) { + } else if (!/lintFilesStrict\s*\(|lintTextStrict\s*\(/.test(src)) { // The docblock's own thesis, asserted rather than assumed: a guard imported // once is not a guard still called. Importing this module runs none of it, // and the `.lintFiles(` test below cannot cover the gap — a gate that // stopped calling anything has no direct call left to catch. + // + // EITHER guarded entry point arms a gate (#10599). A gate whose whole + // population is text would route it through lintTextStrict() and never + // call lintFilesStrict() at all; demanding the files spelling would report + // a fully guarded gate as unguarded, which is how a true gate gets argued + // back out. problems.push( - `${gate}: imports scripts/eslint-fatal-guard.mjs but never calls lintFilesStrict(). ` + + `${gate}: imports scripts/eslint-fatal-guard.mjs but never calls lintFilesStrict() ` + + 'or lintTextStrict(). ' + 'Importing the guard does not arm it: a gate measuring around it still scores an ' + 'unparseable file as clean (#10123).', ); } + // The same claim about the OTHER method (#10599). `lintFilesStrict()` wraps + // `lintFiles` and nothing else, so a gate that kept one guarded call and + // measured a SECOND population through `eslint.lintText()` satisfied every + // test above while that second population went unguarded — measured on this + // tree, three problems reported, zero of them this one. The two tests above + // catch the gate that measures ONLY through lintText (it has no + // `lintFilesStrict(` call left to find); they cannot see the mixed one. + // + // Bare is the finding, not `lintText` itself: a guarded gate spells the + // counted ones `lintTextStrict(` and declares the rest `lintTextUnguarded(`, + // neither of which carries a `.lintText(`. That is why this is a ban and not + // a heuristic — nothing here has to guess which call is the measurement. + if (/\.lintText\s*\(/.test(src)) { + problems.push( + `${gate}: calls \`.lintText(\` directly. A counted lintText result discards a ` + + 'parse failure exactly as `.lintFiles(` did — it comes back as a message with no ' + + 'rule id. Call lintTextStrict() if the result is counted, or lintTextUnguarded() ' + + 'with a `why` if it is not a measurement (#10599).', + ); + } if (/\.lintFiles\s*\(/.test(src)) { problems.push( `${gate}: calls \`.lintFiles(\` directly, so a parse failure in its population ` +