Skip to content

fix(scripts): the fatal-guard adoption check reads a docblock mention as an import — mask the source and assert the guard is called#10598

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
os-zhuang merged 1 commit into from
Aug 21, 2026
+145 −17
Merged

fix(scripts): the fatal-guard adoption check reads a docblock mention as an import — mask the source and assert the guard is called #10598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 75 additions & 5 deletions scripts/check-query-options-erasure-ratchet.mjs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand DownExpand Up@@ -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); };
Expand DownExpand Up@@ -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);
}

Expand DownExpand Up@@ -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.`,
);
}
Expand Down
82 changes: 70 additions & 12 deletions scripts/eslint-fatal-guard.mjs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;

Expand DownExpand Up@@ -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;
}
Loading