Phase 5 of #3464, following #3465, #3466, #3502 and #3508. Those cut the
ceiling 305 -> 278, made the rule accurate, and ended file-wide amnesty. This
one fixes the number itself.
scripts/lint-allow-test-rule-refs.cjs counted FILES CONTAINING MARKER TEXT.
Only 5 of those files carry a marker that actually suppresses a violation the
rule detects, across 10 sites. The ratcheted number was ~98% noise, which is
exactly why bumping it was frictionless: the metric was never coupled to the
thing it claimed to govern. That is the whole complaint this epic opened with,
stated precisely.
Verified directly rather than assumed: only eslint-rules/no-source-grep.cjs
functionally honors the marker. Four other rule files mention allow-test-rule
in prose only, and no-raw-rmsync-in-tests.cjs:24 explicitly states it does not
apply. So the large count was not legitimately large because several rules
share the annotation.
Now two numbers, only the first ratcheted:
EFFECTIVE EXEMPTIONS -- markers that actually suppress a detected violation.
10 sites across 5 files. Tightly ratcheted in both directions, as before:
over the ceiling fails, and slack beyond grace fails.
UNVERIFIED MARKERS -- marker-bearing files with no detectable violation. 273
files. Reported and given a loose ceiling so the pool cannot silently
balloon, but deliberately NOT tightly ratcheted, because shrinking it is a
rule-coverage problem and not a delete-the-markers problem.
A file with at least one effective site counts as effective and is not also
counted as unverified; the two numbers never double-count.
Reporting ONLY the effective count was considered and rejected. It would say
five files and look excellent while being falsely reassuring, because "no
detectable violation" is not "no violation". This phase's own measurement found
two genuine source-greps that are unsuppressed AND undetected --
tests/security-prompt-injection.security.test.cjs:852 and
tests/check-update-config-dir.test.cjs:91 -- each reading a real shipped file
and text-searching it, invisible only because the path is bound to a separate
const the rule never resolves back to its literal. Markers guarding that class
count as zero-effective and would look vestigial. Trading a number that is too
big and meaningless for one that is too small and falsely reassuring is not
progress, so the script prints the known-limit caveat alongside the numbers and
the two undetected violations are filed separately rather than lost.
Single source of truth is structural, not a matter of discipline. The script
does not re-implement detection or the site-scoped adjacency predicate -- that
is the generative-fix-divergence class this repo has shipped before. The rule
now exports MAX_MARKER_LOOKAHEAD_LINES, MARKER_COMMENT_RE,
collectMarkerAndCommentLines and isSuppressedAt (extracted verbatim, no logic
change), plus a default-off neutralizeSuppression option so the counter can
enumerate every site through the real rule via ESLint's Linter API and then
classify each with the rule's own predicate, replicating reportUnlessSuppressed's
search-line-OR-read-line check exactly. Default rule behavior is byte-identical:
tests/eslint-rules.test.cjs passes 168/168 unchanged. A parity test asserts the
script's suppressed/not verdict equals the rule's own report/no-report outcome
for every site in a fixture corpus.
A real silent-failure bug surfaced and was fixed while building this: ESLint's
flat-config Linter reports "No matching configuration" and returns ZERO messages
for any filename resolving outside its cwd. That would have quietly
misclassified every sandboxed test fixture as having no violations -- a test
suite that passes while asserting nothing. Fixed by anchoring the Linter to the
tests dir, with a defensive throw if it ever recurs.
Two earlier claims of mine are corrected by this phase's measurement. Widening
the source-dir allowlist to include hooks/ -- the "fifth blind spot" recorded in
#3508 -- rescues ZERO sites; it is real in principle and has no practical
effect, because the hooks reads that exist are missed for other reasons
(.sh extension, identifier-indirection, dynamic filenames). And the #3508
correction that attributed those reads to the hooks/ gap rather than to variable
indirection was itself incomplete: both are independently sufficient, so fixing
either alone changes nothing. I accepted the reviewer's causal claim as
uncritically as I had made my own.
The unverified count is 273, not the ~289 in the phase design doc. That is
legitimate drift -- the baseline was measured at fba7c9032 and other merged work
has since removed markers. Left as measured rather than adjusted to match the
document.
Adversarial review found a BLOCKER in the first revision and it is fixed here.
The counter walked only tests/**/*.test.cjs, but the rule is registered on
tests/**/*.cjs -- every .cjs, not just test files -- plus scripts/**,
eslint-rules/**, bin/lib/**, pi/**, examples/**, gsd-core/bin/** and three
plugin globs. 35 non-.test.cjs files under tests/ were never walked, and
tests/helpers/live-command-registry.cjs:1 carries a real marker that appeared in
NEITHER reported number. A counter that undercounts is worse than the
meaningless one it replaces, because it will be trusted.
The scan set is now derived programmatically: the script dynamically imports
eslint.config.mjs and extracts the `files` globs from every config block that
enables local/no-source-grep. There is no hardcoded list to drift, which is the
same divergence class the parity test already guards.
Two further defects surfaced while fixing it. The silent-clean catch around
linter.verify() was worse than reported -- ESLint signals a parse error by
returning a message with fatal:true rather than throwing, so the original catch
would not even have fired for the common case. Both paths now throw with the
file path. That silent swallow was actively hiding a broken fixture in this
suite's own tests: 'no marker here\n' is not valid JS and the case only
"passed" because the parse failure was discarded. Fixed.
And once the scan widened, the raw-text marker scanner started matching this
tooling's own doc comments and RuleTester fixture strings, so marker extraction
moved to AST comment nodes. That corrected three long-standing FALSE POSITIVES:
allowlist entries for tests/eslint-rules.test.cjs that were never real markers,
only fixture payload. Allowlist 137 -> 135: three false positives pruned, one
real entry added for live-command-registry.
The headline numbers are coincidentally unchanged (10/10 effective across 5
files, 273/280 unverified) but the composition is corrected: one real file
gained, one phantom dropped. Verified by direct diff rather than inferred from
the totals matching.
The first remote run of this branch came back RED with 23 failures, all one
cause, and it is fixed here. Classification drove ESLint's flat-config Linter,
which resolves configuration relative to a cwd and refuses to lint any file
outside it. The test harness writes fixtures into an OS temp dir, so every
sandbox row hit "No matching configuration found" and tripped the defensive
throw. It surfaced only on the container because the repo lives at /work there
and the fixtures at /tmp, making the mismatch unmissable; a local run had
reported the suite green, which it was not.
Fixed by not depending on config *resolution* at all: the script now builds an
eslintrc-format Linter and registers the rule directly with defineRule, since it
already knows exactly which rule and options it wants. That removes the
cwd-ancestor constraint entirely and makes repo files and out-of-tree fixtures
classify identically. Verified out-of-tree explicitly, not just in-repo, because
the local temp path shape is what hid the bug the first time. Parse failures
still throw loudly -- that behavior is required and tested. Classifications are
unchanged for real repo files (10/10 effective across 5, 273/280 unverified,
0 live), which is the check that the config swap did not quietly alter results.
The second remote run cut the failures from 23 to 2, and the survivors were a
genuinely different and more interesting defect:
tests/packaging-shipped-scripts-require-only-shipped.test.cjs caught that
scripts/ SHIPS in the published package while eslint-rules/ does not, so
importing the rule for single-source-of-truth would MODULE_NOT_FOUND in a real
consumer's install. That test statically extracts require() calls, so hiding the
import inside a function would have dodged the check without fixing the problem.
Resolved along the grain of existing convention rather than by weakening
anything: package.json already excludes several repo-internal lint gates from
the shipped set via `!scripts/...`, including
`!scripts/lint-no-adhoc-regex-escape.cjs`, which is the same situation. This
gate is CI-only and has no meaning in a consumer install, so it joins them.
Verified with `npm pack --dry-run` that the .cjs is genuinely absent from the
tarball (its inert JSON config files remain, and carry no requires).
The third remote run failed on shard 3/3 across all three OSes with exit code
NULL and empty output -- the spawned gate was killed by a timeout, not failing
an assertion. Cause: replacing a raw text scan with a full ESLint Linter pass
over every file in every registered glob took the gate from 0.57s to ~7-12s,
and GitHub's runners are slower than the bench that had just passed it green.
Fixed by narrowing the work rather than raising the timeout to hide it. Both
numbers the gate computes are properties of files that CONTAIN a marker, so
only those (~294) need linting; the repo-wide byte walk that finds them stays,
since that was the undercount fix. Live violations in files carrying NO marker
are already enforced by npm run lint over exactly these globs, so re-detecting
them here was redundant. 2.87s now, from ~7s measured locally.
That narrowing changes what one reported number means, so the wording changed
with it: "live violations in marker-bearing files: 0 (unmarked files are
enforced separately by npm run lint)". A number that quietly covers less than
it reads is the failure this whole epic is about, so it is stated rather than
left implicit, and the test row that asserted the old broader behavior was
split -- an unmarked live violation now passes this gate (and is caught by
eslint), while a live violation in a marker-bearing file still fails it.
The repo-baseline test's timeout was also raised to 30s with a comment, since a
gate that legitimately takes ~3s must not sit at a timeout close to its own
runtime. Every other row keeps the shorter sandbox-scoped timeout.
Closes #3520
Co-authored-by: sim <sim@local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
@@ -45,6 +45,95 @@ const MAX_TRANSITIVE_HOPS = 3;
|
||||
// site 40+ lines later (test-matrix.md row 4, the defect this closes).
|
||||
const MAX_MARKER_LOOKAHEAD_LINES = 8;
|
||||
|
||||
// Matches a `allow-test-rule: <reason>` directive inside a comment's VALUE
|
||||
// (the AST comment node's text with delimiters stripped -- i.e. this is
|
||||
// tested against `c.value`, never against raw source text). Exported so
|
||||
// external consumers (scripts/lint-allow-test-rule-refs.cjs) that need to
|
||||
// find marker comments via the SAME AST-comment definition the rule itself
|
||||
// honors can import this instead of hand-rolling an equivalent pattern that
|
||||
// could silently drift from what the rule actually recognizes.
|
||||
const MARKER_COMMENT_RE = /allow-test-rule:\s*\S/;
|
||||
|
||||
/**
|
||||
* Given every comment in a file (`sourceCode.getAllComments()`), compute the
|
||||
* two line-level facts `isSuppressedAt` needs: which lines carry a
|
||||
* `allow-test-rule:` marker, and which lines are wholly comment (used for the
|
||||
* "nothing but blank/comment lines between the marker and the violation"
|
||||
* purity check). Extracted verbatim from the per-file computation `create()`
|
||||
* used to do inline, so a second consumer (the effective-exemption counter)
|
||||
* can derive the identical inputs without re-deriving the marker-detection
|
||||
* logic itself.
|
||||
*
|
||||
* @param {{value: string, loc: {start: {line:number}, end: {line:number}}}[]} allComments
|
||||
* @returns {{markerLines: number[], commentLineSet: Set<number>}}
|
||||
*/
|
||||
function collectMarkerAndCommentLines(allComments) {
|
||||
const markerLines = [];
|
||||
const commentLineSet = new Set();
|
||||
for (const c of allComments) {
|
||||
for (let l = c.loc.start.line; l <= c.loc.end.line; l++) {
|
||||
commentLineSet.add(l);
|
||||
}
|
||||
if (MARKER_COMMENT_RE.test(c.value)) {
|
||||
for (let l = c.loc.start.line; l <= c.loc.end.line; l++) {
|
||||
markerLines.push(l);
|
||||
}
|
||||
}
|
||||
}
|
||||
return { markerLines, commentLineSet };
|
||||
}
|
||||
|
||||
/**
|
||||
* Site-scoped suppression predicate: is `violationLine` suppressed by any of
|
||||
* `markerLines`? Extracted verbatim from the logic `create()` used to close
|
||||
* over directly (previously named `isSuppressed`), generalized to take its
|
||||
* per-file inputs as parameters instead of reading them off closure state, so
|
||||
* a second consumer (scripts/lint-allow-test-rule-refs.cjs, the
|
||||
* effective-exemption counter) can call the EXACT SAME adjacency arithmetic
|
||||
* the rule uses at report time -- rather than reimplementing it, which is the
|
||||
* generative-fix-divergence defect class this repo has shipped before.
|
||||
*
|
||||
* A violation at `violationLine` is suppressed if some marker sits on that
|
||||
* exact line (trailing form) or on an earlier line within `maxLookahead`,
|
||||
* with every line strictly between the marker and the violation being blank
|
||||
* and/or itself a comment line -- i.e. no live code sits between the marker
|
||||
* and the call it suppresses. This is what makes suppression SITE-scoped
|
||||
* rather than file-wide.
|
||||
*
|
||||
* @param {object} opts
|
||||
* @param {number[]} opts.markerLines - line numbers carrying a marker.
|
||||
* @param {number} opts.violationLine - the candidate violation's line.
|
||||
* @param {Set<number>} opts.commentLineSet - lines wholly occupied by a comment.
|
||||
* @param {string[]} opts.lines - the file's source lines (sourceCode.lines).
|
||||
* @param {number} [opts.maxLookahead] - defaults to MAX_MARKER_LOOKAHEAD_LINES.
|
||||
* @returns {boolean}
|
||||
*/
|
||||
function isSuppressedAt({
|
||||
markerLines,
|
||||
violationLine,
|
||||
commentLineSet,
|
||||
lines,
|
||||
maxLookahead = MAX_MARKER_LOOKAHEAD_LINES,
|
||||
}) {
|
||||
function isBlankLine(line) {
|
||||
const text = lines[line - 1];
|
||||
return text !== undefined && text.trim() === '';
|
||||
}
|
||||
for (const markerLine of markerLines) {
|
||||
if (markerLine > violationLine) continue;
|
||||
if (violationLine - markerLine > maxLookahead) continue;
|
||||
let pure = true;
|
||||
for (let l = markerLine + 1; l < violationLine; l++) {
|
||||
if (!isBlankLine(l) && !commentLineSet.has(l)) {
|
||||
pure = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (pure) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
const TEXT_METHODS = new Set([
|
||||
'includes',
|
||||
'match',
|
||||
@@ -120,10 +209,32 @@ const rule = {
|
||||
'Disallow reading source .cjs/.cts/.js/.mjs/.mts/.ts files with readFileSync and then doing text search on the result',
|
||||
category: 'Best Practices',
|
||||
},
|
||||
schema: [],
|
||||
// `neutralizeSuppression` is a diagnostic-only knob for
|
||||
// scripts/lint-allow-test-rule-refs.cjs (the effective-exemption
|
||||
// counter): when true, every candidate violation is reported regardless
|
||||
// of a marker, so the script can enumerate the FULL site inventory via
|
||||
// one real ESLint pass, then classify each site with isSuppressedAt
|
||||
// (exported below) against the real markers. No config in this repo
|
||||
// passes this option, so default (real) linting is unaffected -- this is
|
||||
// an extract-and-export refactor of existing logic, not a behavior
|
||||
// change to `no-source-grep` itself.
|
||||
schema: [
|
||||
{
|
||||
type: 'object',
|
||||
properties: {
|
||||
neutralizeSuppression: { type: 'boolean' },
|
||||
},
|
||||
additionalProperties: false,
|
||||
},
|
||||
],
|
||||
messages: {
|
||||
noSourceGrep:
|
||||
'Source-grep test: do not read source .cjs/.cts/.js/.mjs/.mts/.ts files with readFileSync and call .includes/.match/.matchAll/.startsWith/.indexOf/.split/.replace/.search (or regex.test()) on the result. Use require() to run the module instead. Add // allow-test-rule: <reason> (#NNN) directly above (or trailing) the flagged line to suppress just that site.',
|
||||
// Diagnostic-only companion to `noSourceGrep`, emitted ONLY when the
|
||||
// `neutralizeSuppression` schema option is set (see its doc comment
|
||||
// and `reportUnlessSuppressed` above) -- never fires with the real
|
||||
// (shipped) config, so this never appears in real lint output.
|
||||
noSourceGrepDiagnosticReadLine: '{{readLine}}',
|
||||
},
|
||||
},
|
||||
create(context) {
|
||||
@@ -136,34 +247,18 @@ const rule = {
|
||||
const allComments = sourceCode.getAllComments();
|
||||
|
||||
// Line numbers of every `// allow-test-rule: <reason>` marker comment in
|
||||
// the file. A marker may span one line (the normal `//` form) or several
|
||||
// (a block comment) -- record every line it occupies so a violation on
|
||||
// any of those lines counts as "same line" (trailing-marker form, row 2
|
||||
// of the test matrix).
|
||||
const markerLines = [];
|
||||
for (const c of allComments) {
|
||||
if (/allow-test-rule:\s*\S/.test(c.value)) {
|
||||
for (let l = c.loc.start.line; l <= c.loc.end.line; l++) {
|
||||
markerLines.push(l);
|
||||
}
|
||||
}
|
||||
}
|
||||
// the file (a marker may span one line or several -- see
|
||||
// collectMarkerAndCommentLines' doc comment above), and the set of lines
|
||||
// fully occupied by ANY comment (used by isSuppressedAt's purity check).
|
||||
const { markerLines, commentLineSet } = collectMarkerAndCommentLines(allComments);
|
||||
|
||||
// Line numbers fully occupied by ANY comment (marker or not) -- a marker
|
||||
// followed by ordinary prose lines before the flagged call is the repo's
|
||||
// real style (test-matrix.md row 3), so those in-between lines must not
|
||||
// disqualify the marker.
|
||||
const commentLineSet = new Set();
|
||||
for (const c of allComments) {
|
||||
for (let l = c.loc.start.line; l <= c.loc.end.line; l++) {
|
||||
commentLineSet.add(l);
|
||||
}
|
||||
}
|
||||
|
||||
function isBlankLine(line) {
|
||||
const text = sourceCode.lines[line - 1];
|
||||
return text !== undefined && text.trim() === '';
|
||||
}
|
||||
// Diagnostic-only: see the `neutralizeSuppression` schema option doc
|
||||
// comment above. Never true for any config in this repo.
|
||||
const neutralizeSuppression = !!(
|
||||
context.options &&
|
||||
context.options[0] &&
|
||||
context.options[0].neutralizeSuppression
|
||||
);
|
||||
|
||||
// A violation at `violationLine` is suppressed if some marker sits on
|
||||
// that exact line (trailing form) or on an earlier line within
|
||||
@@ -173,21 +268,17 @@ const rule = {
|
||||
// ostensibly about) sits between the marker and the call it suppresses.
|
||||
// This is what makes suppression SITE-scoped rather than file-wide: a
|
||||
// marker parked far above an unrelated later violation (test-matrix.md
|
||||
// row 4) no longer reaches it.
|
||||
// row 4) no longer reaches it. Delegates to the exported isSuppressedAt
|
||||
// predicate (see its doc comment) rather than duplicating the adjacency
|
||||
// arithmetic here.
|
||||
function isSuppressed(violationLine) {
|
||||
for (const markerLine of markerLines) {
|
||||
if (markerLine > violationLine) continue;
|
||||
if (violationLine - markerLine > MAX_MARKER_LOOKAHEAD_LINES) continue;
|
||||
let pure = true;
|
||||
for (let l = markerLine + 1; l < violationLine; l++) {
|
||||
if (!isBlankLine(l) && !commentLineSet.has(l)) {
|
||||
pure = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (pure) return true;
|
||||
}
|
||||
return false;
|
||||
if (neutralizeSuppression) return false;
|
||||
return isSuppressedAt({
|
||||
markerLines,
|
||||
violationLine,
|
||||
commentLineSet,
|
||||
lines: sourceCode.lines,
|
||||
});
|
||||
}
|
||||
|
||||
// Map from Identifier AST node -> resolved ESLint `Variable`, built once
|
||||
@@ -548,6 +639,25 @@ const rule = {
|
||||
// site-scoped, way to annotate it). `readLine` is optional -- pass
|
||||
// it whenever the call site can determine one.
|
||||
function reportUnlessSuppressed(node, readLine) {
|
||||
if (neutralizeSuppression) {
|
||||
// Diagnostic-only mode (see the `neutralizeSuppression` schema
|
||||
// option doc comment): report every candidate site regardless of
|
||||
// suppression, PLUS a paired companion message at the exact same
|
||||
// node carrying `readLine` -- the other half of the read+search
|
||||
// pair that the real (non-neutralized) suppression check above
|
||||
// also consults. This lets a consumer (the effective-exemption
|
||||
// counter) replicate this rule's own OR-of-two-lines suppression
|
||||
// decision from the OUTSIDE via `isSuppressedAt` without this
|
||||
// rule re-exposing its internal hop/scope-resolution machinery.
|
||||
// Never fires with the real (shipped) config.
|
||||
context.report({ node, messageId: 'noSourceGrep' });
|
||||
context.report({
|
||||
node,
|
||||
messageId: 'noSourceGrepDiagnosticReadLine',
|
||||
data: { readLine: readLine === undefined || readLine === null ? '' : String(readLine) },
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (isSuppressed(node.loc.start.line)) return;
|
||||
if (readLine !== undefined && readLine !== null && isSuppressed(readLine)) {
|
||||
return;
|
||||
@@ -596,3 +706,14 @@ const rule = {
|
||||
};
|
||||
|
||||
module.exports = rule;
|
||||
// Named exports consumed by scripts/lint-allow-test-rule-refs.cjs (the
|
||||
// effective-exemption counter) and its tests -- ESLint itself only reads
|
||||
// `.create`/`.meta` off this module, so these extra properties are inert to
|
||||
// ESLint and exist purely as the single source of truth for anything that
|
||||
// needs to reason about marker suppression outside the rule's own
|
||||
// Program:exit walk. See each function's doc comment above for why this
|
||||
// extraction exists (generative-fix-divergence prevention).
|
||||
module.exports.MAX_MARKER_LOOKAHEAD_LINES = MAX_MARKER_LOOKAHEAD_LINES;
|
||||
module.exports.MARKER_COMMENT_RE = MARKER_COMMENT_RE;
|
||||
module.exports.collectMarkerAndCommentLines = collectMarkerAndCommentLines;
|
||||
module.exports.isSuppressedAt = isSuppressedAt;
|
||||
|
||||
@@ -28,6 +28,7 @@
|
||||
"!scripts/affected-tests-lib.cjs",
|
||||
"!scripts/run-affected-tests.cjs",
|
||||
"!scripts/lint-no-adhoc-regex-escape.cjs",
|
||||
"!scripts/lint-allow-test-rule-refs.cjs",
|
||||
"pi",
|
||||
"vscode"
|
||||
],
|
||||
|
||||
@@ -43,9 +43,6 @@
|
||||
"tests/edge-probe-planner-contract.test.cjs :: source-text-is-the-product",
|
||||
"tests/edge-probe-spec-phase-contract.test.cjs :: source-text-is-the-product",
|
||||
"tests/edit-phase.test.cjs :: source-text-is-the-product",
|
||||
"tests/eslint-rules.test.cjs :: <source-grep reason> must still error",
|
||||
"tests/eslint-rules.test.cjs :: pending migration",
|
||||
"tests/eslint-rules.test.cjs :: source-text-is-the-product",
|
||||
"tests/execute-phase-active-flags.test.cjs :: source-text-is-the-product",
|
||||
"tests/execute-phase-step-5-5-deviation-doc.test.cjs :: source-text-is-the-product",
|
||||
"tests/execute-phase-wave.test.cjs :: source-text-is-the-product",
|
||||
@@ -60,6 +57,7 @@
|
||||
"tests/gsd-researcher-app-aware.test.cjs :: source-text-is-the-product",
|
||||
"tests/gsd-researcher-flow-diagram.test.cjs :: source-text-is-the-product",
|
||||
"tests/gsd-settings-advanced.test.cjs :: source-text-is-the-product",
|
||||
"tests/helpers/live-command-registry.cjs :: source-text-is-the-product",
|
||||
"tests/hermes-skills-migration.test.cjs :: source-text-is-the-product",
|
||||
"tests/import-command.test.cjs :: source-text-is-the-product",
|
||||
"tests/ingest-docs.test.cjs :: source-text-is-the-product",
|
||||
|
||||
@@ -1,4 +0,0 @@
|
||||
{
|
||||
"maxFiles": 278,
|
||||
"grace": 3
|
||||
}
|
||||
@@ -3,23 +3,16 @@
|
||||
|
||||
/**
|
||||
* lint-allow-test-rule-refs.cjs — enforce that NEW `allow-test-rule:` exemption
|
||||
* comments carry a tracking-issue reference, AND that the total exemption
|
||||
* count only ever ratchets down.
|
||||
* comments carry a tracking-issue reference, AND that the number of exemptions
|
||||
* that ACTUALLY suppress a detected violation only ever ratchets down.
|
||||
*
|
||||
* ## Why
|
||||
*
|
||||
* `allow-test-rule:` is an inline comment that disables the `no-source-grep`
|
||||
* ESLint rule for a whole test file. Today many such comments exist with no
|
||||
* issue reference, making it impossible to audit or revisit them. Per ADR-456
|
||||
* ESLint rule at a specific site. Per ADR-456
|
||||
* (docs/adr/456-test-rigor-architecture.md) every NEW exemption must carry a
|
||||
* `#NNN` issue reference or an https:// URL so the decision is traceable.
|
||||
*
|
||||
* A citation alone doesn't stop the raw count from growing forever — a cited
|
||||
* exemption is legitimate under ADR-456 §(d), but nothing previously stopped
|
||||
* the total (cited + uncited) from creeping up PR by PR. This is the same
|
||||
* masking gap the citation ratchet fixes for identity, applied to the count:
|
||||
* a cited exemption looks compliant while the aggregate debt keeps growing.
|
||||
*
|
||||
* ## What "compliant" means (citation check)
|
||||
*
|
||||
* A compliant `allow-test-rule:` comment is one whose reason text (everything
|
||||
@@ -48,75 +41,365 @@
|
||||
* If a file has multiple non-compliant comments with the SAME reason text, only
|
||||
* one identifier is recorded (deduped via Set).
|
||||
*
|
||||
* ## Total-count ceiling
|
||||
* ## Effective exemptions vs. unverified markers (#3520 / epic #3464 phase 5)
|
||||
*
|
||||
* Independently of citation status, the number of DISTINCT files carrying at
|
||||
* least one `allow-test-rule:` marker is checked against a tight ceiling in
|
||||
* scripts/lint-allow-test-rule-refs.ceiling.json via `assertTightCeiling`
|
||||
* (scripts/lib/allowlist-ratchet.cjs). The ceiling may only decrease — a
|
||||
* shrinking count that leaves too much slack above the ceiling fails the gate
|
||||
* just as much as growth past it, forcing the ceiling to track the real
|
||||
* high-water mark rather than sitting stale and loose.
|
||||
* A marker-bearing FILE is not the same thing as a marker that suppresses
|
||||
* anything. Historically this gate counted "distinct files containing marker
|
||||
* text" (294 at one baseline), but only a handful of those markers actually
|
||||
* sit next to a violation the `no-source-grep` rule can detect — the rest are
|
||||
* "counted" only because they contain the literal substring, which made the
|
||||
* old ceiling ~98% noise and free to bump.
|
||||
*
|
||||
* This gate now reports and ratchets TWO separate numbers instead:
|
||||
*
|
||||
* 1. **Effective exemption SITES** — a site (one flagged read+search pair)
|
||||
* where a marker is adjacent enough (per the rule's own
|
||||
* `isSuppressedAt` site-scoped adjacency predicate) to suppress a
|
||||
* violation the rule actually detects there. Tightly ratcheted via
|
||||
* `assertTightCeiling` (both directions — growth past ceiling fails,
|
||||
* and slack beyond `grace` fails, forcing the ceiling to track the
|
||||
* real high-water mark).
|
||||
*
|
||||
* 2. **Unverified marker-bearing FILES** — files that carry an
|
||||
* `allow-test-rule:` marker but for which no detectable violation was
|
||||
* found adjacent to any marker in the file (rule-coverage gap, not
|
||||
* necessarily a vestigial marker — see "Known limits" below). Reported
|
||||
* with its own ceiling so it cannot silently balloon, but NOT tightly
|
||||
* ratcheted: dropping below the ceiling never fails the gate, because
|
||||
* shrinking this pool means widening rule coverage file-by-file with
|
||||
* evidence, not deleting markers on the strength of this metric alone.
|
||||
*
|
||||
* A file with >=1 effective site counts ONLY as effective, never also as
|
||||
* unverified (test-matrix.md #3520 row 5) — no double counting.
|
||||
*
|
||||
* ## Scan scope: every glob that registers the rule, not just `tests/*.test.cjs`
|
||||
*
|
||||
* `local/no-source-grep` is registered by `eslint.config.mjs` on several
|
||||
* glob blocks, not only `tests/** /*.cjs`: also `scripts/** /*.cjs`,
|
||||
* `eslint-rules/** /*.cjs`, `bin/lib/** /*.cjs`, `pi/** /*.cjs`,
|
||||
* `examples/** /*.cjs`, `gsd-core/bin/** /*.cjs`, `vscode/*.js`,
|
||||
* `.kilo/plugins/*.js`, and `.opencode/plugins/*.js`. A prior version of
|
||||
* this script only walked `tests/** /*.test.cjs`, which silently dropped
|
||||
* every non-`.test.cjs` file under `tests/` AND every file under every one
|
||||
* of those other blocks from BOTH reported numbers — a marker there would
|
||||
* vanish rather than trip a ceiling. `deriveNoSourceGrepGlobs` (below) reads
|
||||
* the authoritative glob list directly out of `eslint.config.mjs` at run
|
||||
* time (dynamic `import()`, since this is a `.cjs` script and the config is
|
||||
* ESM) so the scan set cannot drift from what ESLint itself actually lints.
|
||||
*
|
||||
* ## Single source of truth for detection
|
||||
*
|
||||
* This script does NOT reimplement the `no-source-grep` rule's AST walk or
|
||||
* its site-scoped adjacency predicate — doing so would be the
|
||||
* generative-fix-divergence defect class this repo has shipped before.
|
||||
* Instead it drives the REAL rule through ESLint's `Linter` API with its
|
||||
* `neutralizeSuppression` diagnostic option (see the rule's `meta.schema`
|
||||
* doc comment) to enumerate every candidate violation SITE in a file
|
||||
* regardless of markers, then classifies each site by calling the rule's own
|
||||
* exported `isSuppressedAt` predicate against that file's real markers (also
|
||||
* obtained via the rule's exported `collectMarkerAndCommentLines`) — the
|
||||
* exact function the rule itself calls at report time. See
|
||||
* `eslint-rules/no-source-grep.cjs`.
|
||||
*
|
||||
* ## Known limits (do not read "effective" as "all exemptions")
|
||||
*
|
||||
* The effective-sites count is only as good as `no-source-grep`'s own
|
||||
* detection. Real source-text-search violations behind identifier
|
||||
* indirection (a path bound to a separate `const` the rule never resolves
|
||||
* back to its literal), dynamic paths, non-`.js`-family extensions (`.sh`),
|
||||
* and array/object round-trips are documented, accepted blind spots of the
|
||||
* rule (see its own "Known limits" in 40-design.md) and remain invisible to
|
||||
* BOTH numbers here. Two such sites are known and tracked separately:
|
||||
* tests/security-prompt-injection.security.test.cjs and
|
||||
* tests/check-update-config-dir.test.cjs. This is stated explicitly in this
|
||||
* script's own `ok` output so the effective count can never be read as "all
|
||||
* exemptions are accounted for."
|
||||
*
|
||||
* ## Linter scope: marker-bearing files only (#3464 perf follow-up)
|
||||
*
|
||||
* The byte-based `walkGlobs` walk above still reads EVERY file in every
|
||||
* `local/no-source-grep`-registered glob (~1200 files) — that walk is cheap
|
||||
* (raw `fs.readFileSync`, no parsing) and stays repo-wide so no marker can
|
||||
* hide from the inventory. But driving the real ESLint `Linter` over all
|
||||
* ~1200 files to classify effective/live sites was the actual cost (~12s+
|
||||
* under CI load, enough to blow the test harness's spawn timeout). Only
|
||||
* files containing the literal `allow-test-rule:` substring can possibly
|
||||
* produce a genuine directive (`extractGenuineMarkerReasons` requires that
|
||||
* substring to be present) or an "effective"/"unverified" classification —
|
||||
* so `main` filters to marker-bearing files (a plain `String#includes` scan
|
||||
* over content already in memory from the walk, deliberately NOT a shell
|
||||
* `grep`: `tests/security-prompt-injection.security.test.cjs` contains a
|
||||
* literal NUL byte that a shell grep pipeline handles differently than
|
||||
* Node's own string/buffer handling does) BEFORE calling `classifySites`,
|
||||
* narrowing the Linter pass from ~1200 files to the ~294 that actually carry
|
||||
* a marker.
|
||||
*
|
||||
* This means the "live violations" figure this script reports now only
|
||||
* covers marker-bearing files. A live (unsuppressed) violation in a file
|
||||
* with NO marker at all is still caught — just not by this script: it is
|
||||
* exactly what `local/no-source-grep` itself flags when `npm run lint` /
|
||||
* `npm run lint:ci` runs across the same globs, and that run fails the build
|
||||
* independently. This script re-checking unmarked files for live violations
|
||||
* would be pure duplicate work with no distinct consequence (the marker
|
||||
* inventory and the citation/ratchet checks — the reason this script exists
|
||||
* — only ever concern marker-bearing files in the first place). If a live
|
||||
* violation IS found in a marker-bearing file, it still fails this gate
|
||||
* exactly as before this change.
|
||||
*
|
||||
* See docs/adr/456-test-rigor-architecture.md for the full policy.
|
||||
*/
|
||||
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const { pathToFileURL } = require('url');
|
||||
const { Linter } = require('eslint');
|
||||
const { assertWithinAllowlist, assertTightCeiling } = require('./lib/allowlist-ratchet.cjs');
|
||||
const { ExitError, runMain } = require('./lib/cli-exit.cjs');
|
||||
const noSourceGrepRule = require('../eslint-rules/no-source-grep.cjs');
|
||||
|
||||
const ROOT = path.join(__dirname, '..');
|
||||
const ESLINT_CONFIG_PATH = path.join(ROOT, 'eslint.config.mjs');
|
||||
const TESTS_DIR = process.env.GSD_LINT_ALLOW_TEST_RULE_TESTS_DIR || path.join(ROOT, 'tests');
|
||||
// Overrides the ROOT that non-`tests/` glob base directories (scripts/,
|
||||
// eslint-rules/, bin/lib/, pi/, examples/, gsd-core/bin/, vscode/,
|
||||
// .kilo/plugins/, .opencode/plugins/ — see deriveNoSourceGrepGlobs) are
|
||||
// resolved under. TESTS_DIR already has its own override (above) for the
|
||||
// `tests/**/*.cjs` block; this is the equivalent knob for every OTHER block
|
||||
// that registers `local/no-source-grep`, so sandbox fixtures can exercise
|
||||
// the widened scan scope (#3464 phase 5 BLOCKER fix) without the test suite
|
||||
// re-scanning this repo's real scripts/eslint-rules/etc. trees on every run.
|
||||
const EXTRA_ROOT = process.env.GSD_LINT_ALLOW_TEST_RULE_EXTRA_ROOT || ROOT;
|
||||
const ALLOWLIST_PATH =
|
||||
process.env.GSD_LINT_ALLOW_TEST_RULE_ALLOWLIST ||
|
||||
path.join(__dirname, 'lint-allow-test-rule-refs.allowlist.json');
|
||||
const CEILING_PATH =
|
||||
process.env.GSD_LINT_ALLOW_TEST_RULE_CEILING ||
|
||||
path.join(__dirname, 'lint-allow-test-rule-refs.ceiling.json');
|
||||
const EFFECTIVE_CEILING_PATH =
|
||||
process.env.GSD_LINT_ALLOW_TEST_RULE_EFFECTIVE_CEILING ||
|
||||
path.join(__dirname, 'lint-allow-test-rule-refs.effective-ceiling.json');
|
||||
const UNVERIFIED_CEILING_PATH =
|
||||
process.env.GSD_LINT_ALLOW_TEST_RULE_UNVERIFIED_CEILING ||
|
||||
path.join(__dirname, 'lint-allow-test-rule-refs.unverified-ceiling.json');
|
||||
|
||||
/**
|
||||
* Extracts the reason text after `allow-test-rule:` from a single line of source
|
||||
* text in any comment form that the no-source-grep ESLint rule honours.
|
||||
*
|
||||
* The ESLint rule tests `c.value` (AST comment node value, delimiters stripped)
|
||||
* with /allow-test-rule:\s*\S/, which fires on BOTH:
|
||||
* // allow-test-rule: <reason> (line comment)
|
||||
* /* allow-test-rule: <reason> * / (block comment, single-line)
|
||||
*
|
||||
* By scanning line-by-line and extracting everything after `allow-test-rule:` on
|
||||
* each line, we cover both forms without a cross-line regex (which was previously
|
||||
* matching arbitrary `/* ... * /` pairs spanning hundreds of lines, causing false
|
||||
* positives).
|
||||
*
|
||||
* The trailing `*\/` and whitespace are stripped so block-comment closers don't
|
||||
* bleed into the extracted reason.
|
||||
*/
|
||||
const ALLOW_TEST_RULE_LINE_RE = /allow-test-rule:\s*(.+)/;
|
||||
/** Matches a compliant issue reference or URL */
|
||||
const ISSUE_REF_RE = /#\d+|https?:\/\//;
|
||||
|
||||
/**
|
||||
* Recursively read every *.test.cjs file under dir, once.
|
||||
* Extract the "genuine directive" reason text(s) from a file's real AST
|
||||
* comments (never string/template literals, never source code — only actual
|
||||
* `//` or `/* * /` comment tokens, obtained the same way the rule itself does
|
||||
* via `sourceCode.getAllComments()`).
|
||||
*
|
||||
* Both the citation check and the total-count ceiling need the same file set
|
||||
* and content — walking twice would be the generative-fix-divergence class of
|
||||
* bug (two scans that can silently drift apart), so both classifiers below
|
||||
* consume this single walk's output.
|
||||
* A prior version of this scanned raw file TEXT line-by-line for the
|
||||
* substring `allow-test-rule:` anywhere on a line. That was safe while the
|
||||
* scan was confined to `tests/*.test.cjs` (real test files essentially never
|
||||
* discuss the marker syntax in prose), but widening the scan to
|
||||
* `scripts/** /*.cjs` and `eslint-rules/** /*.cjs` (#3464 phase 5 BLOCKER fix)
|
||||
* means it now also walks the RULE's own implementation and THIS script —
|
||||
* files that must document `// allow-test-rule: <reason> (#NNN)` as example
|
||||
* syntax in prose, JSDoc, and the rule's own message string. A raw
|
||||
* substring scan cannot tell "a real directive" apart from "a sentence that
|
||||
* mentions the directive" or "a string literal that quotes it" — so a
|
||||
* directly-widened raw scan produced dozens of false "uncited exemption"
|
||||
* failures on this repo's own documentation.
|
||||
*
|
||||
* @param {string} dir absolute path to scan
|
||||
* @returns {{relpath: string, content: string}[]}
|
||||
* Fixed here by requiring a match to be BOTH (a) inside a real comment token
|
||||
* (excludes string/template literals and code entirely — mirrors row 11's
|
||||
* "marker text inside a string literal is not a directive" contract) AND
|
||||
* (b) the FIRST non-whitespace content of that comment's line, once an
|
||||
* optional JSDoc continuation-line `*` prefix is stripped (excludes prose
|
||||
* that merely MENTIONS `allow-test-rule:` mid-sentence, e.g. "Matches a
|
||||
* `allow-test-rule: <reason>` directive..."). This is deliberately STRICTER
|
||||
* than the rule's own `MARKER_COMMENT_RE` (which matches the substring
|
||||
* anywhere in a comment's value, by design, so a marker's suppression reach
|
||||
* cannot be defeated by trailing prose on the same comment) — citation
|
||||
* tracking asks a narrower question ("is this meant as a real directive
|
||||
* that needs a tracking ref") than suppression does, so the two are allowed
|
||||
* to diverge.
|
||||
*
|
||||
* @param {{value: string, loc: {start: {line: number}}}[]} allComments
|
||||
* @returns {string[]} raw reason text for every genuine directive line found
|
||||
*/
|
||||
function walkTestFiles(dir) {
|
||||
const files = [];
|
||||
function extractGenuineMarkerReasons(allComments) {
|
||||
const reasons = [];
|
||||
for (const comment of allComments) {
|
||||
for (const rawLine of comment.value.split('\n')) {
|
||||
const trimmed = rawLine.replace(/^\s*\*\s?/, '').trim();
|
||||
if (!/^allow-test-rule:/.test(trimmed)) continue;
|
||||
const reason = trimmed.slice('allow-test-rule:'.length).trim();
|
||||
if (reason) reasons.push(reason);
|
||||
}
|
||||
}
|
||||
return reasons;
|
||||
}
|
||||
|
||||
function scan(current) {
|
||||
for (const entry of fs.readdirSync(current, { withFileTypes: true })) {
|
||||
const full = path.join(current, entry.name);
|
||||
if (entry.isDirectory()) {
|
||||
scan(full);
|
||||
} else if (entry.isFile() && entry.name.endsWith('.test.cjs')) {
|
||||
const relpath = path.relative(ROOT, full).split(path.sep).join('/');
|
||||
/**
|
||||
* Derive the authoritative set of glob patterns that `eslint.config.mjs`
|
||||
* actually registers `local/no-source-grep` on (#3464 phase 5 BLOCKER fix).
|
||||
*
|
||||
* Historically this script hardcoded `tests/** /*.test.cjs` as "the" scan
|
||||
* set, which silently missed every non-`.test.cjs` file under `tests/`
|
||||
* (`tests/helpers/**`, `tests/qa/**`, `tests/fixtures/**`, ...) AND every one
|
||||
* of the other config blocks the rule is registered on (`scripts/** /*.cjs`,
|
||||
* `eslint-rules/** /*.cjs`, `bin/lib/** /*.cjs`, `pi/** /*.cjs`,
|
||||
* `examples/** /*.cjs`, `gsd-core/bin/** /*.cjs`, `vscode/*.js`,
|
||||
* `.kilo/plugins/*.js`, `.opencode/plugins/*.js`). A marker in any of those
|
||||
* would vanish from BOTH reported numbers instead of tripping anything.
|
||||
*
|
||||
* Rather than hand-maintain a second copy of that glob list (the exact
|
||||
* generative-fix-divergence class this script's own doc comment above
|
||||
* already warns about for the rule's AST walk), this dynamically imports
|
||||
* the REAL flat config and reads which config objects set
|
||||
* `rules['local/no-source-grep']` to something other than `'off'`/`0`,
|
||||
* collecting their `files` globs. If `eslint.config.mjs` ever adds, removes,
|
||||
* or narrows a block that registers this rule, this list moves with it
|
||||
* automatically — there is no second list to fall out of sync.
|
||||
*
|
||||
* @returns {Promise<string[]>} sorted, deduped glob patterns
|
||||
*/
|
||||
async function deriveNoSourceGrepGlobs() {
|
||||
const mod = await import(pathToFileURL(ESLINT_CONFIG_PATH).href);
|
||||
const config = mod.default;
|
||||
if (!Array.isArray(config)) {
|
||||
throw new Error(
|
||||
`lint-allow-test-rule-refs: expected eslint.config.mjs's default export to be an array, got ${typeof config}`
|
||||
);
|
||||
}
|
||||
|
||||
const globs = new Set();
|
||||
for (const entry of config) {
|
||||
if (!entry || typeof entry !== 'object' || !entry.rules || !Array.isArray(entry.files)) continue;
|
||||
if (!Object.prototype.hasOwnProperty.call(entry.rules, 'local/no-source-grep')) continue;
|
||||
const ruleValue = entry.rules['local/no-source-grep'];
|
||||
const level = Array.isArray(ruleValue) ? ruleValue[0] : ruleValue;
|
||||
if (level === 'off' || level === 0) continue;
|
||||
for (const glob of entry.files) globs.add(glob);
|
||||
}
|
||||
|
||||
if (globs.size === 0) {
|
||||
throw new Error(
|
||||
'lint-allow-test-rule-refs: derived zero glob patterns for local/no-source-grep from eslint.config.mjs ' +
|
||||
'— this would silently scan nothing; the config shape likely changed and this script needs updating.'
|
||||
);
|
||||
}
|
||||
|
||||
return [...globs].sort();
|
||||
}
|
||||
|
||||
/**
|
||||
* The literal (non-wildcard) leading path segments of a glob — the
|
||||
* directory that must actually be walked on disk to find candidate files.
|
||||
* e.g. `tests/** /*.cjs` -> `tests`, `.kilo/plugins/*.js` -> `.kilo/plugins`.
|
||||
*
|
||||
* The LAST segment is always excluded from consideration even when it has
|
||||
* no wildcard of its own (e.g. the single-file fixture globs
|
||||
* `tests/_ff_lint_violation.cjs` / `tests/_ff_lint_clean.cjs`, #1279/#2126):
|
||||
* it is the filename pattern to MATCH against directory entries, never a
|
||||
* directory to descend INTO. Treating a fully-literal glob's whole path as
|
||||
* a "directory" would try to `readdirSync` a plain file and silently find
|
||||
* nothing (caught by `walkGlobs`'s existing not-a-directory guard) — quietly
|
||||
* dropping that glob from the scan entirely, the same silent-omission defect
|
||||
* class this fix exists to eliminate.
|
||||
*
|
||||
* @param {string} glob
|
||||
* @returns {string}
|
||||
*/
|
||||
function globBaseDir(glob) {
|
||||
const segments = glob.split('/');
|
||||
const literalSegments = [];
|
||||
for (let i = 0; i < segments.length - 1; i++) {
|
||||
if (/[*?[\]{}]/.test(segments[i])) break;
|
||||
literalSegments.push(segments[i]);
|
||||
}
|
||||
return literalSegments.join('/');
|
||||
}
|
||||
|
||||
/**
|
||||
* Compile a glob pattern (only `*` and `**` wildcards appear in this repo's
|
||||
* config — no `?`, `{}`, or `[]` are used on any `local/no-source-grep`
|
||||
* block) into an anchored RegExp matched against a POSIX-slash repo-relative
|
||||
* path, mirroring minimatch/eslint's own glob semantics for those two
|
||||
* operators: `** /` matches zero or more path segments, a bare `*` matches
|
||||
* within a single segment only.
|
||||
*
|
||||
* @param {string} glob
|
||||
* @returns {RegExp}
|
||||
*/
|
||||
function globToRegExp(glob) {
|
||||
let re = '';
|
||||
for (let i = 0; i < glob.length; i++) {
|
||||
const c = glob[i];
|
||||
if (c === '*' && glob[i + 1] === '*') {
|
||||
i++; // consume second '*'
|
||||
if (glob[i + 1] === '/') {
|
||||
re += '(?:.*/)?';
|
||||
i++; // consume the following '/'
|
||||
} else {
|
||||
re += '.*';
|
||||
}
|
||||
} else if (c === '*') {
|
||||
re += '[^/]*';
|
||||
} else if ('.+^${}()|[]\\'.includes(c)) {
|
||||
re += `\\${c}`;
|
||||
} else {
|
||||
re += c;
|
||||
}
|
||||
}
|
||||
return new RegExp(`^${re}$`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Recursively read every file matching ANY of `globs`, once, deduped by
|
||||
* absolute path.
|
||||
*
|
||||
* The citation check, the total marker-file inventory, and the
|
||||
* effective-site classification all need the same file set and content —
|
||||
* walking twice would be the generative-fix-divergence class of bug (two
|
||||
* scans that can silently drift apart), so every classifier below consumes
|
||||
* this single walk's output.
|
||||
*
|
||||
* Each glob's literal base directory (`globBaseDir`) is resolved to an
|
||||
* actual directory on disk: the `tests` base uses `testsDir` (the existing
|
||||
* `GSD_LINT_ALLOW_TEST_RULE_TESTS_DIR` override contract, unchanged), every
|
||||
* other base is resolved under `extraRoot` (defaults to `ROOT`, overridable
|
||||
* via `GSD_LINT_ALLOW_TEST_RULE_EXTRA_ROOT` so sandbox fixtures can exercise
|
||||
* the widened scan scope in isolation). Matching itself is always done
|
||||
* against the glob's own repo-relative path string (e.g. `tests/foo.cjs`,
|
||||
* `scripts/bar.cjs`), never against the physical scan location, so a
|
||||
* sandboxed base directory is matched exactly as if it WERE that
|
||||
* repo-relative path.
|
||||
*
|
||||
* @param {string[]} globs
|
||||
* @param {{testsDir: string, extraRoot: string, root: string}} opts
|
||||
* @returns {{relpath: string, full: string, content: string}[]}
|
||||
*/
|
||||
function walkGlobs(globs, { testsDir, extraRoot, root }) {
|
||||
const compiled = globs.map((glob) => ({
|
||||
baseDir: globBaseDir(glob),
|
||||
regex: globToRegExp(glob),
|
||||
}));
|
||||
const seen = new Map(); // full path -> {relpath, full, content}
|
||||
|
||||
for (const { baseDir, regex } of compiled) {
|
||||
const scanRoot = baseDir === 'tests' ? testsDir : path.join(extraRoot, baseDir);
|
||||
|
||||
const scan = (current, relParts) => {
|
||||
let entries;
|
||||
try {
|
||||
entries = fs.readdirSync(current, { withFileTypes: true });
|
||||
} catch {
|
||||
// Base directory does not exist (e.g. an isolated sandbox EXTRA_ROOT
|
||||
// that only sets up the fixture-relevant subtree) — nothing to scan.
|
||||
return;
|
||||
}
|
||||
for (const entry of entries) {
|
||||
const full = path.join(current, entry.name);
|
||||
if (entry.isDirectory()) {
|
||||
scan(full, [...relParts, entry.name]);
|
||||
continue;
|
||||
}
|
||||
if (!entry.isFile()) continue;
|
||||
const matchPath = [baseDir, ...relParts, entry.name].join('/');
|
||||
if (!regex.test(matchPath)) continue;
|
||||
if (seen.has(full)) continue;
|
||||
let content;
|
||||
try {
|
||||
content = fs.readFileSync(full, 'utf8');
|
||||
@@ -124,40 +407,30 @@ function walkTestFiles(dir) {
|
||||
// skip unreadable files (e.g. binary)
|
||||
continue;
|
||||
}
|
||||
files.push({ relpath, content });
|
||||
const relpath = path.relative(root, full).split(path.sep).join('/');
|
||||
seen.set(full, { relpath, full, content });
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
scan(scanRoot, []);
|
||||
}
|
||||
|
||||
scan(dir);
|
||||
return files;
|
||||
return [...seen.values()];
|
||||
}
|
||||
|
||||
/**
|
||||
* Collect offender identifiers (files with an UNCITED allow-test-rule marker)
|
||||
* from an already-walked file set.
|
||||
* from a per-file map of genuine directive reasons (see
|
||||
* `extractGenuineMarkerReasons`).
|
||||
*
|
||||
* @param {{relpath: string, content: string}[]} files
|
||||
* @param {Map<string, string[]>} markerReasonsByFile relpath -> reasons
|
||||
* @returns {string[]} sorted, deduped list of `<relpath> :: <reason>` strings
|
||||
*/
|
||||
function collectUncitedOffenders(files) {
|
||||
function collectUncitedOffenders(markerReasonsByFile) {
|
||||
const offenders = new Set();
|
||||
|
||||
for (const { relpath, content } of files) {
|
||||
// Scan line-by-line. By testing each line for `allow-test-rule:` we
|
||||
// cover BOTH comment forms without a cross-line regex:
|
||||
// // allow-test-rule: <reason> ← line comment
|
||||
// /* allow-test-rule: <reason> */ ← single-line block comment
|
||||
//
|
||||
// For each matching line we extract the reason (everything after the
|
||||
// colon), then strip any trailing block-comment closer `*/` and
|
||||
// whitespace so the identifier stays clean.
|
||||
for (const line of content.split('\n')) {
|
||||
const m = ALLOW_TEST_RULE_LINE_RE.exec(line);
|
||||
if (!m) continue;
|
||||
// Strip trailing block-comment closer and whitespace if present
|
||||
const reason = m[1].replace(/\s*\*\/\s*$/, '').trim();
|
||||
if (!reason) continue;
|
||||
for (const [relpath, reasons] of markerReasonsByFile) {
|
||||
for (const reason of reasons) {
|
||||
if (ISSUE_REF_RE.test(reason)) continue; // compliant — skip
|
||||
offenders.add(`${relpath} :: ${reason}`);
|
||||
}
|
||||
@@ -167,37 +440,317 @@ function collectUncitedOffenders(files) {
|
||||
}
|
||||
|
||||
/**
|
||||
* Count distinct files carrying at least one allow-test-rule marker, cited or
|
||||
* not — the raw total the ceiling ratchets down, independent of citation
|
||||
* status.
|
||||
* Files carrying at least one genuine allow-test-rule directive, cited or
|
||||
* not — the universe that effective/unverified partitions.
|
||||
*
|
||||
* @param {{relpath: string, content: string}[]} files
|
||||
* @param {Map<string, string[]>} markerReasonsByFile relpath -> reasons
|
||||
* @returns {string[]} sorted, deduped list of relpaths
|
||||
*/
|
||||
function collectExemptionFiles(files) {
|
||||
const marked = new Set();
|
||||
function collectExemptionFiles(markerReasonsByFile) {
|
||||
const marked = [];
|
||||
for (const [relpath, reasons] of markerReasonsByFile) {
|
||||
if (reasons.length > 0) marked.push(relpath);
|
||||
}
|
||||
return marked.sort();
|
||||
}
|
||||
|
||||
for (const { relpath, content } of files) {
|
||||
if (ALLOW_TEST_RULE_LINE_RE.test(content)) {
|
||||
marked.add(relpath);
|
||||
/**
|
||||
* ESLint eslintrc-format config fragment that runs ONLY `no-source-grep`,
|
||||
* with suppression neutralized so every candidate site is reported
|
||||
* regardless of a marker being present. Shared by every file classified
|
||||
* below.
|
||||
*
|
||||
* Deliberately eslintrc-shaped (`parserOptions`/`env`), not flat-config
|
||||
* (`languageOptions`/`plugins`+`files`) (#3464 phase 5 follow-up). Flat
|
||||
* config's `Linter#verify` still validates that the filename resolves to an
|
||||
* ancestor of the Linter's `cwd` even for a config with no `files` glob
|
||||
* (i.e. one meant to apply universally) — any filename that only resolves
|
||||
* via a leading `..` (exactly what `walkGlobs`' `path.relative(ROOT, full)`
|
||||
* produces for a sandbox fixture outside ROOT, e.g. in CI's `/tmp`) trips
|
||||
* flat config's "No matching configuration found" error, indistinguishable
|
||||
* from this script's own defensive throw for a real misconfiguration
|
||||
* (`classifySiteLines`, `configProblem` below). eslintrc-format
|
||||
* `Linter#verify` performs no such cwd/ancestor check at all — the config
|
||||
* passed here fully determines what runs, `filename` is used only for
|
||||
* per-file diagnostics/overrides, never resolved against `cwd` — so
|
||||
* classification behaves identically for repo files and out-of-tree sandbox
|
||||
* fixtures. `sourceType`/`ecmaVersion`/node globals mirror the
|
||||
* `languageOptions` on every `eslint.config.mjs` block that registers
|
||||
* `local/no-source-grep` (see module doc comment above); verified to
|
||||
* produce byte-identical `Linter#verify` messages against flat config for
|
||||
* real repo files before this change shipped.
|
||||
*/
|
||||
const NEUTRALIZED_CONFIG = {
|
||||
parserOptions: { ecmaVersion: 2022, sourceType: 'commonjs' },
|
||||
env: { node: true },
|
||||
rules: { 'local/no-source-grep': ['error', { neutralizeSuppression: true }] },
|
||||
};
|
||||
|
||||
/**
|
||||
* Build an eslintrc-format `Linter` with `local/no-source-grep` registered
|
||||
* directly via `defineRule` (no plugin-name resolution, no flat-config
|
||||
* `files` matching) — see `NEUTRALIZED_CONFIG` doc comment for why this
|
||||
* sidesteps the cwd/ancestor constraint entirely.
|
||||
*
|
||||
* @returns {import('eslint').Linter}
|
||||
*/
|
||||
function makeLinter() {
|
||||
const linter = new Linter({ configType: 'eslintrc' });
|
||||
linter.defineRule('local/no-source-grep', noSourceGrepRule);
|
||||
return linter;
|
||||
}
|
||||
|
||||
/**
|
||||
* For one file, drive the REAL `no-source-grep` rule via ESLint's `Linter`
|
||||
* API (suppression neutralized) to enumerate every candidate violation site,
|
||||
* then classify each site with the rule's own exported `isSuppressedAt`
|
||||
* predicate against that file's real markers (obtained via the rule's own
|
||||
* exported `collectMarkerAndCommentLines`, fed by the SAME parse the
|
||||
* enumeration pass just produced via `linter.getSourceCode()`).
|
||||
*
|
||||
* This does not hand-roll a second AST walk or a second adjacency
|
||||
* arithmetic: both the enumeration (the rule's Program:exit walk) and the
|
||||
* classification (`isSuppressedAt`) are the rule's own code, imported and
|
||||
* invoked directly. Returns per-site LINE NUMBERS (not just counts) so a
|
||||
* consumer (notably the parity test in
|
||||
* tests/lint-allow-test-rule-refs.test.cjs) can compare this classification
|
||||
* site-by-site against the real rule's independent report/no-report outcome.
|
||||
*
|
||||
* Also returns `markerReasons` (see `extractGenuineMarkerReasons`) computed
|
||||
* from the SAME parse, so the citation check and exemption-file inventory
|
||||
* (`main`, via `classifySites`) never re-parse a file a second time just to
|
||||
* find its markers.
|
||||
*
|
||||
* @param {import('eslint').Linter} linter
|
||||
* @param {{relpath: string, content: string}} file
|
||||
* @returns {{effectiveLines: number[], liveLines: number[], markerReasons: string[]}}
|
||||
*/
|
||||
function classifySiteLines(linter, file) {
|
||||
let messages;
|
||||
try {
|
||||
messages = linter.verify(file.content, NEUTRALIZED_CONFIG, file.relpath);
|
||||
} catch (err) {
|
||||
// An unparseable file must NOT be silently classified as "zero sites"
|
||||
// (indistinguishable from a genuinely clean file) — that is exactly the
|
||||
// false-clean failure mode this counter exists to eliminate, and it
|
||||
// would be inconsistent with the sibling "No matching configuration
|
||||
// found" case just below, which correctly throws. Fail loudly with the
|
||||
// file path and the underlying error instead. If a specific file
|
||||
// genuinely cannot be parsed for a known, accepted reason, exempt it
|
||||
// explicitly by name with a comment here — never blanket-swallow.
|
||||
throw new Error(
|
||||
`lint-allow-test-rule-refs: failed to parse/verify ${file.relpath}: ` +
|
||||
`${err && err.message ? err.message : err}`
|
||||
);
|
||||
}
|
||||
|
||||
// A parse error does NOT throw a JS exception from linter.verify() -- ESLint
|
||||
// catches it internally and reports it as a `fatal: true` message instead,
|
||||
// with `sourceCode` left unset (getSourceCode() below would return null).
|
||||
// This is the same silent-clean failure mode the catch block above guards
|
||||
// against, just reached via a different ESLint API shape: without this
|
||||
// check a genuinely unparseable file would either crash on the next line
|
||||
// (`sourceCode.getAllComments()` on null) or, before that line existed,
|
||||
// silently fall through the `sites.length === 0` early return below and be
|
||||
// reported as zero violations -- indistinguishable from a clean file. Fail
|
||||
// loudly with the file path and message instead, matching the sibling
|
||||
// "No matching configuration found" case just below.
|
||||
const fatalProblem = messages.find((m) => m.fatal);
|
||||
if (fatalProblem) {
|
||||
throw new Error(
|
||||
`lint-allow-test-rule-refs: failed to parse/verify ${file.relpath}: ` +
|
||||
`${fatalProblem.message} (line ${fatalProblem.line}, column ${fatalProblem.column})`
|
||||
);
|
||||
}
|
||||
|
||||
// Defensive: "No matching configuration found for ..." was ESLint's
|
||||
// flat-config-specific signal that it could not match ANY configuration to
|
||||
// a filename (notably, when the Linter's cwd was not an ancestor of the
|
||||
// file -- exactly what an out-of-tree sandbox fixture path hit, #3464
|
||||
// phase 5 follow-up). `makeLinter`/`NEUTRALIZED_CONFIG` now use
|
||||
// eslintrc-format `Linter#verify`, which performs no cwd/ancestor
|
||||
// resolution at all, so this specific message should never fire in
|
||||
// practice. Kept as a defensive throw (rather than deleted) in case a
|
||||
// future ESLint version emits an equivalent `ruleId === null` signal for
|
||||
// eslintrc-format verification too -- silently reporting zero rule
|
||||
// messages would be indistinguishable from a genuinely clean file, exactly
|
||||
// the kind of silently-wrong number this counter exists to avoid.
|
||||
const configProblem = messages.find(
|
||||
(m) => m.ruleId === null && /No matching configuration found/.test(m.message)
|
||||
);
|
||||
if (configProblem) {
|
||||
throw new Error(
|
||||
`lint-allow-test-rule-refs: ESLint could not lint ${file.relpath} (${configProblem.message}).`
|
||||
);
|
||||
}
|
||||
|
||||
const sourceCode = linter.getSourceCode();
|
||||
const allComments = sourceCode.getAllComments();
|
||||
const markerReasons = extractGenuineMarkerReasons(allComments);
|
||||
|
||||
const sites = messages.filter((m) => m.messageId === 'noSourceGrep');
|
||||
if (sites.length === 0) return { effectiveLines: [], liveLines: [], markerReasons };
|
||||
|
||||
// The rule's own suppression check (isSuppressed, called from
|
||||
// reportUnlessSuppressed) is an OR over TWO lines: the search call's own
|
||||
// line, and the line of the readFileSync() call that originated the
|
||||
// tracked value (a marker adjacent to EITHER half suppresses). In
|
||||
// neutralizeSuppression mode the rule emits a paired
|
||||
// `noSourceGrepDiagnosticReadLine` message at the SAME node carrying that
|
||||
// read line, so this classification can replicate the identical two-line
|
||||
// OR check the rule performs internally -- via the rule's OWN
|
||||
// `isSuppressedAt` predicate, called exactly as `reportUnlessSuppressed`
|
||||
// does, rather than only checking the search line (which would
|
||||
// misclassify the documented "marker adjacent to the read, not the
|
||||
// search" placement style as unsuppressed).
|
||||
const readLineByPos = new Map();
|
||||
for (const m of messages) {
|
||||
if (m.messageId !== 'noSourceGrepDiagnosticReadLine') continue;
|
||||
readLineByPos.set(`${m.line}:${m.column}`, m.message === '' ? null : Number(m.message));
|
||||
}
|
||||
|
||||
const { markerLines, commentLineSet } = noSourceGrepRule.collectMarkerAndCommentLines(allComments);
|
||||
|
||||
const effectiveLines = [];
|
||||
const liveLines = [];
|
||||
for (const site of sites) {
|
||||
const readLine = readLineByPos.get(`${site.line}:${site.column}`) ?? null;
|
||||
const suppressedBySearchLine = noSourceGrepRule.isSuppressedAt({
|
||||
markerLines,
|
||||
violationLine: site.line,
|
||||
commentLineSet,
|
||||
lines: sourceCode.lines,
|
||||
});
|
||||
const suppressedByReadLine =
|
||||
readLine !== null &&
|
||||
noSourceGrepRule.isSuppressedAt({
|
||||
markerLines,
|
||||
violationLine: readLine,
|
||||
commentLineSet,
|
||||
lines: sourceCode.lines,
|
||||
});
|
||||
if (suppressedBySearchLine || suppressedByReadLine) effectiveLines.push(site.line);
|
||||
else liveLines.push(site.line);
|
||||
}
|
||||
|
||||
return { effectiveLines, liveLines, markerReasons };
|
||||
}
|
||||
|
||||
/**
|
||||
* Count-only view of classifySiteLines, used by the aggregate walk below.
|
||||
*
|
||||
* @param {import('eslint').Linter} linter
|
||||
* @param {{relpath: string, content: string}} file
|
||||
* @returns {{effectiveSites: number, liveSites: number, markerReasons: string[]}}
|
||||
*/
|
||||
function classifyFile(linter, file) {
|
||||
const { effectiveLines, liveLines, markerReasons } = classifySiteLines(linter, file);
|
||||
return { effectiveSites: effectiveLines.length, liveSites: liveLines.length, markerReasons };
|
||||
}
|
||||
|
||||
/**
|
||||
* Convenience wrapper for tests: classify a single in-memory code string
|
||||
* without a caller having to construct a Linter instance or a file object.
|
||||
*
|
||||
* @param {string} code
|
||||
* @param {string} [relpath] - filename passed to ESLint (affects nothing but
|
||||
* diagnostics; the eslintrc-format `Linter` built by `makeLinter` never
|
||||
* resolves it against a base path, so this may be any string, in-tree or
|
||||
* not). Defaults to a generic test filename.
|
||||
* @returns {{effectiveLines: number[], liveLines: number[]}}
|
||||
*/
|
||||
function classifyCode(code, relpath = 'tests/fixture.test.cjs') {
|
||||
return classifySiteLines(makeLinter(), { relpath, content: code });
|
||||
}
|
||||
|
||||
/**
|
||||
* Classify every file's violation sites into effective (suppressed by a real
|
||||
* marker) vs. live (unsuppressed — should be zero in a clean tree), and
|
||||
* derive which files count as "effective" (>=1 effective site).
|
||||
*
|
||||
* @param {{relpath: string, content: string}[]} files
|
||||
* Also returns `markerReasonsByFile` (relpath -> genuine directive reasons,
|
||||
* see `extractGenuineMarkerReasons`), collected from the SAME per-file parse
|
||||
* this function already performs, so `main` can derive the citation check
|
||||
* and exemption-file inventory without a second file-content scan.
|
||||
*
|
||||
* @returns {{
|
||||
* effectiveSiteCount: number,
|
||||
* effectiveFiles: string[],
|
||||
* liveSiteCount: number,
|
||||
* liveFiles: string[],
|
||||
* markerReasonsByFile: Map<string, string[]>,
|
||||
* }}
|
||||
*/
|
||||
function classifySites(files) {
|
||||
const linter = makeLinter();
|
||||
let effectiveSiteCount = 0;
|
||||
let liveSiteCount = 0;
|
||||
const effectiveFiles = new Set();
|
||||
const liveFiles = new Set();
|
||||
const markerReasonsByFile = new Map();
|
||||
|
||||
for (const file of files) {
|
||||
const { effectiveSites, liveSites, markerReasons } = classifyFile(linter, file);
|
||||
if (effectiveSites > 0) {
|
||||
effectiveSiteCount += effectiveSites;
|
||||
effectiveFiles.add(file.relpath);
|
||||
}
|
||||
if (liveSites > 0) {
|
||||
liveSiteCount += liveSites;
|
||||
liveFiles.add(file.relpath);
|
||||
}
|
||||
if (markerReasons.length > 0) {
|
||||
markerReasonsByFile.set(file.relpath, markerReasons);
|
||||
}
|
||||
}
|
||||
|
||||
return [...marked].sort();
|
||||
return {
|
||||
effectiveSiteCount,
|
||||
effectiveFiles: [...effectiveFiles].sort(),
|
||||
liveSiteCount,
|
||||
liveFiles: [...liveFiles].sort(),
|
||||
markerReasonsByFile,
|
||||
};
|
||||
}
|
||||
|
||||
function main() {
|
||||
async function main() {
|
||||
const args = process.argv.slice(2);
|
||||
const unknown = args.filter((a) => a !== '--help');
|
||||
if (unknown.length > 0) {
|
||||
throw new ExitError(2, `lint-allow-test-rule-refs: unknown argument(s): ${unknown.join(', ')}`);
|
||||
}
|
||||
|
||||
const files = walkTestFiles(TESTS_DIR);
|
||||
const current = collectUncitedOffenders(files);
|
||||
const globs = await deriveNoSourceGrepGlobs();
|
||||
const files = walkGlobs(globs, { testsDir: TESTS_DIR, extraRoot: EXTRA_ROOT, root: ROOT });
|
||||
const known = JSON.parse(fs.readFileSync(ALLOWLIST_PATH, 'utf8'));
|
||||
const exemptionFiles = collectExemptionFiles(files);
|
||||
const ceiling = JSON.parse(fs.readFileSync(CEILING_PATH, 'utf8'));
|
||||
const effectiveCeiling = JSON.parse(fs.readFileSync(EFFECTIVE_CEILING_PATH, 'utf8'));
|
||||
const unverifiedCeiling = JSON.parse(fs.readFileSync(UNVERIFIED_CEILING_PATH, 'utf8'));
|
||||
|
||||
// Narrow the expensive Linter pass to marker-bearing files only (see
|
||||
// "Linter scope" doc comment above) — a plain in-memory substring scan
|
||||
// over content the walk already read, never a shell grep (NUL-byte
|
||||
// fixture, see comment). This is a strict superset filter: a file with no
|
||||
// `allow-test-rule:` substring at all cannot produce any genuine directive
|
||||
// (extractGenuineMarkerReasons requires the substring) or count toward
|
||||
// effective/unverified, so nothing is lost by skipping it here.
|
||||
const markerBearingFiles = files.filter((f) => f.content.includes('allow-test-rule:'));
|
||||
|
||||
// classifySites does one Linter parse per marker-bearing file and returns
|
||||
// both the effective/live site classification AND (as a byproduct of that
|
||||
// SAME parse) every file's genuine marker reasons — the citation check and
|
||||
// exemption-file inventory below consume that instead of re-scanning file
|
||||
// content a second time.
|
||||
const { effectiveSiteCount, effectiveFiles, liveSiteCount, liveFiles, markerReasonsByFile } =
|
||||
classifySites(markerBearingFiles);
|
||||
|
||||
const current = collectUncitedOffenders(markerReasonsByFile);
|
||||
const exemptionFiles = collectExemptionFiles(markerReasonsByFile);
|
||||
|
||||
// Row 5 (#3520 test-matrix): a file with >=1 effective site counts ONLY as
|
||||
// effective, never also as unverified — no double counting.
|
||||
const effectiveFileSet = new Set(effectiveFiles);
|
||||
const unverifiedFiles = exemptionFiles.filter((f) => !effectiveFileSet.has(f));
|
||||
|
||||
const failures = [];
|
||||
const { novel } = assertWithinAllowlist({
|
||||
@@ -208,14 +761,48 @@ function main() {
|
||||
pruneHint: 'edit scripts/lint-allow-test-rule-refs.allowlist.json',
|
||||
});
|
||||
|
||||
// Effective exemption SITES: tightly ratcheted, both directions — the
|
||||
// metric this gate actually governs.
|
||||
assertTightCeiling({
|
||||
label: 'allow-test-rule-total-files',
|
||||
actualMax: exemptionFiles.length,
|
||||
ceiling: ceiling.maxFiles,
|
||||
grace: ceiling.grace,
|
||||
fail: (msg) => failures.push(`${msg}\n(edit scripts/lint-allow-test-rule-refs.ceiling.json)`),
|
||||
label: 'allow-test-rule-effective-sites',
|
||||
actualMax: effectiveSiteCount,
|
||||
ceiling: effectiveCeiling.maxSites,
|
||||
grace: effectiveCeiling.grace,
|
||||
fail: (msg) => failures.push(`${msg}\n(edit scripts/lint-allow-test-rule-refs.effective-ceiling.json)`),
|
||||
});
|
||||
|
||||
// Unverified marker-bearing FILES: a loose upper bound only — growth past
|
||||
// the ceiling fails, but a drop below it never does (test-matrix.md row 9;
|
||||
// shrinking this pool is a rule-coverage improvement, not something to
|
||||
// ratchet on deletion).
|
||||
if (unverifiedFiles.length > unverifiedCeiling.maxFiles) {
|
||||
failures.push(
|
||||
`[allow-test-rule-unverified-markers] ${unverifiedFiles.length} unverified marker-bearing ` +
|
||||
`file(s) exceeds ceiling ${unverifiedCeiling.maxFiles} (marker-bearing files with no ` +
|
||||
`detectable violation grew — audit the new markers before raising the ceiling; this is ` +
|
||||
`growth of the UNVERIFIED pool, distinct from the effective-sites ratchet above).\n` +
|
||||
`(edit scripts/lint-allow-test-rule-refs.unverified-ceiling.json)`
|
||||
);
|
||||
}
|
||||
|
||||
// Live (unsuppressed) violations: this only covers MARKER-BEARING files
|
||||
// (see "Linter scope" doc comment above) — a live violation in a file with
|
||||
// NO marker at all is not re-checked here because `npx eslint .` /
|
||||
// `npm run lint` / `npm run lint:ci` already fail the build on those via
|
||||
// `local/no-source-grep` directly. Surfacing the marker-bearing subset
|
||||
// here makes it visible in this gate's own output rather than only in a
|
||||
// separate lint pass, without duplicating the repo-wide check.
|
||||
if (liveSiteCount > 0) {
|
||||
failures.push(
|
||||
`[allow-test-rule-live-violations] ${liveSiteCount} unsuppressed no-source-grep ` +
|
||||
`violation(s) found in ${liveFiles.length} marker-bearing file(s) (unmarked files are ` +
|
||||
`already enforced separately by npm run lint / npm run lint:ci; this gate only re-checks ` +
|
||||
`files that carry an allow-test-rule: marker) ` +
|
||||
`— fix at the source or add a site-scoped // allow-test-rule: <reason> (#NNN):\n` +
|
||||
liveFiles.map((f) => ` - ${f}`).join('\n')
|
||||
);
|
||||
}
|
||||
|
||||
if (failures.length > 0) {
|
||||
for (const msg of failures) process.stderr.write(`${msg}\n`);
|
||||
if (novel.length > 0) {
|
||||
@@ -229,8 +816,38 @@ function main() {
|
||||
|
||||
console.log(
|
||||
`ok lint-allow-test-rule-refs: ${current.length} grandfathered exemption(s) tracked, ` +
|
||||
`${exemptionFiles.length}/${ceiling.maxFiles} exemption file(s) (ceiling), no novel untracked offenders`
|
||||
`no novel untracked offenders; ` +
|
||||
`effective exemptions: ${effectiveSiteCount}/${effectiveCeiling.maxSites} site(s) ` +
|
||||
`across ${effectiveFiles.length} file(s) (ratcheted); ` +
|
||||
`unverified markers: ${unverifiedFiles.length}/${unverifiedCeiling.maxFiles} file(s) ` +
|
||||
`(tracked, not ratcheted); ` +
|
||||
`live violations in marker-bearing files: ${liveSiteCount} (unmarked files are enforced ` +
|
||||
`separately by npm run lint / npm run lint:ci, not re-checked here). ` +
|
||||
`Known limit: "effective" is bounded by no-source-grep's own detection — ` +
|
||||
`identifier-indirection, dynamic paths, .sh files, and array/object round-trips are ` +
|
||||
`real blind spots this count cannot see, so it is never "all exemptions."`
|
||||
);
|
||||
}
|
||||
|
||||
runMain(main);
|
||||
// Exported for tests/lint-allow-test-rule-refs.test.cjs (unit-level
|
||||
// classification checks and the parity/teeth structural guard) — never
|
||||
// re-implemented in the test file, imported directly like the CLI itself
|
||||
// does. `runMain(main)` only fires when this file is executed as the CLI
|
||||
// entrypoint (`node scripts/lint-allow-test-rule-refs.cjs` /
|
||||
// `runNode([SCRIPT])`), not when it is `require()`d for its exports.
|
||||
module.exports = {
|
||||
deriveNoSourceGrepGlobs,
|
||||
globBaseDir,
|
||||
globToRegExp,
|
||||
walkGlobs,
|
||||
collectUncitedOffenders,
|
||||
collectExemptionFiles,
|
||||
classifySiteLines,
|
||||
classifyFile,
|
||||
classifySites,
|
||||
classifyCode,
|
||||
};
|
||||
|
||||
if (require.main === module) {
|
||||
runMain(main);
|
||||
}
|
||||
|
||||
4
scripts/lint-allow-test-rule-refs.effective-ceiling.json
Normal file
4
scripts/lint-allow-test-rule-refs.effective-ceiling.json
Normal file
@@ -0,0 +1,4 @@
|
||||
{
|
||||
"maxSites": 10,
|
||||
"grace": 2
|
||||
}
|
||||
@@ -0,0 +1,3 @@
|
||||
{
|
||||
"maxFiles": 280
|
||||
}
|
||||
@@ -1,12 +1,15 @@
|
||||
'use strict';
|
||||
|
||||
// Tests for scripts/lint-allow-test-rule-refs.cjs — the dual guard that (a)
|
||||
// Tests for scripts/lint-allow-test-rule-refs.cjs — the guard that (a)
|
||||
// ratchets exemption-marker comments on IDENTITY (uncited comments must
|
||||
// carry a tracking-issue ref or be grandfathered) and (b) ratchets the total
|
||||
// distinct exemption-file count against a tight ceiling via assertTightCeiling
|
||||
// (scripts/lib/allowlist-ratchet.cjs). Uses the script's env overrides to
|
||||
// point at sandbox fixture dirs/files; never touches the real tests/ dir or
|
||||
// the real allowlist/ceiling JSON.
|
||||
// carry a tracking-issue ref or be grandfathered), (b) ratchets EFFECTIVE
|
||||
// exemption SITES (a marker actually suppressing a violation the
|
||||
// `no-source-grep` rule detects there) against a tight ceiling, and (c)
|
||||
// tracks UNVERIFIED marker-bearing files (a marker with no detectable
|
||||
// violation nearby) against a loose ceiling that never fails on a drop
|
||||
// (#3520 / epic #3464 phase 5). Uses the script's env overrides to point at
|
||||
// sandbox fixture dirs/files; never touches the real tests/ dir or the real
|
||||
// allowlist/ceiling JSON.
|
||||
//
|
||||
// Identifier note: the script computes `relpath = path.relative(ROOT, full)`
|
||||
// where ROOT is the repo root (path.join(__dirname, '..') inside the script),
|
||||
@@ -21,6 +24,7 @@ const { describe, test, before, after } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const { Linter } = require('eslint');
|
||||
const { createTempDir, cleanup } = require('./helpers.cjs');
|
||||
const { runNode } = require('./helpers/process-seam.cjs');
|
||||
const { toLegacyResult } = require('./helpers/git-fixture.cjs');
|
||||
@@ -29,13 +33,33 @@ const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
|
||||
const ROOT = path.join(__dirname, '..');
|
||||
const SCRIPT = path.join(ROOT, 'scripts', 'lint-allow-test-rule-refs.cjs');
|
||||
|
||||
// The "repo baseline" row below (unlike every other row in this file) points
|
||||
// the script at the REAL repo tests/scripts/eslint-rules/etc trees with no
|
||||
// sandbox override, so it drives ESLint's `Linter` over every real
|
||||
// marker-bearing file the script finds (~294 files after the #3464 perf
|
||||
// follow-up narrowed the Linter pass off the full ~1200-file glob walk).
|
||||
// That is a heavier class than `PROBE_TIMEOUT_MS` describes ("a single short
|
||||
// CLI query or `node -e` probe against a temp fixture") — measured ~2.3-3.3s
|
||||
// locally post-narrowing, down from ~7-12s pre-narrowing, which is what
|
||||
// previously died at exactly the `PROBE_TIMEOUT_MS=15000` bound under CI
|
||||
// load (empty stdout/stderr, exitCode null — SIGKILLed by the harness
|
||||
// timeout, not a real assertion failure). 30000ms is ~10x the measured local
|
||||
// runtime, leaving real headroom for slower CI runners without hand-waving
|
||||
// the bound back up to "whatever makes it pass this once."
|
||||
const REPO_BASELINE_LINT_TIMEOUT_MS = 30000;
|
||||
|
||||
// Deliberately split so this file's OWN source never contains the contiguous
|
||||
// exemption-marker substring the script under test scans for — the script
|
||||
// does a raw-text scan (not AST/comment parsing), so an unsplit literal here
|
||||
// would make THIS test file register as its own offender when the real
|
||||
// lint:ci run scans tests/.
|
||||
// does a raw-text scan (not AST/comment parsing) for the marker-inventory
|
||||
// check, so an unsplit literal here would make THIS test file register as
|
||||
// its own offender when the real lint:ci run scans tests/.
|
||||
const MARKER = 'allow' + '-test-rule:';
|
||||
|
||||
// Row 13/14/15 (structural guard) import the rule and the script under test
|
||||
// directly by reference, never re-declaring their logic.
|
||||
const noSourceGrepRule = require('../eslint-rules/no-source-grep.cjs');
|
||||
const scriptUnderTest = require('../scripts/lint-allow-test-rule-refs.cjs');
|
||||
|
||||
let sandbox;
|
||||
let fixtureCount = 0;
|
||||
|
||||
@@ -48,30 +72,63 @@ function relToRoot(fullPath) {
|
||||
|
||||
/**
|
||||
* Writes `files` (name -> content) into a fresh sandbox tests dir, plus an
|
||||
* allowlist JSON and a ceiling JSON, then invokes the script via its three
|
||||
* allowlist JSON and the two ceiling JSONs, then invokes the script via its
|
||||
* env-var overrides.
|
||||
*
|
||||
* @param {object} opts
|
||||
* @param {Object<string,string>} opts.files - filename -> file content.
|
||||
* @param {Object<string,string>} opts.files - filename (may include `/` for
|
||||
* a nested subdirectory, e.g. `helpers/foo.cjs`) -> file content.
|
||||
* @param {string[]} [opts.allowlist] - allowlist array (default []).
|
||||
* @param {{maxFiles:number,grace:number}} [opts.ceiling] - default is
|
||||
* deliberately generous so a case testing ONLY the citation check does not
|
||||
* accidentally also trip the ceiling check.
|
||||
* @param {{maxSites:number,grace:number}} [opts.effectiveCeiling] - default
|
||||
* is deliberately generous so a case not targeting the effective-sites
|
||||
* ratchet does not accidentally also trip it.
|
||||
* @param {{maxFiles:number}} [opts.unverifiedCeiling] - default is
|
||||
* deliberately generous, same reasoning.
|
||||
* @param {string[]} [opts.args] - extra CLI argv.
|
||||
* @param {Object<string,string>} [opts.extraFiles] - filename (relative to a
|
||||
* FRESH per-call `extraRoot`, e.g. `scripts/fixture.cjs`) -> file content,
|
||||
* for exercising the non-`tests/` glob blocks (scripts/** , eslint-rules/** ,
|
||||
* etc. — #3464 phase 5 BLOCKER fix widened scan scope) in isolation. Left
|
||||
* empty by default, which keeps every existing row's runtime unaffected: a
|
||||
* per-call `extraRoot` is always created (never shared/reused across
|
||||
* calls, so nothing leaks between rows) and set as
|
||||
* `GSD_LINT_ALLOW_TEST_RULE_EXTRA_ROOT`; with no `extraFiles` it stays an
|
||||
* empty directory, so every non-`tests/` glob costs one cheap empty
|
||||
* `readdirSync`, never a real-repo scan.
|
||||
*/
|
||||
function runLint({ files, allowlist = [], ceiling = { maxFiles: 1000, grace: 1000 }, args = [] }) {
|
||||
function runLint({
|
||||
files,
|
||||
allowlist = [],
|
||||
effectiveCeiling = { maxSites: 1000, grace: 1000 },
|
||||
unverifiedCeiling = { maxFiles: 1000 },
|
||||
args = [],
|
||||
extraFiles = {},
|
||||
}) {
|
||||
const testsDir = path.join(sandbox, `tests-${fixtureCount}`);
|
||||
fs.mkdirSync(testsDir, { recursive: true });
|
||||
const relpaths = {};
|
||||
for (const [name, content] of Object.entries(files)) {
|
||||
const full = path.join(testsDir, name);
|
||||
fs.mkdirSync(path.dirname(full), { recursive: true });
|
||||
fs.writeFileSync(full, content);
|
||||
relpaths[name] = relToRoot(full);
|
||||
}
|
||||
|
||||
const extraRoot = path.join(sandbox, `extra-${fixtureCount}`);
|
||||
fs.mkdirSync(extraRoot, { recursive: true });
|
||||
for (const [name, content] of Object.entries(extraFiles)) {
|
||||
const full = path.join(extraRoot, name);
|
||||
fs.mkdirSync(path.dirname(full), { recursive: true });
|
||||
fs.writeFileSync(full, content);
|
||||
relpaths[`extra:${name}`] = relToRoot(full);
|
||||
}
|
||||
|
||||
const allowlistPath = path.join(sandbox, `allowlist-${fixtureCount}.json`);
|
||||
fs.writeFileSync(allowlistPath, JSON.stringify(allowlist));
|
||||
const ceilingPath = path.join(sandbox, `ceiling-${fixtureCount}.json`);
|
||||
fs.writeFileSync(ceilingPath, JSON.stringify(ceiling));
|
||||
const effectiveCeilingPath = path.join(sandbox, `effective-ceiling-${fixtureCount}.json`);
|
||||
fs.writeFileSync(effectiveCeilingPath, JSON.stringify(effectiveCeiling));
|
||||
const unverifiedCeilingPath = path.join(sandbox, `unverified-ceiling-${fixtureCount}.json`);
|
||||
fs.writeFileSync(unverifiedCeilingPath, JSON.stringify(unverifiedCeiling));
|
||||
fixtureCount += 1;
|
||||
|
||||
const result = runNode([SCRIPT, ...args], {
|
||||
@@ -80,11 +137,13 @@ function runLint({ files, allowlist = [], ceiling = { maxFiles: 1000, grace: 100
|
||||
env: {
|
||||
...process.env,
|
||||
GSD_LINT_ALLOW_TEST_RULE_TESTS_DIR: testsDir,
|
||||
GSD_LINT_ALLOW_TEST_RULE_EXTRA_ROOT: extraRoot,
|
||||
GSD_LINT_ALLOW_TEST_RULE_ALLOWLIST: allowlistPath,
|
||||
GSD_LINT_ALLOW_TEST_RULE_CEILING: ceilingPath,
|
||||
GSD_LINT_ALLOW_TEST_RULE_EFFECTIVE_CEILING: effectiveCeilingPath,
|
||||
GSD_LINT_ALLOW_TEST_RULE_UNVERIFIED_CEILING: unverifiedCeilingPath,
|
||||
},
|
||||
});
|
||||
return { ...toLegacyResult(result), relpaths, testsDir };
|
||||
return { ...toLegacyResult(result), relpaths, testsDir, extraRoot };
|
||||
}
|
||||
|
||||
describe('lint-allow-test-rule-refs', () => {
|
||||
@@ -96,35 +155,22 @@ describe('lint-allow-test-rule-refs', () => {
|
||||
cleanup(sandbox);
|
||||
});
|
||||
|
||||
test('passes when a known uncited exemption is grandfathered', () => {
|
||||
const files = { 'foo.test.cjs': `// ${MARKER} some-reason\n` };
|
||||
const testsDir = path.join(sandbox, `tests-${fixtureCount}`);
|
||||
fs.mkdirSync(testsDir, { recursive: true });
|
||||
const full = path.join(testsDir, 'foo.test.cjs');
|
||||
fs.writeFileSync(full, files['foo.test.cjs']);
|
||||
const relpath = relToRoot(full);
|
||||
const allowlistPath = path.join(sandbox, `allowlist-${fixtureCount}.json`);
|
||||
fs.writeFileSync(allowlistPath, JSON.stringify([`${relpath} :: some-reason`]));
|
||||
const ceilingPath = path.join(sandbox, `ceiling-${fixtureCount}.json`);
|
||||
fs.writeFileSync(ceilingPath, JSON.stringify({ maxFiles: 100, grace: 100 }));
|
||||
fixtureCount += 1;
|
||||
// ─── citation check (unchanged contract) ───────────────────────────────
|
||||
|
||||
const r = toLegacyResult(
|
||||
runNode([SCRIPT], {
|
||||
cwd: ROOT,
|
||||
timeoutMs: PROBE_TIMEOUT_MS,
|
||||
env: {
|
||||
...process.env,
|
||||
GSD_LINT_ALLOW_TEST_RULE_TESTS_DIR: testsDir,
|
||||
GSD_LINT_ALLOW_TEST_RULE_ALLOWLIST: allowlistPath,
|
||||
GSD_LINT_ALLOW_TEST_RULE_CEILING: ceilingPath,
|
||||
},
|
||||
})
|
||||
);
|
||||
test('passes when a known uncited exemption is grandfathered', () => {
|
||||
// runLint's testsDir naming (`tests-${fixtureCount}`) is deterministic
|
||||
// and only increments once files are written, so the relpath the script
|
||||
// will compute can be predicted before the run.
|
||||
const testsDir = path.join(sandbox, `tests-${fixtureCount}`);
|
||||
const relpath = relToRoot(path.join(testsDir, 'foo.test.cjs'));
|
||||
const r = runLint({
|
||||
files: { 'foo.test.cjs': `// ${MARKER} some-reason\n` },
|
||||
allowlist: [`${relpath} :: some-reason`],
|
||||
});
|
||||
assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`);
|
||||
});
|
||||
|
||||
test('fails on a novel uncited exemption not in the allowlist', () => {
|
||||
test('fails on a novel uncited exemption not in the allowlist (test-matrix row 10)', () => {
|
||||
const r = runLint({
|
||||
files: { 'foo.test.cjs': `// ${MARKER} mystery-reason\n` },
|
||||
allowlist: [],
|
||||
@@ -135,7 +181,11 @@ describe('lint-allow-test-rule-refs', () => {
|
||||
|
||||
test('fails on a stale allowlist entry (ratchet-down enforcement)', () => {
|
||||
const r = runLint({
|
||||
files: { 'foo.test.cjs': 'no marker here\n' },
|
||||
// Must be VALID JS (parsed by the real Linter, not just grepped) — a
|
||||
// bare "no marker here" is not valid syntax and would now correctly
|
||||
// throw a loud parse failure (the SECOND FIX below) rather than
|
||||
// silently exercising the ratchet-down path this test targets.
|
||||
files: { 'foo.test.cjs': '// no marker here\n' },
|
||||
allowlist: ['tests/definitely-stale-file.test.cjs :: stale-reason'],
|
||||
});
|
||||
assert.notStrictEqual(r.status, 0);
|
||||
@@ -143,152 +193,560 @@ describe('lint-allow-test-rule-refs', () => {
|
||||
assert.match(r.stderr, /definitely-stale-file\.test\.cjs/);
|
||||
});
|
||||
|
||||
test('passes when exemption-file count == ceiling exactly (boundary)', () => {
|
||||
const r = runLint({
|
||||
files: {
|
||||
'a.test.cjs': `// ${MARKER} see #123\n`,
|
||||
'b.test.cjs': `// ${MARKER} see #123\n`,
|
||||
'c.test.cjs': `// ${MARKER} see #123\n`,
|
||||
},
|
||||
allowlist: [],
|
||||
ceiling: { maxFiles: 3, grace: 0 },
|
||||
});
|
||||
assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`);
|
||||
});
|
||||
|
||||
test('passes when exemption-file count == ceiling - 1 (limit-1 boundary)', () => {
|
||||
// 2 files, ceiling {maxFiles:3, grace:1} -> slack = 3-2 = 1, not > grace.
|
||||
// NOTE: grace:0 here (matching the sibling boundary tests) would fail on
|
||||
// the SEPARATE slack-ratchet check (slack 1 > grace 0), not the count
|
||||
// check this test targets, so grace is widened to 1 to isolate the axis
|
||||
// under test.
|
||||
const r = runLint({
|
||||
files: {
|
||||
'a.test.cjs': `// ${MARKER} see #123\n`,
|
||||
'b.test.cjs': `// ${MARKER} see #123\n`,
|
||||
},
|
||||
allowlist: [],
|
||||
ceiling: { maxFiles: 3, grace: 1 },
|
||||
});
|
||||
assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`);
|
||||
});
|
||||
|
||||
test('fails when exemption-file count == ceiling + 1 (limit+1 boundary)', () => {
|
||||
const r = runLint({
|
||||
files: {
|
||||
'a.test.cjs': `// ${MARKER} see #123\n`,
|
||||
'b.test.cjs': `// ${MARKER} see #123\n`,
|
||||
'c.test.cjs': `// ${MARKER} see #123\n`,
|
||||
'd.test.cjs': `// ${MARKER} see #123\n`,
|
||||
},
|
||||
allowlist: [],
|
||||
ceiling: { maxFiles: 3, grace: 0 },
|
||||
});
|
||||
assert.notStrictEqual(r.status, 0);
|
||||
assert.match(r.stderr, /exceeds budget ceiling/);
|
||||
});
|
||||
|
||||
test('passes when slack == grace exactly (boundary)', () => {
|
||||
// 2 files, ceiling {maxFiles:5, grace:3} -> slack = 5-2 = 3, not > grace.
|
||||
const r = runLint({
|
||||
files: {
|
||||
'a.test.cjs': `// ${MARKER} see #123\n`,
|
||||
'b.test.cjs': `// ${MARKER} see #123\n`,
|
||||
},
|
||||
allowlist: [],
|
||||
ceiling: { maxFiles: 5, grace: 3 },
|
||||
});
|
||||
assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`);
|
||||
});
|
||||
|
||||
test('passes when slack == grace - 1 (limit-1 boundary)', () => {
|
||||
// 3 files, ceiling {maxFiles:5, grace:3} -> slack = 5-3 = 2, less than grace (2 < 3, not > grace).
|
||||
const r = runLint({
|
||||
files: {
|
||||
'a.test.cjs': `// ${MARKER} see #123\n`,
|
||||
'b.test.cjs': `// ${MARKER} see #123\n`,
|
||||
'c.test.cjs': `// ${MARKER} see #123\n`,
|
||||
},
|
||||
allowlist: [],
|
||||
ceiling: { maxFiles: 5, grace: 3 },
|
||||
});
|
||||
assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`);
|
||||
});
|
||||
|
||||
test('fails when slack == grace + 1 (one past boundary)', () => {
|
||||
// 1 file, ceiling {maxFiles:5, grace:3} -> slack = 5-1 = 4 > grace.
|
||||
const r = runLint({
|
||||
files: {
|
||||
'a.test.cjs': `// ${MARKER} see #123\n`,
|
||||
},
|
||||
allowlist: [],
|
||||
ceiling: { maxFiles: 5, grace: 3 },
|
||||
});
|
||||
assert.notStrictEqual(r.status, 0);
|
||||
assert.match(r.stderr, /Tighten the ceiling/);
|
||||
});
|
||||
|
||||
test('a cited exemption passes the citation check with an empty allowlist', () => {
|
||||
const r = runLint({
|
||||
files: { 'cited.test.cjs': `// ${MARKER} see #456\n` },
|
||||
allowlist: [],
|
||||
ceiling: { maxFiles: 100, grace: 100 },
|
||||
});
|
||||
assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`);
|
||||
});
|
||||
|
||||
test('the SAME cited exemption still counts toward the ceiling total', () => {
|
||||
test('the SAME cited exemption (no adjacent violation) still counts toward the unverified-files ceiling', () => {
|
||||
// This marker has NOTHING to suppress (no fs.readFileSync at all in the
|
||||
// file), so under the effective/unverified split it lands in the
|
||||
// UNVERIFIED pool, not the effective one — distinct from the old
|
||||
// single-ceiling behavior this replaces.
|
||||
const r = runLint({
|
||||
files: { 'cited.test.cjs': `// ${MARKER} see #456\n` },
|
||||
allowlist: [],
|
||||
ceiling: { maxFiles: 0, grace: 0 },
|
||||
unverifiedCeiling: { maxFiles: 0 },
|
||||
});
|
||||
assert.notStrictEqual(r.status, 0);
|
||||
assert.match(r.stderr, /allow-test-rule-total-files/);
|
||||
assert.match(r.stderr, /allow-test-rule-unverified-markers/);
|
||||
});
|
||||
|
||||
test('a duplicate uncited reason in one file dedupes to one identifier', () => {
|
||||
const files = {
|
||||
'dup.test.cjs': `// ${MARKER} dup-reason\n// ${MARKER} dup-reason\n`,
|
||||
};
|
||||
const testsDir = path.join(sandbox, `tests-${fixtureCount}`);
|
||||
fs.mkdirSync(testsDir, { recursive: true });
|
||||
const full = path.join(testsDir, 'dup.test.cjs');
|
||||
fs.writeFileSync(full, files['dup.test.cjs']);
|
||||
const relpath = relToRoot(full);
|
||||
const allowlistPath = path.join(sandbox, `allowlist-${fixtureCount}.json`);
|
||||
const relpath = relToRoot(path.join(testsDir, 'dup.test.cjs'));
|
||||
// A SINGLE allowlist entry suffices — if the dedupe regressed (two
|
||||
// identifiers produced), this single-entry allowlist would leave one
|
||||
// novel offender and fail.
|
||||
fs.writeFileSync(allowlistPath, JSON.stringify([`${relpath} :: dup-reason`]));
|
||||
const ceilingPath = path.join(sandbox, `ceiling-${fixtureCount}.json`);
|
||||
fs.writeFileSync(ceilingPath, JSON.stringify({ maxFiles: 100, grace: 100 }));
|
||||
fixtureCount += 1;
|
||||
|
||||
const r = toLegacyResult(
|
||||
runNode([SCRIPT], {
|
||||
cwd: ROOT,
|
||||
timeoutMs: PROBE_TIMEOUT_MS,
|
||||
env: {
|
||||
...process.env,
|
||||
GSD_LINT_ALLOW_TEST_RULE_TESTS_DIR: testsDir,
|
||||
GSD_LINT_ALLOW_TEST_RULE_ALLOWLIST: allowlistPath,
|
||||
GSD_LINT_ALLOW_TEST_RULE_CEILING: ceilingPath,
|
||||
},
|
||||
})
|
||||
);
|
||||
const r = runLint({
|
||||
files: { 'dup.test.cjs': `// ${MARKER} dup-reason\n// ${MARKER} dup-reason\n` },
|
||||
allowlist: [`${relpath} :: dup-reason`],
|
||||
});
|
||||
assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`);
|
||||
});
|
||||
|
||||
test('repo baseline passes (real tests/ dir against real allowlist + ceiling)', () => {
|
||||
const r = runNode([SCRIPT], { cwd: ROOT, timeoutMs: PROBE_TIMEOUT_MS });
|
||||
assert.strictEqual(r.exitCode, 0, `stderr: ${r.stderr}\nstdout: ${r.stdout}`);
|
||||
});
|
||||
|
||||
test('unknown CLI arguments are rejected with exit code 2', () => {
|
||||
const r = runLint({ files: {}, allowlist: [], args: ['--bogus'] });
|
||||
assert.strictEqual(r.status, 2);
|
||||
assert.match(r.stderr, /unknown argument/);
|
||||
assert.match(r.stderr, /--bogus/);
|
||||
});
|
||||
|
||||
test('repo baseline passes (real tests/ dir against real allowlist + ceilings)', () => {
|
||||
const r = runNode([SCRIPT], { cwd: ROOT, timeoutMs: REPO_BASELINE_LINT_TIMEOUT_MS });
|
||||
assert.strictEqual(r.exitCode, 0, `stderr: ${r.stderr}\nstdout: ${r.stdout}`);
|
||||
assert.match(r.stdout, /effective exemptions:/);
|
||||
assert.match(r.stdout, /unverified markers:/);
|
||||
assert.match(r.stdout, /Known limit:/);
|
||||
});
|
||||
|
||||
// ─── #3520 test-matrix rows 1-12 ────────────────────────────────────────
|
||||
|
||||
test('row 1: marker adjacent to a detected violation counts as effective, not unverified', () => {
|
||||
const code = [
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
`// ${MARKER} reason (#1)`,
|
||||
"const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); src.includes('x');",
|
||||
].join('\n');
|
||||
// Pin the exact effective-site count via a boundary ceiling: passes at
|
||||
// exactly 1, fails at 0 — that is how this file proves "counted as
|
||||
// effective" without needing to parse the ok-message text.
|
||||
const pass = runLint({
|
||||
files: { 'row1.test.cjs': code },
|
||||
effectiveCeiling: { maxSites: 1, grace: 0 },
|
||||
unverifiedCeiling: { maxFiles: 0 }, // must NOT also count as unverified
|
||||
});
|
||||
assert.strictEqual(pass.status, 0, `stderr: ${pass.stderr}`);
|
||||
|
||||
const failsAtZero = runLint({
|
||||
files: { 'row1.test.cjs': code },
|
||||
effectiveCeiling: { maxSites: 0, grace: 0 },
|
||||
});
|
||||
assert.notStrictEqual(failsAtZero.status, 0);
|
||||
assert.match(failsAtZero.stderr, /allow-test-rule-effective-sites/);
|
||||
});
|
||||
|
||||
test('row 2: marker with no detectable violation counts as unverified, not effective', () => {
|
||||
const code = [
|
||||
`// ${MARKER} reason (#2)`,
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
"const content = fs.readFileSync(path.join(__dirname, '..', 'docs', 'readme.md'), 'utf-8');",
|
||||
"content.includes('hello');",
|
||||
].join('\n');
|
||||
const pass = runLint({
|
||||
files: { 'row2.test.cjs': code },
|
||||
effectiveCeiling: { maxSites: 0, grace: 0 }, // must NOT count as effective
|
||||
unverifiedCeiling: { maxFiles: 1 },
|
||||
});
|
||||
assert.strictEqual(pass.status, 0, `stderr: ${pass.stderr}`);
|
||||
|
||||
const failsAtZeroUnverified = runLint({
|
||||
files: { 'row2.test.cjs': code },
|
||||
unverifiedCeiling: { maxFiles: 0 },
|
||||
});
|
||||
assert.notStrictEqual(failsAtZeroUnverified.status, 0);
|
||||
assert.match(failsAtZeroUnverified.stderr, /allow-test-rule-unverified-markers/);
|
||||
});
|
||||
|
||||
test('row 3: no marker, no violation — counted in neither pool', () => {
|
||||
const code = [
|
||||
"const assert = require('node:assert/strict');",
|
||||
"assert.strictEqual(1 + 1, 2);",
|
||||
].join('\n');
|
||||
const r = runLint({
|
||||
files: { 'row3.test.cjs': code },
|
||||
effectiveCeiling: { maxSites: 0, grace: 0 },
|
||||
unverifiedCeiling: { maxFiles: 0 },
|
||||
});
|
||||
assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`);
|
||||
});
|
||||
|
||||
test('row 4: a violation with NO marker anywhere in the file is not flagged by this gate (#3464 perf follow-up narrowed live-violation detection to marker-bearing files; npm run lint / npm run lint:ci enforce unmarked files separately)', () => {
|
||||
const code = [
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
"const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); src.includes('x');",
|
||||
].join('\n');
|
||||
const r = runLint({ files: { 'row4.test.cjs': code } });
|
||||
assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`);
|
||||
});
|
||||
|
||||
test('row 4b: a live violation in a file that DOES carry a marker (elsewhere, unrelated) still fails the gate', () => {
|
||||
// Same shape as the parity corpus's "marker far above an unrelated later
|
||||
// violation" fixture: M1 suppresses V1, but V2 is far enough away (and
|
||||
// has no marker of its own) to stay live. Proves the marker-bearing
|
||||
// narrowing (row 4 above) does not also let a genuine live violation
|
||||
// slip through in a file this gate DOES still lint.
|
||||
const code = [
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
`// ${MARKER} reason for V1 (#1)`,
|
||||
"const s1 = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); s1.includes('x');",
|
||||
...Array.from({ length: 20 }, (_, i) => `// unrelated filler line ${i + 1}`),
|
||||
"const s2 = fs.readFileSync(path.join(__dirname, '..', 'lib', 'b.cjs'), 'utf-8'); s2.includes('y');",
|
||||
].join('\n');
|
||||
const r = runLint({ files: { 'row4b.test.cjs': code } });
|
||||
assert.notStrictEqual(r.status, 0);
|
||||
assert.match(r.stderr, /allow-test-rule-live-violations/);
|
||||
assert.match(r.stderr, /row4b\.test\.cjs/);
|
||||
});
|
||||
|
||||
test('row 5: a file with one effective site and one inert marker (nothing nearby to suppress) counts as effective only', () => {
|
||||
// "One suppressing, one not" per test-matrix row 5, realized without an
|
||||
// actual unsuppressed violation (which would make row 5 indistinguishable
|
||||
// from row 4's assertion): M1 suppresses a real V1; M2 sits above a .md
|
||||
// read the rule never flags, so it suppresses nothing because there is
|
||||
// nothing there to suppress. The file must still classify as effective
|
||||
// (>=1 effective site), never also unverified — no double counting.
|
||||
const code = [
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
`// ${MARKER} effective site (#1)`,
|
||||
"const s1 = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); s1.includes('x');",
|
||||
'',
|
||||
`// ${MARKER} inert marker, nothing nearby (#2)`,
|
||||
"const content = fs.readFileSync(path.join(__dirname, '..', 'docs', 'readme.md'), 'utf-8');",
|
||||
"content.includes('hello');",
|
||||
].join('\n');
|
||||
const r = runLint({
|
||||
files: { 'row5.test.cjs': code },
|
||||
effectiveCeiling: { maxSites: 1, grace: 0 },
|
||||
unverifiedCeiling: { maxFiles: 0 }, // the file must NOT also appear here
|
||||
});
|
||||
assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`);
|
||||
});
|
||||
|
||||
test('row 6: effective count exceeds its ceiling fails, message names effective sites', () => {
|
||||
const code = [
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
`// ${MARKER} reason (#1)`,
|
||||
"const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); src.includes('x');",
|
||||
].join('\n');
|
||||
const r = runLint({
|
||||
files: { 'row6.test.cjs': code },
|
||||
effectiveCeiling: { maxSites: 0, grace: 0 },
|
||||
});
|
||||
assert.notStrictEqual(r.status, 0);
|
||||
assert.match(r.stderr, /allow-test-rule-effective-sites/);
|
||||
assert.match(r.stderr, /exceeds budget ceiling/);
|
||||
});
|
||||
|
||||
test('row 7: effective count far below ceiling (slack beyond grace) fails — ratchet-down enforced', () => {
|
||||
const code = [
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
`// ${MARKER} reason (#1)`,
|
||||
"const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); src.includes('x');",
|
||||
].join('\n');
|
||||
// actualMax=1, ceiling=10, grace=2 -> slack=9 > grace -> fails.
|
||||
const r = runLint({
|
||||
files: { 'row7.test.cjs': code },
|
||||
effectiveCeiling: { maxSites: 10, grace: 2 },
|
||||
});
|
||||
assert.notStrictEqual(r.status, 0);
|
||||
assert.match(r.stderr, /allow-test-rule-effective-sites/);
|
||||
assert.match(r.stderr, /Tighten the ceiling/);
|
||||
});
|
||||
|
||||
test('row 8: unverified count exceeds its ceiling fails with a message distinct from row 6', () => {
|
||||
const code = [
|
||||
`// ${MARKER} reason (#2)`,
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
"const content = fs.readFileSync(path.join(__dirname, '..', 'docs', 'readme.md'), 'utf-8');",
|
||||
"content.includes('hello');",
|
||||
].join('\n');
|
||||
const r = runLint({
|
||||
files: { 'row8.test.cjs': code },
|
||||
unverifiedCeiling: { maxFiles: 0 },
|
||||
});
|
||||
assert.notStrictEqual(r.status, 0);
|
||||
assert.match(r.stderr, /allow-test-rule-unverified-markers/);
|
||||
assert.doesNotMatch(r.stderr, /allow-test-rule-effective-sites/);
|
||||
});
|
||||
|
||||
test('row 9: unverified count dropping below its ceiling still passes (not tightly ratcheted)', () => {
|
||||
const code = [
|
||||
"const assert = require('node:assert/strict');",
|
||||
"assert.ok(true);",
|
||||
].join('\n');
|
||||
// Zero unverified files, ceiling deliberately loose (100) -- a real
|
||||
// "tight" ratchet (like effective-sites) would force this down; the
|
||||
// unverified ceiling must not.
|
||||
const r = runLint({
|
||||
files: { 'row9.test.cjs': code },
|
||||
unverifiedCeiling: { maxFiles: 100 },
|
||||
});
|
||||
assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`);
|
||||
});
|
||||
|
||||
test('row 10: uncited marker is still an allowlist offender (citation contract unchanged)', () => {
|
||||
const r = runLint({
|
||||
files: { 'row10.test.cjs': `// ${MARKER} no citation here\n` },
|
||||
allowlist: [],
|
||||
});
|
||||
assert.notStrictEqual(r.status, 0);
|
||||
assert.match(r.stderr, /no citation here/);
|
||||
});
|
||||
|
||||
test('row 11: marker text inside a string literal is not a directive — the violation it sits above stays live', () => {
|
||||
const code = [
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
`const note = 'not a directive: ${MARKER} fake reason';`,
|
||||
"const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); src.includes(note);",
|
||||
].join('\n');
|
||||
const r = runLint({ files: { 'row11.test.cjs': code } });
|
||||
assert.notStrictEqual(r.status, 0);
|
||||
assert.match(r.stderr, /allow-test-rule-live-violations/);
|
||||
assert.match(r.stderr, /row11\.test\.cjs/);
|
||||
});
|
||||
|
||||
test('row 12: a NUL-byte file is discovered and classified without a shell grep or a crash', () => {
|
||||
const testsDir = path.join(sandbox, `tests-${fixtureCount}`);
|
||||
fs.mkdirSync(testsDir, { recursive: true });
|
||||
const full = path.join(testsDir, 'row12.test.cjs');
|
||||
// A literal NUL byte embedded in otherwise-valid-looking source, plus a
|
||||
// marker comment, written via a real byte buffer (never a shell
|
||||
// redirect/grep, which would choke on the NUL differently than Node's
|
||||
// own fs + parser do).
|
||||
const buf = Buffer.concat([
|
||||
Buffer.from(`// ${MARKER} reason (#1)\nconst x = '`),
|
||||
Buffer.from([0]),
|
||||
Buffer.from("';\n"),
|
||||
]);
|
||||
fs.writeFileSync(full, buf);
|
||||
const extraRoot = path.join(sandbox, `extra-${fixtureCount}`);
|
||||
fs.mkdirSync(extraRoot, { recursive: true });
|
||||
const effectiveCeilingPath = path.join(sandbox, `effective-ceiling-${fixtureCount}.json`);
|
||||
fs.writeFileSync(effectiveCeilingPath, JSON.stringify({ maxSites: 1000, grace: 1000 }));
|
||||
const unverifiedCeilingPath = path.join(sandbox, `unverified-ceiling-${fixtureCount}.json`);
|
||||
fs.writeFileSync(unverifiedCeilingPath, JSON.stringify({ maxFiles: 1000 }));
|
||||
const allowlistPath = path.join(sandbox, `allowlist-${fixtureCount}.json`);
|
||||
fs.writeFileSync(allowlistPath, JSON.stringify([]));
|
||||
fixtureCount += 1;
|
||||
|
||||
const result = runNode([SCRIPT], {
|
||||
cwd: ROOT,
|
||||
timeoutMs: PROBE_TIMEOUT_MS,
|
||||
env: {
|
||||
...process.env,
|
||||
GSD_LINT_ALLOW_TEST_RULE_TESTS_DIR: testsDir,
|
||||
GSD_LINT_ALLOW_TEST_RULE_EXTRA_ROOT: extraRoot,
|
||||
GSD_LINT_ALLOW_TEST_RULE_ALLOWLIST: allowlistPath,
|
||||
GSD_LINT_ALLOW_TEST_RULE_EFFECTIVE_CEILING: effectiveCeilingPath,
|
||||
GSD_LINT_ALLOW_TEST_RULE_UNVERIFIED_CEILING: unverifiedCeilingPath,
|
||||
},
|
||||
});
|
||||
const r = toLegacyResult(result);
|
||||
// Never crashes with an uncaught exception (no raw Node stack trace) —
|
||||
// it either passes or fails cleanly through the script's own messaging.
|
||||
assert.ok(
|
||||
r.status === 0 || r.status === 1,
|
||||
`unexpected exit status ${r.status}; stderr: ${r.stderr}`
|
||||
);
|
||||
assert.doesNotMatch(r.stderr, /at Object\.<anonymous>/);
|
||||
assert.doesNotMatch(r.stderr, /SyntaxError/);
|
||||
});
|
||||
|
||||
// ─── #3464 phase 5 BLOCKER fix rows: widened scan scope + loud parse failure ───
|
||||
|
||||
test('row 16: a non-.test.cjs file nested under tests/ carrying a marker is counted (unverified) — the live-command-registry.cjs case', () => {
|
||||
// A prior version of the scan only walked `tests/**/*.test.cjs`, so a
|
||||
// marker in a `tests/helpers/*.cjs`-shaped file (this repo's real
|
||||
// tests/helpers/live-command-registry.cjs instance) was invisible to
|
||||
// BOTH reported numbers. This file has no adjacent readFileSync+search
|
||||
// call, so it lands in the unverified pool, not effective.
|
||||
const code = [
|
||||
`// ${MARKER} reason (#2)`,
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
"const content = fs.readFileSync(path.join(__dirname, '..', 'docs', 'readme.md'), 'utf-8');",
|
||||
"content.includes('hello');",
|
||||
].join('\n');
|
||||
const pass = runLint({
|
||||
files: { 'helpers/row16.cjs': code },
|
||||
effectiveCeiling: { maxSites: 0, grace: 0 }, // must NOT count as effective
|
||||
unverifiedCeiling: { maxFiles: 1 },
|
||||
});
|
||||
assert.strictEqual(pass.status, 0, `stderr: ${pass.stderr}`);
|
||||
|
||||
const failsAtZero = runLint({
|
||||
files: { 'helpers/row16.cjs': code },
|
||||
unverifiedCeiling: { maxFiles: 0 },
|
||||
});
|
||||
assert.notStrictEqual(failsAtZero.status, 0);
|
||||
assert.match(failsAtZero.stderr, /allow-test-rule-unverified-markers/);
|
||||
});
|
||||
|
||||
test('row 17: a marker in a NON-tests registered glob (scripts/**/*.cjs-shaped sandbox path) is counted as effective', () => {
|
||||
// Exercises the widened scan scope itself (not just the tests/ side):
|
||||
// `eslint.config.mjs` also registers local/no-source-grep on
|
||||
// scripts/**/*.cjs (among others), and this must now actually be
|
||||
// scanned, not merely assumed reachable in theory.
|
||||
const code = [
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
`// ${MARKER} reason (#1)`,
|
||||
"const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); src.includes('x');",
|
||||
].join('\n');
|
||||
const pass = runLint({
|
||||
files: {},
|
||||
extraFiles: { 'scripts/row17-fixture.cjs': code },
|
||||
effectiveCeiling: { maxSites: 1, grace: 0 },
|
||||
unverifiedCeiling: { maxFiles: 0 },
|
||||
});
|
||||
assert.strictEqual(pass.status, 0, `stderr: ${pass.stderr}`);
|
||||
|
||||
const failsAtZero = runLint({
|
||||
files: {},
|
||||
extraFiles: { 'scripts/row17-fixture.cjs': code },
|
||||
effectiveCeiling: { maxSites: 0, grace: 0 },
|
||||
});
|
||||
assert.notStrictEqual(failsAtZero.status, 0);
|
||||
assert.match(failsAtZero.stderr, /allow-test-rule-effective-sites/);
|
||||
});
|
||||
|
||||
test('row 18: an unparseable file makes the gate throw loudly instead of counting it as clean', () => {
|
||||
// Sibling to the "No matching configuration found" case, which already
|
||||
// throws — before this fix, a parse/verify failure inside
|
||||
// classifySiteLines was silently swallowed into `{effectiveLines: [],
|
||||
// liveLines: []}`, indistinguishable from a genuinely clean file. Genuine
|
||||
// invalid JS syntax (unbalanced braces / stray token), not just an
|
||||
// embedded NUL byte (row 12 proves NUL bytes alone parse fine).
|
||||
//
|
||||
// Must carry a marker (#3464 perf follow-up): the script now only drives
|
||||
// the Linter over marker-bearing files (a cheap substring pre-filter
|
||||
// over raw content — see the script's "Linter scope" doc comment), so an
|
||||
// unparseable file with NO marker at all is out of scope for THIS gate
|
||||
// entirely (an unparseable file fails `npm run lint` on its own, via
|
||||
// ESLint's own parse error, independent of this script). The marker
|
||||
// comment itself is valid syntax; only what follows it is broken.
|
||||
const code = `// ${MARKER} reason (#1)\nconst x = {{{ this is not valid javascript syntax ]`;
|
||||
const r = runLint({ files: { 'row18.test.cjs': code } });
|
||||
assert.notStrictEqual(r.status, 0);
|
||||
assert.match(r.stderr, /failed to parse\/verify/);
|
||||
assert.match(r.stderr, /row18\.test\.cjs/);
|
||||
// Never the "ok" success message — a parse failure must never present as
|
||||
// a clean run.
|
||||
assert.doesNotMatch(r.stdout, /^ok lint-allow-test-rule-refs/);
|
||||
});
|
||||
|
||||
// ─── #3520 test-matrix rows 13-15: structural guard (parity + teeth) ───
|
||||
|
||||
test('row 13: the script imports the predicate/detection from the rule module — no second copy of the adjacency arithmetic', () => {
|
||||
const scriptSource = fs.readFileSync(SCRIPT, 'utf8');
|
||||
// Imports the rule module directly.
|
||||
assert.match(scriptSource, /require\(['"]\.\.\/eslint-rules\/no-source-grep\.cjs['"]\)/);
|
||||
// Calls the rule's own exported predicate/collector, not a re-derivation.
|
||||
assert.match(scriptSource, /\.isSuppressedAt\(/);
|
||||
assert.match(scriptSource, /\.collectMarkerAndCommentLines\(/);
|
||||
// No duplicated lookahead constant or adjacency loop in the script's own
|
||||
// source — the tell for a hand-rolled second copy of isSuppressedAt's
|
||||
// arithmetic would be a re-declared "MAX_MARKER_LOOKAHEAD_LINES ="
|
||||
// assignment or a hand-written `markerLine + 1; l < violationLine`
|
||||
// purity loop.
|
||||
assert.doesNotMatch(scriptSource, /MAX_MARKER_LOOKAHEAD_LINES\s*=\s*\d/);
|
||||
assert.doesNotMatch(scriptSource, /markerLine \+ 1;\s*l\s*<\s*violationLine/);
|
||||
// Reference identity: what the script requires IS the same object the
|
||||
// rule module exports (not a structurally-similar copy).
|
||||
assert.strictEqual(
|
||||
require('../eslint-rules/no-source-grep.cjs').isSuppressedAt,
|
||||
noSourceGrepRule.isSuppressedAt
|
||||
);
|
||||
});
|
||||
|
||||
test('row 14: parity — the script\'s suppressed verdict equals the real rule\'s independent report/no-report outcome, for every site in a fixture corpus', () => {
|
||||
// Ground truth, computed WITHOUT using isSuppressedAt or
|
||||
// collectMarkerAndCommentLines at all: run the REAL (non-neutralized)
|
||||
// `no-source-grep` rule through ESLint's Linter and record which lines
|
||||
// it reports. This is an independent code path from
|
||||
// scriptUnderTest.classifySiteLines (which calls the rule's exported
|
||||
// predicate directly) — the two must still agree everywhere, because
|
||||
// both are ultimately driven by the same rule.
|
||||
function realRuleReportedLines(code, filename) {
|
||||
const linter = new Linter({ configType: 'flat' });
|
||||
const config = {
|
||||
languageOptions: { ecmaVersion: 2022, sourceType: 'commonjs' },
|
||||
plugins: { local: { rules: { 'no-source-grep': noSourceGrepRule } } },
|
||||
rules: { 'local/no-source-grep': 'error' },
|
||||
};
|
||||
return linter
|
||||
.verify(code, config, filename)
|
||||
.filter((m) => m.messageId === 'noSourceGrep')
|
||||
.map((m) => m.line)
|
||||
.sort((a, b) => a - b);
|
||||
}
|
||||
|
||||
const AT = MARKER; // already split above; reuse for fixture text
|
||||
|
||||
const corpus = [
|
||||
// adjacent marker suppresses its violation
|
||||
[
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
`// ${AT} reason (#1)`,
|
||||
"const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); src.includes('x');",
|
||||
].join('\n'),
|
||||
// no marker: violation stays live
|
||||
[
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
"const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'b.cjs'), 'utf-8'); src.includes('x');",
|
||||
].join('\n'),
|
||||
// marker far above an unrelated later violation: does not reach it
|
||||
[
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
`// ${AT} reason for V1 (#1)`,
|
||||
"const s1 = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); s1.includes('x');",
|
||||
...Array.from({ length: 20 }, (_, i) => `// unrelated filler line ${i + 1}`),
|
||||
"const s2 = fs.readFileSync(path.join(__dirname, '..', 'lib', 'b.cjs'), 'utf-8'); s2.includes('y');",
|
||||
].join('\n'),
|
||||
// marker in a string literal does not suppress
|
||||
[
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
`const note = 'not a directive: ${AT} fake reason';`,
|
||||
"const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); src.includes(note);",
|
||||
].join('\n'),
|
||||
// marker directly above the read, search several comment-pure lines later
|
||||
[
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
`// ${AT} reason (#1)`,
|
||||
"const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf8');",
|
||||
'// comment-pure line one',
|
||||
'// comment-pure line two',
|
||||
"src.includes('x');",
|
||||
].join('\n'),
|
||||
// boundary: marker exactly MAX_MARKER_LOOKAHEAD_LINES (8) above is suppressed
|
||||
[
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
`// ${AT} reason (#1)`,
|
||||
...Array.from({ length: 7 }, (_, i) => `// filler comment line ${i + 1}`),
|
||||
"const s = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf8'); s.includes('x');",
|
||||
].join('\n'),
|
||||
// boundary: marker one line beyond the lookahead is not suppressed
|
||||
[
|
||||
"const fs = require('fs');",
|
||||
"const path = require('path');",
|
||||
`// ${AT} reason (#1)`,
|
||||
...Array.from({ length: 8 }, (_, i) => `// filler comment line ${i + 1}`),
|
||||
"const s = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf8'); s.includes('x');",
|
||||
].join('\n'),
|
||||
// no violation at all
|
||||
[
|
||||
"const assert = require('node:assert/strict');",
|
||||
"assert.ok(true);",
|
||||
].join('\n'),
|
||||
];
|
||||
|
||||
for (const [i, code] of corpus.entries()) {
|
||||
const filename = `tests/parity-fixture-${i}.test.cjs`;
|
||||
const { liveLines } = scriptUnderTest.classifyCode(code, filename);
|
||||
const realLines = realRuleReportedLines(code, filename);
|
||||
assert.deepStrictEqual(
|
||||
[...liveLines].sort((a, b) => a - b),
|
||||
realLines,
|
||||
`fixture ${i}: script's live-line verdict disagreed with the real rule's report/no-report outcome`
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('row 15 (teeth for row 14, by construction): the exported isSuppressedAt is the SAME function the rule calls internally, so its default lookahead cannot diverge from the rule\'s', () => {
|
||||
// Runtime mutation of MAX_MARKER_LOOKAHEAD_LINES is not possible: it is
|
||||
// declared `const` at module scope inside eslint-rules/no-source-grep.cjs
|
||||
// and isSuppressedAt's default parameter closes over that lexical
|
||||
// binding directly, not over a mutable exported property — reassigning
|
||||
// `noSourceGrepRule.MAX_MARKER_LOOKAHEAD_LINES` would silently do
|
||||
// nothing to the real default. That immutability is deliberate (the
|
||||
// constant is not meant to be a runtime knob), so per the brief this
|
||||
// teeth check is asserted BY CONSTRUCTION rather than by faking a
|
||||
// mutation:
|
||||
//
|
||||
// 1. isSuppressedAt is a single exported function (verified by
|
||||
// reference identity in row 13) — the script never has its own copy.
|
||||
// 2. Its `maxLookahead` parameter is genuinely load-bearing (not a
|
||||
// decorative unused default): explicitly overriding it changes the
|
||||
// verdict at the exact boundary where the DEFAULT (backed by
|
||||
// MAX_MARKER_LOOKAHEAD_LINES) says "suppressed".
|
||||
// 3. Because the rule's own internal isSuppressed() call site and the
|
||||
// script's classification call site both invoke this identical
|
||||
// function with NO override (so both fall back to the same
|
||||
// MAX_MARKER_LOOKAHEAD_LINES default), any future edit to that
|
||||
// constant necessarily moves both call sites together — there is no
|
||||
// second constant anywhere to drift out of step with it.
|
||||
// Default (no override) -> uses MAX_MARKER_LOOKAHEAD_LINES: land exactly
|
||||
// at the real boundary using the exported constant itself, so this test
|
||||
// tracks the constant's actual value rather than hardcoding "8".
|
||||
const gapAtBoundary = noSourceGrepRule.MAX_MARKER_LOOKAHEAD_LINES;
|
||||
const atBoundary = noSourceGrepRule.isSuppressedAt({
|
||||
markerLines: [1],
|
||||
violationLine: 1 + gapAtBoundary,
|
||||
commentLineSet: new Set(Array.from({ length: gapAtBoundary }, (_, i) => i + 2)),
|
||||
lines: Array.from({ length: gapAtBoundary + 1 }, () => '// filler'),
|
||||
});
|
||||
assert.strictEqual(atBoundary, true, 'default lookahead should suppress exactly at the boundary');
|
||||
|
||||
// Same fixture, EXPLICIT override tighter than the real default: proves
|
||||
// the parameter actually drives the verdict (not ignored), which is the
|
||||
// load-bearing fact that makes "both call sites use the same default"
|
||||
// meaningful rather than vacuous.
|
||||
const overridden = noSourceGrepRule.isSuppressedAt({
|
||||
markerLines: [1],
|
||||
violationLine: 1 + gapAtBoundary,
|
||||
commentLineSet: new Set(Array.from({ length: gapAtBoundary }, (_, i) => i + 2)),
|
||||
lines: Array.from({ length: gapAtBoundary + 1 }, () => '// filler'),
|
||||
maxLookahead: gapAtBoundary - 1,
|
||||
});
|
||||
assert.strictEqual(overridden, false, 'an explicit tighter lookahead must change the verdict');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user