refactor(test): read the ignore sets with the TypeScript parser, not a hand-rolled scanner

The guards read ignore-service.ts as source because the sets are module-private.
The first pass hand-rolled a character scanner to do it, and the repo already
vendors the right tool: ts.createSourceFile, used this way in literal-collectors,
query-determinism-guard, cli-index-help and group/sync-partial-extraction.

The scanner had two silent gaps a real parser does not have:

- It rejected `${` by substring, but template literals were consumed whole, so
  that branch could never fire and an interpolated member was accepted as a
  literal — the exact under-report the file refused to allow.
- It took the first `[` after the marker, which on a type-annotated declaration
  (`readonly string[] = ...`) is the annotation's empty pair. It returned [] with
  no throw, which would make every assertion in a suite vacuously true. This is
  the hazard receiver-twin-list-drift.test.ts documents having hit.

Reading the declaration node removes both, along with the comment-vs-string
ordering problem that motivated the scanner: a parser cannot mistake a comment
for a string or a glob's `/*` for a comment-open.

Also drops the four pinned exact counts. They were a ratchet — these sets are
edited by unrelated PRs, each of which would have failed a count assertion about
nothing it touched — and with a real parser the partial-parse hazard they existed
to catch cannot happen silently: a member that is not a plain string literal
throws.

Markers collapse to set names, and the duplicated path-resolution boilerplate
moves into the helper the two suites already share.

Net 187 deletions against 123 insertions. Verified by mutation: backtick,
inline comment, same-line, double-quote, duplicate, interpolation, spread and
runtime .add() are all caught; a type-annotated declaration now reads correctly
instead of returning empty. 194 tests pass, tsc clean.
This commit is contained in:
Gergo Magyar 2026-08-27 11:50:32 +00:00
parent 1dc5328af5
commit 3accd3749e
3 changed files with 127 additions and 191 deletions

View file

