Skip to content

Commit c109edb

Browse files
claude[bot]claude
andauthored
fix(scripts): read the fatal-guard adoption check's source as CODE, and assert the guard is CALLED (#10598)
`checkGuardAdoption()` decided both of its questions by regex over the RAW gate source, comments included. Both guarded gates name `scripts/eslint-fatal-guard.mjs` in their own docblocks, so the import test was satisfied by PROSE. Measured on this tree: deleting check-slot-lookup-ratchet.mjs's real `import { lintFilesStrict } …` line and leaving its line-27 docblock exactly as it was gave 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 printed sentence false, on the one check whose whole job is noticing that a gate went quiet. Two holes, both closed here: 1. The source is now read through `stripComments()` from scripts/js-comment-mask.mjs (#9367's ONE answer to "comment or code"). `stripComments` rather than `maskComments` because this check reports gate NAMES, never a line or an offset. 2. "A guard imported once is not a guard still called" is the docblock's own thesis and nothing asserted it. `lintFilesStrict(` must now actually be called; the pre-existing `.lintFiles(` test cannot cover that gap, since a gate that stopped calling anything has no direct call left to catch. The per-gate verdict moves into a pure `guardAdoptionProblems(gate, source)` so the self-test can drive it over synthetic sources in BOTH directions: the live-tree assertion can only ever confirm the direction this tree is already in, and both gates are adopted today. Six fixtures, one of them a positive control that must come back clean. Both call shapes in those fixtures are spelled with a `+` on purpose: `checkGuardAdoption()` reads that file, `stripComments` deliberately keeps string literals, and a contiguous `lintFilesStrict(` or `.lintFiles(` in a fixture would be a decoy in the gate's own source — the same hole one layer up. A case LABEL carrying the raw call shape reddened the gate once while this was being written, which is why the labels avoid it too. Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt Co-authored-by: Claude <noreply@anthropic.com>
1 parent 71a0198 commit c109edb

2 files changed

Lines changed: 145 additions & 17 deletions

File tree

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

