diff --git a/scripts/check-query-options-erasure-ratchet.mjs b/scripts/check-query-options-erasure-ratchet.mjs index 21aa87071b..0242f50abd 100644 --- a/scripts/check-query-options-erasure-ratchet.mjs +++ b/scripts/check-query-options-erasure-ratchet.mjs @@ -97,7 +97,12 @@ import eslintConfig, { QUERY_OPTIONS_TEST_GLOBS, QUERY_OPTIONS_ANY_MESSAGE, } from '../eslint.config.mjs'; -import { checkGuardAdoption, collectFatalMessages, lintFilesStrict } from './eslint-fatal-guard.mjs'; +import { + checkGuardAdoption, + collectFatalMessages, + guardAdoptionProblems, + lintFilesStrict, +} from './eslint-fatal-guard.mjs'; import { HEADROOM_CANARY_FILE, PARSER_STACK_SIZE_KB, @@ -270,6 +275,59 @@ function baselineKeysAddedSinceMergeBase(baselineKeys) { // --------------------------------------------------------------------------- // --self-test +// ── Guard-adoption fixtures (#10458) ────────────────────────────────────── +// +// checkGuardAdoption() reads THIS FILE, so its fixtures cannot be written the +// obvious way. `stripComments` deliberately keeps string literals — a gate's +// signal usually IS a string — so a contiguous `lintFilesStrict` + `(` inside +// a fixture would satisfy this gate's own call test after its real calls were +// gone, and a contiguous `.lintFiles` + `(` would report this gate as +// unguarded outright. Both call shapes are therefore spelled with a `+`: the +// runtime string is what the check sees, the source text is not a decoy. Do +// not "tidy" them into single literals. +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_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 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'; + +/** + * The adoption check in both directions, over sources written here. + * + * The live-tree assertion below can only prove the direction today's tree is + * in, and both gates are adopted today — so on its own it is exactly the shape + * #4690 warns about: a check that has only ever been green. The reject side is + * asserted positively here, and each case is a real regression someone could + * land: `[name, source lines, the problems it must produce]`. + */ +const GUARD_ADOPTION_CASES = [ + // The positive control. A zero-hit result over the other five means nothing + // without a case that is supposed to come back clean and does. + ['imports the guard and calls it', [FIXTURE_PROSE, FIXTURE_IMPORT, FIXTURE_CALL_STRICT], []], + // 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]], + // "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]], + // 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]], + // Back to unguarded ESLint: both problems, because it is both. + // (the case NAME avoids the raw call shape too — a decoy is a decoy in a + // label as much as in a fixture, and this one did red the gate once.) + ['went back to unguarded ESLint', [FIXTURE_IMPORT, FIXTURE_CALL_RAW], [NOT_ARMED, RAW_CALL]], + // The mask's other direction. Over-masking costs recall; UNDER-masking + // 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], []], +]; + async function selfTest() { const failures = []; const assert = (cond, msg) => { if (!cond) failures.push(msg); }; @@ -456,9 +514,20 @@ async function selfTest() { '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. + // 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. + for (const [name, lines, expected] of GUARD_ADOPTION_CASES) { + const problems = guardAdoptionProblems('scripts/__adoption_fixture__.mjs', lines.join('\n')); + assert( + problems.length === expected.length && expected.every((e) => problems.some((p) => p.includes(e))), + `guard adoption, ${name}: expected ${JSON.stringify(expected)}, got ${JSON.stringify(problems)}`, + ); + } + + // And the live tree. 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); } @@ -544,7 +613,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), ` + - `fatal-parse guard proved both ways over real ESLint output, both gates still routed through it, ` + + `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)), ` + `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 09536ca56d..da1bbcc67c 100644 --- a/scripts/eslint-fatal-guard.mjs +++ b/scripts/eslint-fatal-guard.mjs @@ -71,10 +71,32 @@ // 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. +// +// ── MEASURED (#10458): reading the source has to mean reading CODE ──────── +// +// Those assertions scanned the RAW file, comments included — and both gates +// carry a `//` line naming this module in their own docblocks. So the import +// test was satisfied by PROSE. Deleting check-slot-lookup-ratchet.mjs's real +// `import { lintFilesStrict } …` line and leaving its docblock exactly as it +// was measured: +// +// ON-DISK: real import lines=0 ; docblock mentions=1 +// $ node scripts/check-query-options-erasure-ratchet.mjs --self-test +// ✓ self-test: … both gates still routed through it. +// exit=0 +// +// Green, with the sentence it printed false, on the one check whose whole job +// is noticing that a gate went quiet. Two things follow, and both are below: +// the source is read through scripts/js-comment-mask.mjs (the repo-wide answer +// 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. import { readFileSync } from 'node:fs'; import { relative, resolve } from 'node:path'; import process from 'node:process'; +import { stripComments } from './js-comment-mask.mjs'; + /** "This gate could not measure", as distinct from 1 = "the ratchet moved". */ export const FATAL_GUARD_EXIT_CODE = 2; @@ -197,18 +219,54 @@ export function checkGuardAdoption(repoRoot) { 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.', - ); - } + problems.push(...guardAdoptionProblems(gate, src)); + } + return problems; +} + +/** + * The adoption verdict for ONE gate, from its source text. + * + * Split out and kept pure so the self-test can drive it over synthetic sources + * in BOTH directions. The live-tree call above can only ever prove the + * direction today's tree happens to be in, and being green when it should be + * red is this check's entire failure mode (#10458). + * + * @param {string} gate the gate's name, for the messages + * @param {string} source the gate's source, comments and all + * @returns {string[]} problems, empty when this gate is still guarded + */ +export function guardAdoptionProblems(gate, source) { + const problems = []; + // Prose is not adoption. Both gates name this module in their docblocks, so + // against the RAW text the import test below was satisfied by a comment — + // green at exactly the moment a gate stopped importing it (#10458). The + // repo-wide answer to "comment or code" is scripts/js-comment-mask.mjs + // (#9367); a private strip here would be another copy of what that exists to + // retire. `stripComments` rather than `maskComments` because this reports + // gate NAMES, never a line or an offset into the original text. + const src = stripComments(source); + 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).', + ); + } else if (!/lintFilesStrict\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. + problems.push( + `${gate}: imports scripts/eslint-fatal-guard.mjs but never calls lintFilesStrict(). ` + + 'Importing the guard does not arm it: a gate measuring around it still scores an ' + + 'unparseable file as clean (#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; }