@ -1,139 +1,107 @@
/**
* Source-parsing extractor for the bare-name sets in `src/config/ignore-service.ts`.
* Reads the bare-name sets in `src/config/ignore-service.ts` out of source.
*
* Those sets are module-private, and exporting them purely to be testable would
* widen a production surface to satisfy a test the same call
* widen a production surface to satisfy a test the call
* `receiver-twin-list-drift.test.ts` documents. So the guards read the source
* text instead, and this helper is the one parser they share. It lives here
* rather than inside a single test file because two suites need it: the
* slash-free guard and the cross-package drift guard.
* instead, through the TypeScript parser the repo already vendors and already
* uses this way (`literal-collectors.ts`, `query-determinism-guard.test.ts`,
* `cli-index-help.test.ts`).
*
* It is a single-pass scanner rather than a regex chain, because comments and
* strings can each contain the other's delimiters and neither can be removed
* independently:
* Using the real parser is what makes the guards trustworthy. A text scanner has
* to decide whether a delimiter opens a comment or sits inside a string, and it
* gets that wrong in both directions here: the ignore-list comments quote paths
* and carry an apostrophe (`Next.js's`), while a glob string such as `'** / *'`
* contains a comment-open sequence. It also has to guess which bracket belongs
* to the declaration rather than to a type annotation. Each of those is a way to
* silently read fewer members and a guard that quietly stops seeing members is
* the exact defect these guards exist to catch.
*
* - The ignore-list comments quote paths and carry an apostrophe (`Next.js's`),
* so matching literals before removing comments yields phantom entries
* several slash-bearing, which would fail the slash assertion on correct
* source.
* - A glob string such as `'**/*'` contains a comment-open sequence, so
* removing comments with a regex first swallows the rest of the declaration
* and the scan runs past the closing bracket.
*
* Tracking string and comment state in one pass is the only ordering that is
* correct in both directions.
*
* A source parser still cannot resolve a spread, an interpolation, a
* concatenation, or a later `.add(...)`. Those are rejected loudly rather than
* silently reducing the entry count, because a guard that quietly stops seeing
* members is the defect these guards exist to catch.
* `setEntries` therefore refuses anything that is not a plain list of string
* literals, rather than skipping the members it cannot resolve.
*/
import { readFileSync } from 'node:fs';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import ts from 'typescript';
/** Shapes a source-text parser cannot resolve to a fixed list of string literals. */
const UNRESOLVABLE_SHAPES = ['...', '${', '+'] as const;
const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..', '..');
interface ScanResult {
/** String literals declared directly in the block. */
entries: string[];
/** Block text with comments and string bodies removed, for shape checks. */
skeleton: string;
}
/** The analyzer's ignore rules — the sets every guard in this family reads. */
export const IGNORE_SERVICE_PATH = path.join(
REPO_ROOT,
'gitnexus',
'src',
'config',
'ignore-service.ts',
);
/** The browser upload pre-filter, whose excluded-directory set must not drift from the above. */
export const UPLOAD_FILTER_PATH = path.join(
REPO_ROOT,
'gitnexus-web',
'src',
'lib',
'upload-filter.ts',
);
export const readSource = (file: string): string => readFileSync(file, 'utf8');
/**
* Walk the bracketed block that starts at `open`, collecting string literals and
* a comment-free, string-free skeleton. Returns null when the block never closes.
*/
const scanBlock = (source: string, open: number): ScanResult | null => {
const entries: string[] = [];
let skeleton = '';
let depth = 0;
for (let i = open; i < source.length; i += 1) {
const ch = source[i];
const next = source[i + 1];
if (ch === '/' && next === '*') {
const end = source.indexOf('*/', i + 2);
if (end === -1) return null;
i = end + 1;
continue;
}
if (ch === '/' && next === '/') {
const end = source.indexOf('\n', i + 2);
if (end === -1) return null;
i = end;
continue;
}
if (ch === "'" || ch === '"' || ch === '`') {
const quote = ch;
let literal = '';
let j = i + 1;
for (; j < source.length; j += 1) {
if (source[j] === '\\') {
literal += source[j + 1] ?? '';
j += 1;
continue;
}
if (source[j] === quote) break;
literal += source[j];
}
if (j >= source.length) return null;
if (literal.length > 0) entries.push(literal);
i = j;
continue;
}
if (ch === '[') depth += 1;
if (ch === ']') {
depth -= 1;
if (depth === 0) return { entries, skeleton };
}
skeleton += ch;
}
return null;
};
/**
* String literals declared in the `[...]` block introduced by `marker`.
* The string literals `setName` is constructed from, in declaration order.
*
* Throws never returns a short list when the marker is missing, the block is
* malformed, or the declaration contains a member this parser cannot resolve.
* Throws never returns a short list when the declaration is missing or holds
* anything other than plain string literals (a spread, an interpolation, a
* concatenation, a computed value).
*/
export const setEntries = (source: string, marker: string): string[] => {
const at = source.indexOf(marker);
if (at === -1) {
throw new Error(`${marker} not found in ignore-service.ts — update this test`);
}
export const setEntries = (source: string, setName: string): string[] => {
const sourceFile = ts.createSourceFile(
'ignore-set-source.ts',
source,
ts.ScriptTarget.Latest,
true,
);
const open = source.indexOf('[', at);
if (open === -1) {
throw new Error(`${marker}: no bracketed block follows the marker`);
}
const scanned = scanBlock(source, open);
if (scanned === null) {
throw new Error(`${marker}: bracketed block never closes`);
}
for (const shape of UNRESOLVABLE_SHAPES) {
if (scanned.skeleton.includes(shape)) {
throw new Error(
`${marker}: declaration contains \`${shape}\`, which a source parser cannot resolve. ` +
`Switch this set to a runtime assertion rather than letting the guard read fewer members.`,
);
let elements: ts.NodeArray<ts.Expression> | undefined;
const visit = (node: ts.Node): void => {
if (
elements === undefined &&
ts.isVariableDeclaration(node) &&
ts.isIdentifier(node.name) &&
node.name.text === setName &&
node.initializer !== undefined &&
ts.isNewExpression(node.initializer) &&
node.initializer.arguments?.length === 1 &&
ts.isArrayLiteralExpression(node.initializer.arguments[0])
) {
elements = node.initializer.arguments[0].elements;
return;
}
ts.forEachChild(node, visit);
};
visit(sourceFile);
if (elements === undefined) {
throw new Error(`${setName} is not declared as \`new Set([...])\` — update this test`);
}
return scanned.entries;
const unresolvable = elements.filter((element) => !ts.isStringLiteral(element));
if (unresolvable.length > 0) {
throw new Error(
`${setName} holds ${unresolvable.length} member(s) that are not plain string literals ` +
`(first: \`${unresolvable[0].getText(sourceFile)}\`). A source-reading guard cannot resolve ` +
`those, so switch this set to a runtime assertion rather than letting the guard see fewer members.`,
);
}
return elements.map((element) => (element as ts.StringLiteral).text);
};
/**
* True when `setName` is mutated by `.add(...)` anywhere in `source`.
*
* `setEntries` reads the declaration only, so a member appended afterwards would
* be invisible to it. The guards assert this is false rather than silently
* under-reporting.
* be invisible to it. The guards assert this is false rather than under-reporting.
*/
export const hasRuntimeAdd = (source: string, setName: string): boolean =>
new RegExp(`\\b${setName}\\s*\\.\\s*add\\s*\\(`).test(source);

View file

@ -1,9 +1,11 @@
import { describe, it, expect } from 'vitest';
import fs from 'node:fs';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { isHardcodedIgnoredDirectory, shouldIgnorePath } from '../../src/config/ignore-service.js';
import { hasRuntimeAdd, setEntries } from '../helpers/ignore-set-source.js';
import {
IGNORE_SERVICE_PATH,
hasRuntimeAdd,
readSource,
setEntries,
} from '../helpers/ignore-set-source.js';
/**
* Emitted build output must not be indexed as source (#3007).
@ -59,91 +61,67 @@ describe('build-output ignores', () => {
});
describe('single-component set guards', () => {
const source = fs.readFileSync(
path.resolve(
path.dirname(fileURLToPath(import.meta.url)),
'..',
'..',
'src',
'config',
'ignore-service.ts',
),
'utf8',
);
const source = readSource(IGNORE_SERVICE_PATH);
// Every one of these sets is compared against a single path component, so a
// member containing `/` is dead on arrival — the defect that left
// `'public/build'` inert. Counts are pinned exactly rather than floored: a
// floor cannot protect a two-member set, and it hides a partial parse.
const SETS = [
{ marker: 'const DEFAULT_IGNORE_LIST = new Set([', name: 'DEFAULT_IGNORE_LIST', size: 76 },
{ marker: 'const IGNORED_FILES = new Set([', name: 'IGNORED_FILES', size: 33 },
{
marker: 'const ROOT_ARTIFACT_DIRECTORIES = new Set([',
name: 'ROOT_ARTIFACT_DIRECTORIES',
size: 2,
},
{ marker: 'const IGNORED_EXTENSIONS = new Set([', name: 'IGNORED_EXTENSIONS', size: 104 },
// `'public/build'` inert.
const SET_NAMES = [
'DEFAULT_IGNORE_LIST',
'IGNORED_FILES',
'ROOT_ARTIFACT_DIRECTORIES',
'IGNORED_EXTENSIONS',
] as const;
it.each(SETS)('$name holds no slash-bearing member', ({ marker }) => {
expect(setEntries(source, marker).filter((entry) => entry.includes('/'))).toEqual([]);
it.each(SET_NAMES)('%s holds no slash-bearing member', (setName) => {
expect(setEntries(source, setName).filter((entry) => entry.includes('/'))).toEqual([]);
});
it.each(SETS)('$name parses to its pinned size', ({ marker, size }) => {
expect(setEntries(source, marker)).toHaveLength(size);
});
it.each(SETS)('$name holds no duplicate member', ({ marker }) => {
const entries = setEntries(source, marker);
it.each(SET_NAMES)('%s holds no duplicate member', (setName) => {
const entries = setEntries(source, setName);
expect(new Set(entries).size).toBe(entries.length);
});
it.each(SETS)('$name is never mutated by .add() after construction', ({ name }) => {
expect(hasRuntimeAdd(source, name)).toBe(false);
it.each(SET_NAMES)('%s is never mutated by .add() after construction', (setName) => {
expect(hasRuntimeAdd(source, setName)).toBe(false);
});
it('every IGNORED_EXTENSIONS member starts with a dot', () => {
const entries = setEntries(source, 'const IGNORED_EXTENSIONS = new Set([');
const entries = setEntries(source, 'IGNORED_EXTENSIONS');
expect(entries.filter((entry) => !entry.startsWith('.'))).toEqual([]);
});
it('reads entries the declaration holds, not text the comments quote', () => {
// The comments in DEFAULT_IGNORE_LIST quote paths and carry an apostrophe
// (`Next.js's`). Matching literals before stripping them yields phantom
// entries, several slash-bearing, which would fail the slash assertion on
// correct source.
const entries = setEntries(source, 'const DEFAULT_IGNORE_LIST = new Set([');
// (`Next.js's`), so a text scanner reads phantom entries out of them —
// several slash-bearing, which would fail the assertion above on correct
// source. Parsing the declaration cannot see comments at all.
const entries = setEntries(source, 'DEFAULT_IGNORE_LIST');
expect(entries).toContain('_next');
expect(entries).not.toContain('public/build');
expect(entries).not.toContain('env/');
expect(entries).not.toContain('packages');
});
it('agrees with the runtime set it claims to describe', () => {
// Catches parser drift without exporting the set: every name the parser
// reports must actually be ignored by the module's own predicate.
const entries = setEntries(source, 'const DEFAULT_IGNORE_LIST = new Set([');
// Catches drift between what the guard reads and what the module does,
// without exporting the set.
const entries = setEntries(source, 'DEFAULT_IGNORE_LIST');
expect(entries.filter((entry) => !isHardcodedIgnoredDirectory(entry))).toEqual([]);
});
it('fails loudly when the marker no longer matches', () => {
expect(() => setEntries(source, 'const NOT_A_REAL_SET = new Set([')).toThrow(
/not found in ignore-service\.ts/,
);
it('fails loudly when a set is no longer declared as a Set of literals', () => {
expect(() => setEntries(source, 'NOT_A_REAL_SET')).toThrow(/update this test/);
});
it('fails loudly rather than under-reporting an unresolvable declaration', () => {
it('refuses a declaration whose members it cannot resolve', () => {
// A spread, an interpolation, or a concatenation resolves at runtime, not
// in source text. Parsing fewer members and passing is the failure mode
// these guards exist to prevent, so the parser refuses instead.
// in source. Reading the resolvable members and passing is the failure mode
// these guards exist to prevent, so the reader refuses instead.
const poisoned = source.replace(
'const ROOT_ARTIFACT_DIRECTORIES = new Set([',
'const ROOT_ARTIFACT_DIRECTORIES = new Set([...OTHER_NAMES,',
);
expect(() => setEntries(poisoned, 'const ROOT_ARTIFACT_DIRECTORIES = new Set([')).toThrow(
/cannot resolve/,
);
expect(() => setEntries(poisoned, 'ROOT_ARTIFACT_DIRECTORIES')).toThrow(/not plain string/);
});
});
});

View file

@ -30,32 +30,22 @@
* directions, so re-adding `.gitnexus` to the CLI list or dropping it from the
* web list both fail loudly.
*
* Structural (source-parsed) rather than value-imported: `DEFAULT_IGNORE_LIST`
* is module-private and exporting it purely to be testable would widen a
* production surface to satisfy a test. `EXCLUDED_DIRS` is exported, but no test
* in this package imports across the package boundary every cross-package
* precedent here reads source instead so both sides use the same parser.
* Both sides are read from source rather than imported. `DEFAULT_IGNORE_LIST` is
* module-private. `EXCLUDED_DIRS` is exported, but `upload-filter.ts` types its
* inputs with the DOM `File` interface, which does not resolve under this
* package's `lib: ["ES2022"] / types: ["node"]` so importing it here would not
* typecheck.
*/
import { describe, it, expect } from 'vitest';
import { readFileSync } from 'node:fs';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { setEntries } from '../helpers/ignore-set-source.js';
import {
IGNORE_SERVICE_PATH,
UPLOAD_FILTER_PATH,
readSource,
setEntries,
} from '../helpers/ignore-set-source.js';
const __dirname = path.dirname(fileURLToPath(import.meta.url));
const REPO_ROOT = path.resolve(__dirname, '..', '..', '..');
const cliSource = readFileSync(
path.join(REPO_ROOT, 'gitnexus', 'src', 'config', 'ignore-service.ts'),
'utf8',
);
const webSource = readFileSync(
path.join(REPO_ROOT, 'gitnexus-web', 'src', 'lib', 'upload-filter.ts'),
'utf8',
);
const cliNames = () => setEntries(cliSource, 'const DEFAULT_IGNORE_LIST = new Set([');
const webNames = () => setEntries(webSource, 'export const EXCLUDED_DIRS = new Set([');
const cliNames = () => setEntries(readSource(IGNORE_SERVICE_PATH), 'DEFAULT_IGNORE_LIST');
const webNames = () => setEntries(readSource(UPLOAD_FILTER_PATH), 'EXCLUDED_DIRS');
/**
* Names the browser filter may drop that the analyzer does not list.