Lines changed: 75 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,12 @@ import eslintConfig, {
9797
QUERY_OPTIONS_TEST_GLOBS,
9898
QUERY_OPTIONS_ANY_MESSAGE,
9999
} from '../eslint.config.mjs';
100-
import { checkGuardAdoption, collectFatalMessages, lintFilesStrict } from './eslint-fatal-guard.mjs';
100+
import {
101+
checkGuardAdoption,
102+
collectFatalMessages,
103+
guardAdoptionProblems,
104+
lintFilesStrict,
105+
} from './eslint-fatal-guard.mjs';
101106
import {
102107
HEADROOM_CANARY_FILE,
103108
PARSER_STACK_SIZE_KB,
@@ -270,6 +275,59 @@ function baselineKeysAddedSinceMergeBase(baselineKeys) {
270275
// ---------------------------------------------------------------------------
271276
// --self-test
272277

278+
// ── Guard-adoption fixtures (#10458) ──────────────────────────────────────
279+
//
280+
// checkGuardAdoption() reads THIS FILE, so its fixtures cannot be written the
281+
// obvious way. `stripComments` deliberately keeps string literals — a gate's
282+
// signal usually IS a string — so a contiguous `lintFilesStrict` + `(` inside
283+
// a fixture would satisfy this gate's own call test after its real calls were
284+
// gone, and a contiguous `.lintFiles` + `(` would report this gate as
285+
// unguarded outright. Both call shapes are therefore spelled with a `+`: the
286+
// runtime string is what the check sees, the source text is not a decoy. Do
287+
// not "tidy" them into single literals.
288+
const FIXTURE_CALL_STRICT = 'const results = await lintFilesStrict' + '(eslint, [TARGET], { gate: G });';
289+
const FIXTURE_CALL_RAW = 'const results = await eslint.lintFiles' + '([TARGET]);';
290+
const FIXTURE_IMPORT = "import { lintFilesStrict } from './eslint-fatal-guard.mjs';";
291+
const FIXTURE_PROSE = '// on the same input. scripts/eslint-fatal-guard.mjs carries the measurement and';
292+
const FIXTURE_COUNT = 'const sites = (await eslint.lintText(code)).messages.filter(matches).length;';
293+
294+
const NO_IMPORT = 'does not import scripts/eslint-fatal-guard.mjs';
295+
const NOT_ARMED = 'Importing the guard does not arm it';
296+
const RAW_CALL = 'directly, so a parse failure in its population';
297+
298+
/**
299+
* The adoption check in both directions, over sources written here.
300+
*
301+
* The live-tree assertion below can only prove the direction today's tree is
302+
* in, and both gates are adopted today — so on its own it is exactly the shape
303+
* #4690 warns about: a check that has only ever been green. The reject side is
304+
* asserted positively here, and each case is a real regression someone could
305+
* land: `[name, source lines, the problems it must produce]`.
306+
*/
307+
const GUARD_ADOPTION_CASES = [
308+
// The positive control. A zero-hit result over the other five means nothing
309+
// without a case that is supposed to come back clean and does.
310+
['imports the guard and calls it', [FIXTURE_PROSE, FIXTURE_IMPORT, FIXTURE_CALL_STRICT], []],
311+
// The measured reproduction: the real import line deleted, the docblock left
312+
// exactly as it was. Against the raw text this came back CLEAN and the
313+
// self-test printed "both gates still routed through it".
314+
['a docblock mention is not an import', [FIXTURE_PROSE, FIXTURE_COUNT], [NO_IMPORT]],
315+
// "A guard imported once is not a guard still called" — the docblock's own
316+
// thesis, which nothing used to assert.
317+
['imports the guard and never calls it', [FIXTURE_PROSE, FIXTURE_IMPORT, FIXTURE_COUNT], [NOT_ARMED]],
318+
// The same sentence one step further: commenting the call out leaves the
319+
// identifier in the text.
320+
['a commented-out call is not a call', [FIXTURE_IMPORT, '// ' + FIXTURE_CALL_STRICT], [NOT_ARMED]],
321+
// Back to unguarded ESLint: both problems, because it is both.
322+
// (the case NAME avoids the raw call shape too — a decoy is a decoy in a
323+
// label as much as in a fixture, and this one did red the gate once.)
324+
['went back to unguarded ESLint', [FIXTURE_IMPORT, FIXTURE_CALL_RAW], [NOT_ARMED, RAW_CALL]],
325+
// The mask's other direction. Over-masking costs recall; UNDER-masking
326+
// fabricates a finding out of prose (#9367), and this check must not.
327+
['a commented-out raw call is not a raw call',
328+
[FIXTURE_IMPORT, FIXTURE_CALL_STRICT, '// was: ' + FIXTURE_CALL_RAW], []],
329+
];
330+
273331
async function selfTest() {
274332
const failures = [];
275333
const assert = (cond, msg) => { if (!cond) failures.push(msg); };
@@ -456,9 +514,20 @@ async function selfTest() {
456514
'lintFilesStrict must pass the results through when every file parsed',
457515
);
458516

459-
// A guard imported once is not a guard still called. This is also the only
460-
// wired coverage of the OTHER gate's call site: `pnpm check:slot-lookup`
461-
// has no --self-test hook, and CI runs this one before the gate itself.
517+
// A guard imported once is not a guard still called — proved in both
518+
// directions over the fixtures above, because the live-tree call that
519+
// follows can only ever confirm the direction this tree is already in.
520+
for (const [name, lines, expected] of GUARD_ADOPTION_CASES) {
521+
const problems = guardAdoptionProblems('scripts/__adoption_fixture__.mjs', lines.join('\n'));
522+
assert(
523+
problems.length === expected.length && expected.every((e) => problems.some((p) => p.includes(e))),
524+
`guard adoption, ${name}: expected ${JSON.stringify(expected)}, got ${JSON.stringify(problems)}`,
525+
);
526+
}
527+
528+
// And the live tree. This is also the only wired coverage of the OTHER
529+
// gate's call site: `pnpm check:slot-lookup` has no --self-test hook, and
530+
// CI runs this one before the gate itself.
462531
for (const problem of checkGuardAdoption(repoRoot)) assert(false, problem);
463532
}
464533

@@ -544,7 +613,8 @@ async function selfTest() {
544613
console.log(
545614
`✓ self-test: ${reports.length} reporting shape(s), ${silent.length} silent counterpart(s), ` +
546615
`grandfathering + test-glob channels proved in both directions, ${cases.length} ratchet case(s), ` +
547-
`fatal-parse guard proved both ways over real ESLint output, both gates still routed through it, ` +
616+
`fatal-parse guard proved both ways over real ESLint output, both gates still routed through it ` +
617+
`(adoption proved both ways over ${GUARD_ADOPTION_CASES.length} synthetic gate source(s)), ` +
548618
`and ${HEADROOM_CANARY_FILE} parses at --stack-size=${PARSER_STACK_SIZE_KB} through this gate's own channel.`,
549619
);
550620
}

scripts/eslint-fatal-guard.mjs

Lines changed: 70 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,32 @@
7171
// runs ahead of the gate itself (`pnpm check:query-options-erasure`);
7272
// `pnpm check:slot-lookup` has no self-test hook of its own, so the coverage of
7373
// ITS call site is the source assertion, not a second wired self-test.
74+
//
75+
// ── MEASURED (#10458): reading the source has to mean reading CODE ────────
76+
//
77+
// Those assertions scanned the RAW file, comments included — and both gates
78+
// carry a `//` line naming this module in their own docblocks. So the import
79+
// test was satisfied by PROSE. Deleting check-slot-lookup-ratchet.mjs's real
80+
// `import { lintFilesStrict } …` line and leaving its docblock exactly as it
81+
// was measured:
82+
//
83+
// ON-DISK: real import lines=0 ; docblock mentions=1
84+
// $ node scripts/check-query-options-erasure-ratchet.mjs --self-test
85+
// ✓ self-test: … both gates still routed through it.
86+
// exit=0
87+
//
88+
// Green, with the sentence it printed false, on the one check whose whole job
89+
// is noticing that a gate went quiet. Two things follow, and both are below:
90+
// the source is read through scripts/js-comment-mask.mjs (the repo-wide answer
91+
// to "comment or code", #9367) rather than raw; and "the name appears" was
92+
// never the claim — `lintFilesStrict(` must actually be CALLED, because a
93+
// guard imported once is not a guard still called.
7494
import { readFileSync } from 'node:fs';
7595
import { relative, resolve } from 'node:path';
7696
import process from 'node:process';
7797

98+
import { stripComments } from './js-comment-mask.mjs';
99+
78100
/** "This gate could not measure", as distinct from 1 = "the ratchet moved". */
79101
export const FATAL_GUARD_EXIT_CODE = 2;
80102

@@ -197,18 +219,54 @@ export function checkGuardAdoption(repoRoot) {
197219
problems.push(`${gate}: named by the fatal-parse guard but unreadable — renamed or removed?`);
198220
continue;
199221
}
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-
}
222+
problems.push(...guardAdoptionProblems(gate, src));
223+
}
224+
return problems;
225+
}
226+
227+
/**
228+
* The adoption verdict for ONE gate, from its source text.
229+
*
230+
* Split out and kept pure so the self-test can drive it over synthetic sources
231+
* in BOTH directions. The live-tree call above can only ever prove the
232+
* direction today's tree happens to be in, and being green when it should be
233+
* red is this check's entire failure mode (#10458).
234+
*
235+
* @param {string} gate the gate's name, for the messages
236+
* @param {string} source the gate's source, comments and all
237+
* @returns {string[]} problems, empty when this gate is still guarded
238+
*/
239+
export function guardAdoptionProblems(gate, source) {
240+
const problems = [];
241+
// Prose is not adoption. Both gates name this module in their docblocks, so
242+
// against the RAW text the import test below was satisfied by a comment —
243+
// green at exactly the moment a gate stopped importing it (#10458). The
244+
// repo-wide answer to "comment or code" is scripts/js-comment-mask.mjs
245+
// (#9367); a private strip here would be another copy of what that exists to
246+
// retire. `stripComments` rather than `maskComments` because this reports
247+
// gate NAMES, never a line or an offset into the original text.
248+
const src = stripComments(source);
249+
if (!/eslint-fatal-guard\.mjs/.test(src)) {
250+
problems.push(
251+
`${gate}: does not import scripts/eslint-fatal-guard.mjs. A gate that counts ` +
252+
'ESLint messages scores an unparseable file as clean without it (#10123).',
253+
);
254+
} else if (!/lintFilesStrict\s*\(/.test(src)) {
255+
// The docblock's own thesis, asserted rather than assumed: a guard imported
256+
// once is not a guard still called. Importing this module runs none of it,
257+
// and the `.lintFiles(` test below cannot cover the gap — a gate that
258+
// stopped calling anything has no direct call left to catch.
259+
problems.push(
260+
`${gate}: imports scripts/eslint-fatal-guard.mjs but never calls lintFilesStrict(). ` +
261+
'Importing the guard does not arm it: a gate measuring around it still scores an ' +
262+
'unparseable file as clean (#10123).',
263+
);
264+
}
265+
if (/\.lintFiles\s*\(/.test(src)) {
266+
problems.push(
267+
`${gate}: calls \`.lintFiles(\` directly, so a parse failure in its population ` +
268+
'is discarded as a message matching no rule. Call lintFilesStrict() instead.',
269+
);
212270
}
213271
return problems;
214272
}

0 commit comments

Comments
 (0)