diff --git a/scripts/check-query-options-erasure-ratchet.mjs b/scripts/check-query-options-erasure-ratchet.mjs index 5210d3a885..6cfb10877c 100644 --- a/scripts/check-query-options-erasure-ratchet.mjs +++ b/scripts/check-query-options-erasure-ratchet.mjs @@ -102,6 +102,7 @@ import { collectFatalMessages, guardAdoptionProblems, lintFilesStrict, + lintFilesUnguarded, lintTextStrict, lintTextUnguarded, } from './eslint-fatal-guard.mjs'; @@ -362,6 +363,124 @@ const GUARD_ADOPTION_CASES = [ [FIXTURE_IMPORT, FIXTURE_CALL_STRICT, '// was: ' + FIXTURE_COUNT], []], ]; +// ── Guard-closure fixtures (#10625) ─────────────────────────────────────── +// +// Every case above is one file's text, which is exactly the bound this set +// exists to close: a gate that moved its counting into a sibling module +// presents a gate file with no banned shape in it, and passes all four tests +// on whatever strict call it kept. Measured against the checker as #10599 left +// it, the first case below came back with ZERO problems. +// +// TWO decoy disciplines apply here, and they are NOT the same rule: +// +// • CALL shapes still take the `+` split. The bans read `stripComments`, +// which keeps string literals (#10598), so a contiguous `.lintFiles` + `(` +// in this file is a decoy in the live-tree scan of this very gate. +// • SPECIFIERS do not, and must not be "made consistent" with them. The +// closure walk is literal-AWARE — an import spelling inside a string is +// skipped, which is the only reason scripts/invoked-as.mjs (which writes +// `await import(${…})` into a template) is not read as a computed import +// of its own. The `a specifier inside a string is not an import` case +// below is that behaviour asserted, and it needs the specifier spelled +// contiguously to mean anything. +const FAKE_GATE = 'scripts/__closure_fixture_gate__.mjs'; +const FAKE_HELPER = 'scripts/__closure_fixture_helper__.mjs'; +const FAKE_DEEPER = 'scripts/__closure_fixture_deeper__.mjs'; +const FIXTURE_IMPORT_HELPER = "import { measure } from './__closure_fixture_helper__.mjs';"; +const FIXTURE_IMPORT_DEEPER = "import { count } from './__closure_fixture_deeper__.mjs';"; +const FIXTURE_DELEGATE = 'const second = await measure(eslint, code);'; +const FIXTURE_IMPORT_FILES_DECLARED = "import { lintFilesUnguarded } from './eslint-fatal-guard.mjs';"; +const FIXTURE_CALL_FILES_DECLARED = + "const [r] = await lintFilesUnguarded" + "(eslint, [TARGET], { why: 'ground truth' });"; + +/** A gate that is itself guarded, and hands a second population to a helper. */ +const DELEGATING_GATE = [FIXTURE_IMPORT, FIXTURE_CALL_STRICT, FIXTURE_IMPORT_HELPER, FIXTURE_DELEGATE]; + +const VIA_CLOSURE = "is in the gate's local import closure"; + +/** + * The closure walk in both directions, over synthetic file TREES. + * + * `[name, files, expected]`, where `files` maps a repo-relative path to its + * source lines and each `expected` entry is the list of needles ONE problem + * must carry — the file it names and which test fired, because a case that + * only counts problems cannot tell a right answer from a coincidence. + */ +const GUARD_CLOSURE_CASES = [ + // THE REPRODUCTION. Zero problems before this card. + ['the counted call moved one import out', + { [FAKE_GATE]: DELEGATING_GATE, [FAKE_HELPER]: [FIXTURE_CALL_RAW] }, + [[FAKE_HELPER, RAW_CALL, VIA_CLOSURE]]], + // …and the same move with the other method, which is #10599's hole one + // indirection further out. + ['a second population counted through lintText, one import out', + { [FAKE_GATE]: DELEGATING_GATE, [FAKE_HELPER]: [FIXTURE_COUNT] }, + [[FAKE_HELPER, BARE_TEXT, VIA_CLOSURE]]], + // The positive control. A zero-hit result over the reject cases means + // nothing without a helper that is supposed to come back clean and does. + ['a guarded helper is guarded', + { [FAKE_GATE]: DELEGATING_GATE, [FAKE_HELPER]: [FIXTURE_IMPORT, FIXTURE_CALL_STRICT] }, + []], + // The declaration, one import out: the shape scripts/eslint-stack-headroom.mjs + // uses for its canary. Without this the only way to keep a legitimate raw + // call green is a hand-kept exemption list, which is the thing a derived + // closure was chosen to avoid. + ['a declared non-measurement in a helper is not a finding', + { [FAKE_GATE]: DELEGATING_GATE, + [FAKE_HELPER]: [FIXTURE_IMPORT_FILES_DECLARED, FIXTURE_CALL_FILES_DECLARED] }, + []], + // Transitive, not one level: the walk is what makes the population derived, + // and a one-level walk would just move the same bound to the second hop. + ['a raw call two imports out', + { [FAKE_GATE]: DELEGATING_GATE, + [FAKE_HELPER]: [FIXTURE_IMPORT_DEEPER, 'export const measure = count;'], + [FAKE_DEEPER]: [FIXTURE_CALL_RAW] }, + [[FAKE_DEEPER, RAW_CALL]]], + // The guard module is not in anyone's closure. It holds the raw calls BY + // DESIGN, and — the trap — its own `lintFilesStrict(` definition would + // answer an armed test that is supposed to be about a call somewhere else. + ['the guard module itself is never scanned', + { [FAKE_GATE]: [FIXTURE_IMPORT, FIXTURE_CALL_STRICT], + 'scripts/eslint-fatal-guard.mjs': [FIXTURE_CALL_RAW, FIXTURE_COUNT] }, + []], + // The mask, on the WALK rather than on the bans: a commented-out import + // does not put a file in the closure, so the raw call in it is not this + // gate's problem. Under-masking here fabricates a finding out of prose. + ['a commented-out import does not reach the helper', + { [FAKE_GATE]: [FIXTURE_IMPORT, FIXTURE_CALL_STRICT, '// ' + FIXTURE_IMPORT_HELPER], + [FAKE_HELPER]: [FIXTURE_CALL_RAW] }, + []], + // The literal direction of the same question — the scripts/invoked-as.mjs + // shape, which is in BOTH gates' real closures today. + ['a specifier inside a string is not an import', + { [FAKE_GATE]: [FIXTURE_IMPORT, FIXTURE_CALL_STRICT, + 'const template = ' + JSON.stringify(FIXTURE_IMPORT_HELPER) + ';'], + [FAKE_HELPER]: [FIXTURE_CALL_RAW] }, + []], + // Where the walk STOPS being decidable, it says so rather than reporting a + // closure it cannot claim to have walked. This is the one bound that the + // three cards before this one each discovered the expensive way. + ['a computed specifier is reported, not passed over', + { [FAKE_GATE]: [FIXTURE_IMPORT, FIXTURE_CALL_STRICT, 'const m = await import(specifier);'] }, + [[FAKE_GATE, 'cannot resolve']]], + // A closure member that is not there at all: the walk's own blind spot, + // reported for the same reason a missing GATE is. + ['an unresolvable import is reported', + { [FAKE_GATE]: [FIXTURE_IMPORT, FIXTURE_CALL_STRICT, FIXTURE_IMPORT_HELPER] }, + [[FAKE_HELPER, 'unreadable']]], + // `path.resolve` clamps at `/`, so an import walking out of the repo used to + // come back as a file at the top of it. The bans cannot cover what is not in + // the tree, so the honest answer is to name it. + ['an import that leaves the repository is reported', + { [FAKE_GATE]: [FIXTURE_IMPORT, FIXTURE_CALL_STRICT, "import { x } from '../../outside.mjs';"] }, + [[FAKE_GATE, 'outside the repository']]], + // A cycle must terminate and must report the raw call exactly once. + ['a cycle in the closure terminates', + { [FAKE_GATE]: DELEGATING_GATE, + [FAKE_HELPER]: [FIXTURE_CALL_RAW, "import { gate } from './__closure_fixture_gate__.mjs';"] }, + [[FAKE_HELPER, RAW_CALL]]], +]; + async function selfTest() { const failures = []; const assert = (cond, msg) => { if (!cond) failures.push(msg); }; @@ -623,6 +742,26 @@ async function selfTest() { threw instanceof TypeError && /requires `why`/.test(threw?.message ?? ''), `lintTextUnguarded must refuse an undeclared call (threw: ${threw?.message ?? 'nothing'})`, ); + + // The `lintFiles` twin, same bargain (#10625). It exists because the + // bans now reach a gate's whole import closure, and the closure module + // that lints raw for a real reason needs the same way to say so a gate + // has. If it can be called without naming one, it is not a declaration. + let filesThrew = null; + try { + await lintFilesUnguarded({ lintFiles: async () => [parses] }, [LINT_TARGET], {}); + } catch (err) { filesThrew = err; } + assert( + filesThrew instanceof TypeError && /requires `why`/.test(filesThrew?.message ?? ''), + `lintFilesUnguarded must refuse an undeclared call (threw: ${filesThrew?.message ?? 'nothing'})`, + ); + const declared = await lintFilesUnguarded({ lintFiles: async () => [broken] }, [LINT_TARGET], { + why: 'self-test: proves a declared call passes the fatal STRAIGHT through', + }); + assert( + Array.isArray(declared) && declared.length === 1 && declared[0] === broken, + 'lintFilesUnguarded must return what ESLint returned, fatals and all — it adds no behaviour', + ); } // A guard imported once is not a guard still called — proved in both @@ -636,6 +775,27 @@ async function selfTest() { ); } + // The same sentence about the FILES the tests are applied to (#10625). + // Every case above is one file's text; a measurement moved into a sibling + // module is invisible to all four of them, so the population is walked + // rather than listed — and that walk gets asserted in both directions for + // the same reason the tests themselves do. + for (const [name, files, expected] of GUARD_CLOSURE_CASES) { + const problems = checkGuardAdoption('/__fixture_root__', { + gates: [FAKE_GATE], + readFile: (file) => { + const lines = files[file]; + if (lines === undefined) throw new Error(`ENOENT: ${file}`); + return lines.join('\n'); + }, + }); + assert( + problems.length === expected.length && + expected.every((needles) => problems.some((p) => needles.every((n) => p.includes(n)))), + `guard closure, ${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. @@ -725,8 +885,9 @@ async function selfTest() { `✓ 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, 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), ` + + `gates AND in the closures they import routed through it (adoption proved both ways over ` + + `${GUARD_ADOPTION_CASES.length} synthetic gate source(s), files AND text, and the closure walk ` + + `over ${GUARD_CLOSURE_CASES.length} synthetic tree(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 c4d80bfed6..c6978e1c93 100644 --- a/scripts/eslint-fatal-guard.mjs +++ b/scripts/eslint-fatal-guard.mjs @@ -123,11 +123,71 @@ // BARE spelling and the gate declares which kind each call is — // `lintTextStrict()` when the result is counted, `lintTextUnguarded({ why })` // when it is not. +// +// ── MEASURED (#10625): every test above reads ONE FILE ───────────────────── +// +// The import test, the armed test and both bans are statements about the text +// of the file named in GUARDED_GATES. A gate that moved its counting into a +// sibling module — `import { measure } from './lint-population.mjs'`, with the +// raw call living there — presents a gate file with no banned shape in it at +// all, and keeps passing the import and armed tests on any one strict call it +// still makes. Measured against the checker as #10599 left it: +// +// gate delegating a second population to a sibling helper → 0 problems +// the helper, had anything read it → 2 problems +// +// and nothing reads it: `checkGuardAdoption()` opened exactly GUARDED_GATES. +// +// That was recorded as latent — "neither gate has a helper module today". It +// is not. BOTH gates import `./eslint-stack-headroom.mjs`, and that module has +// held a raw `eslint.lintFiles([file])` since #10449: +// +// $ checkGuardAdoption(repoRoot) → [] +// scripts/eslint-stack-headroom.mjs:212 → eslint.lintFiles([file]) +// imported by check-slot-lookup-ratchet.mjs, check-query-options-…-mjs +// +// Nothing was mis-measured by it — `canaryParseFailures()` hands its results +// straight to `collectFatalMessages()`, which is what the guard would have +// done. So this is still a bound and not a live false green. What the tree +// disproves is the DISTANCE: the sibling module the defect needs already +// exists in both closures, and lands with no diff to any gate. +// +// ── The population, and why it is derived rather than listed ────────────── +// +// The question this file could not answer was WHICH FILES have to carry the +// declarations. It is answered by resolving it instead of writing it down: a +// gate's population is its LOCAL IMPORT CLOSURE — every repo-relative +// specifier it reaches, transitively. That is decidable from source, it is a +// derived fact rather than a hand-kept list that a refactor forgets to update, +// and it is exactly the set of files a measurement can move into without +// touching the gate. +// +// Two exclusions, both load-bearing: +// +// • THIS MODULE is not scanned. Its raw calls ARE the implementation, and +// — the trap — its own `export async function lintFilesStrict(` would +// satisfy an armed test read over the closure, and its own import line +// would satisfy an import test read that way. Scanning it would quietly +// retire two working tests, in the file whose entire history is tests +// going quiet. +// • The import and armed tests stay FILE-scoped on the gate. Read over the +// closure they dilute: `eslint-stack-headroom.mjs` already imports this +// module (for `collectFatalMessages`), so every gate that imports IT +// would pass an import test read over the closure regardless of what the +// gate does. A gate that delegates its whole measurement therefore still +// fails the armed test — a loud false positive, chosen deliberately over +// a silent weakening, and it names the closure so the author can see why. +// +// The bans are what extend, because "no unguarded lint anywhere this gate's +// verdict flows through" is a closure-level claim by nature. A closure module +// that legitimately lints raw declares it, exactly as a gate does — which is +// why `lintFilesUnguarded({ why })` exists below and why the canary above is +// its first caller. import { readFileSync } from 'node:fs'; -import { relative, resolve } from 'node:path'; +import { dirname, relative, resolve } from 'node:path'; import process from 'node:process'; -import { stripComments } from './js-comment-mask.mjs'; +import { blank, scanSource, 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; @@ -141,6 +201,20 @@ export const GUARDED_GATES = [ 'scripts/check-query-options-erasure-ratchet.mjs', ]; +/** + * This module, repo-relative. The one file the closure walk neither scans nor + * walks through: its raw calls are the implementation being adopted, and its + * own `lintFilesStrict(` definition would answer an adoption question that is + * supposed to be about a CALL somewhere else (#10625). + */ +export const GUARD_MODULE = 'scripts/eslint-fatal-guard.mjs'; + +/** Relative specifiers with these extensions can hold a call; others cannot. */ +const CODE_EXTENSIONS = ['.mjs', '.cjs', '.js', '.mts', '.cts', '.ts']; + +/** A stand-in repo root, so an import that walks OUT of the repo stays visible. */ +const CLOSURE_ROOT = '/__closure_root__'; + /** * Every parse failure in an ESLint result set, flattened and repo-relative. * @@ -286,6 +360,43 @@ export async function lintTextUnguarded(eslint, code, { why, ...textOptions } = return eslint.lintText(code, textOptions); } +/** + * `eslint.lintFiles()`, DECLARED as not a measurement. Behaviour: none added. + * + * The `lintFiles` twin of `lintTextUnguarded()`, and it exists for the same + * reason one level out (#10625). Once the bans reach a gate's whole import + * closure, a closure module that lints raw for a legitimate reason needs the + * same way to say so that a gate has — otherwise the only ways to keep the + * tree green are a hand-kept exemption list (the thing a derived closure was + * chosen to avoid) or wrapping a call that must not be wrapped. + * + * `canaryParseFailures()` in scripts/eslint-stack-headroom.mjs is the first + * caller and the shape to copy: it lints a single file and hands the results + * to `collectFatalMessages()` itself, so the parse failure is not discarded — + * it is the thing being looked for. Routing it through `lintFilesStrict()` + * would be circular AND lossy: the guard's exit path would fire first and + * print the generic report, replacing the canary's own text, which is the only + * place a reader is told the remedy is `--stack-size` rather than a code fix. + * + * Same escape hatch, same bargain as the text twin: an author CAN route a real + * measurement through it, and doing so costs typing `Unguarded` and a reason + * next to the call, where a reviewer reads it. + * + * @param {{lintFiles: (targets: string[]) => Promise}} eslint + * @param {string[]} targets + * @param {{why: string}} options `why` is required + * @returns {Promise} whatever ESLint returned, fatals and all + */ +export async function lintFilesUnguarded(eslint, targets, { why } = {}) { + if (typeof why !== 'string' || why.trim() === '') { + throw new TypeError( + 'lintFilesUnguarded() requires `why`: the reason this lint result is not a measurement. ' + + 'If it IS counted, call lintFilesStrict() instead (#10625).', + ); + } + return eslint.lintFiles(targets); +} + /** The default handler: print the report and stop. Never returns. */ function exitOnFatal(report) { console.error(report); @@ -293,27 +404,162 @@ function exitOnFatal(report) { } /** - * Assert every gate in GUARDED_GATES still routes through this module. + * The repo-relative module specifiers one source reaches, and the ones it + * reaches in a way this file cannot resolve. + * + * Anchored on the specifier rather than on the statement. The obvious spelling + * — match `import`/`export`, then a lazy `[\s\S]*?`, then `from` — is the + * quadratic shape scripts/js-comment-mask.mjs measured at 51x on this repo + * once the comments are blanked rather than deleted; every pattern here starts + * at a short fixed token and stops at the closing quote. + * + * Comments are blanked rather than stripped because this needs OFFSETS: a + * specifier that appears inside a STRING is not an import, and the only way to + * tell is to ask the scanner whether the match sits in a literal. That is not + * hypothetical — scripts/invoked-as.mjs writes `await import(${…})` into a + * template it then writes to disk, and reading that as a computed import of + * its own would report a file the gate never loads. + * + * @param {string} source + * @returns {{specifiers: string[], computed: number}} relative specifiers, and + * how many `import(`/`require(` calls took an argument that is not a literal + */ +export function localImportSpecifiers(source) { + const { comment, literal } = scanSource(source); + const code = blank(source, comment); + const specifiers = []; + let computed = 0; + const scan = (re, onMatch) => { + re.lastIndex = 0; + for (let m; (m = re.exec(code)); ) if (!literal[m.index]) onMatch(m); + }; + // `from '…'` covers every static import and re-export; the bare form covers + // `import './x.mjs'`; the last two cover the dynamic and CJS spellings whose + // specifier IS a literal, which are as decidable as a static one. + for (const re of [ + /\bfrom\s*['"]([^'"\n]+)['"]/g, + /\bimport\s*['"]([^'"\n]+)['"]/g, + /\bimport\s*\(\s*['"]([^'"\n]+)['"]\s*\)/g, + /\brequire\s*\(\s*['"]([^'"\n]+)['"]\s*\)/g, + ]) scan(re, (m) => { if (/^\.\.?\//.test(m[1])) specifiers.push(m[1]); }); + // …and the spelling that is NOT decidable. Reported rather than passed over: + // the closure's claim is that it is complete, and a computed specifier is + // precisely the case where that claim stops being checkable. Saying so is + // the difference between a bound this file knows about and the four it did + // not (#10123 → #10458 → #10599 → #10625). + for (const re of [/\bimport\s*\(\s*([^\s'")])/g, /\brequire\s*\(\s*([^\s'")])/g]) scan(re, () => { computed += 1; }); + return { specifiers: [...new Set(specifiers)], computed }; +} + +/** + * Every file a gate reaches, gate included, guard module excluded. + * + * @param {string} gate the gate's repo-relative path + * @param {{repoRoot?: string, readFile: (file: string) => string}} options + * @returns {{files: Map, problems: string[]}} + */ +export function guardClosure(gate, { readFile }) { + const files = new Map(); + const problems = []; + const queue = [gate]; + const seen = new Set([GUARD_MODULE]); + while (queue.length > 0) { + const file = queue.shift(); + if (seen.has(file)) continue; + seen.add(file); + let src; + try { + src = readFile(file); + } catch { + problems.push( + file === gate + ? `${gate}: named by the fatal-parse guard but unreadable — renamed or removed?` + : `${gate} → ${file}: imported from this gate's closure but unreadable — renamed or moved? ` + + 'The closure is what decides which files the call bans cover, so a hole in it is a hole ' + + 'in the check (#10625).', + ); + continue; + } + files.set(file, src); + const { specifiers, computed } = localImportSpecifiers(src); + if (computed > 0) { + problems.push( + `${gate} → ${file}: ${computed} dynamic import/require with a specifier this check cannot ` + + 'resolve. The gate\'s population is its import closure, and a computed specifier is a file ' + + 'the closure cannot name — so the call bans below cannot claim to have covered it. Use a ' + + 'literal specifier, or move the code out of this gate\'s closure (#10625).', + ); + } + for (const specifier of specifiers) { + const target = resolveClosureEntry(file, specifier); + if (target === null) continue; // not code — a .json or an asset holds no call + if (target.escapes) { + problems.push( + `${gate} → ${file}: imports \`${specifier}\`, which resolves outside the repository. ` + + 'The closure stops at the repo boundary, so this file is not covered by the call bans (#10625).', + ); + continue; + } + queue.push(target.file); + } + } + return { files, problems }; +} + +/** + * One relative specifier, as a repo-relative path — or null when it cannot + * hold a call at all. + * + * @param {string} from the importing file, repo-relative + * @param {string} specifier + * @returns {{file: string, escapes: boolean}|null} + */ +function resolveClosureEntry(from, specifier) { + const extension = /(\.[a-z0-9]+)$/i.exec(specifier)?.[1]?.toLowerCase() ?? ''; + if (extension !== '' && !CODE_EXTENSIONS.includes(extension)) return null; + // Resolved against a MARKER root rather than `/`, because `path.resolve` + // clamps at the filesystem root: from `/` a `../../x.mjs` comes back as + // `/x.mjs`, so an import that walks out of the repo would read as a file at + // the top of it. Against a marker the walk-out is still visible to + // `relative()`, which is also what makes this platform-independent. + const rel = relative(CLOSURE_ROOT, resolve(CLOSURE_ROOT, dirname(from), specifier)).replace(/\\/g, '/'); + if (rel === '' || rel === '..' || rel.startsWith('../')) return { file: specifier, escapes: true }; + return { file: rel, escapes: false }; +} + +/** + * Assert every gate in GUARDED_GATES still routes through this module — and + * that nothing in the closure it reaches lints around it. * * 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. * + * The gate file gets the full verdict (import, armed, both bans). Every OTHER + * file in its closure gets the bans only, for the reasons in this file's + * header: read over the closure the import and armed tests are satisfied by + * modules that are not the gate, and one of them is satisfied by this file. + * * @param {string} repoRoot + * @param {{gates?: string[], readFile?: (file: string) => string}} [options] + * `gates` and `readFile` are injection points for the self-test, which must + * be able to drive this over synthetic trees in BOTH directions — the live + * call can only ever prove the direction today's tree is in (#10458). * @returns {string[]} problems, empty when every gate is still guarded */ -export function checkGuardAdoption(repoRoot) { +export function checkGuardAdoption(repoRoot, { gates = GUARDED_GATES, readFile } = {}) { + const read = readFile ?? ((file) => readFileSync(resolve(repoRoot, file), 'utf8')); 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; + for (const gate of gates) { + const { files, problems: closureProblems } = guardClosure(gate, { readFile: read }); + const gateSource = files.get(gate); + if (gateSource !== undefined) problems.push(...guardAdoptionProblems(gate, gateSource)); + problems.push(...closureProblems); + for (const [file, source] of files) { + if (file === gate) continue; + problems.push(...callBanProblems(`${gate} → ${file}`, stripComments(source), CLOSURE_NOTE)); } - problems.push(...guardAdoptionProblems(gate, src)); } return problems; } @@ -375,18 +621,47 @@ export function guardAdoptionProblems(gate, source) { // 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. + problems.push(...callBanProblems(gate, src)); + return problems; +} + +/** + * Why a file that is not a gate is being judged at all. Appended to a closure + * finding so the author is not left looking for the gate's name on a module + * that never appears in GUARDED_GATES. + */ +const CLOSURE_NOTE = + ' This file is not a gate. It is in the gate\'s local import closure, which is ' + + 'the population the bans cover — a measurement moved one import out is still ' + + 'this gate\'s measurement (#10625).'; + +/** + * The two call bans, over source that has already had its comments stripped. + * + * Shared by the gate verdict and the closure sweep so there is one copy of the + * rule and one copy of its wording. Two copies of a guard drift, and a drifted + * copy is invisible — the argument this whole module is built on. + * + * @param {string} subject what to name in the message: a gate, or `gate → file` + * @param {string} src comment-stripped source + * @param {string} [note] appended to each problem + * @returns {string[]} + */ +function callBanProblems(subject, src, note = '') { + const problems = []; if (/\.lintText\s*\(/.test(src)) { problems.push( - `${gate}: calls \`.lintText(\` directly. A counted lintText result discards a ` + + `${subject}: 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).', + 'with a `why` if it is not a measurement (#10599).' + note, ); } 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.', + `${subject}: calls \`.lintFiles(\` directly, so a parse failure in its population ` + + 'is discarded as a message matching no rule. Call lintFilesStrict() instead, or ' + + 'lintFilesUnguarded() with a `why` if it is not a measurement (#10625).' + note, ); } return problems; diff --git a/scripts/eslint-stack-headroom.mjs b/scripts/eslint-stack-headroom.mjs index 6c57a74337..b910e0384a 100644 --- a/scripts/eslint-stack-headroom.mjs +++ b/scripts/eslint-stack-headroom.mjs @@ -58,7 +58,7 @@ import { readFileSync } from 'node:fs'; import { resolve } from 'node:path'; import process from 'node:process'; -import { GUARDED_GATES, collectFatalMessages } from './eslint-fatal-guard.mjs'; +import { GUARDED_GATES, collectFatalMessages, lintFilesUnguarded } from './eslint-fatal-guard.mjs'; import { stripComments } from './js-comment-mask.mjs'; /** @@ -204,12 +204,25 @@ export function ensureStackHeadroom(scriptPath, args = process.argv.slice(2)) { * assertion trips BEFORE the population run starts reddening PRs — the point * of having it at all. * + * DECLARED rather than wrapped (#10625). This module sits in both guarded + * gates' import closure, so the call below is exactly the shape the adoption + * bans now cover — and it is the legitimate one: the fatals are not discarded + * here, they are the RETURN VALUE, collected by the guard's own collector and + * reported by the caller. Routing it through `lintFilesStrict()` would be + * circular and lossy both — the guard's exit path fires first and prints the + * generic report, and `formatCanaryFailure()` below is the only place a reader + * is told the remedy is `--stack-size` rather than a code fix. + * * @param {{lintFiles: (targets: string[]) => Promise}} eslint * @param {{repoRoot: string, file?: string}} options * @returns {Promise>} empty when it parsed */ export async function canaryParseFailures(eslint, { repoRoot, file = HEADROOM_CANARY_FILE }) { - const results = await eslint.lintFiles([file]); + const results = await lintFilesUnguarded(eslint, [file], { + why: 'the canary IS the parse-failure detector: these results go straight to ' + + 'collectFatalMessages() and the fatals are RETURNED, never counted as rule sites. ' + + 'lintFilesStrict() would exit before formatCanaryFailure() could name the remedy.', + }); return collectFatalMessages(results, repoRoot); }