Skip to content

Commit 5cfbd2a

Browse files
hotlongclaude
andauthored
fix(scripts): read a template-literal alias replacement, and shrink the two entries it mis-measured (#8020) (#8107)
`asPath` took the last string literal in a replacement expression. For `path.resolve(__dirname, '../x/src/index.ts')` that is the whole answer. For the template form — the one the subpath rule needs, because the `$1` back-reference has to sit INSIDE the path — replacement: `${path.resolve(__dirname, '../../spec/src')}/$1/index.ts` the only delimiter reached is the backtick, so the entire template body came back as the path. That text has no `src` SEGMENT in it (`spec/src'` is followed by a quote, not a separator), so a config aliasing every namespace correctly read as aliasing nothing. Fail-closed, so never a false green — but `plugin-audit` and `service-knowledge` sat in the shrink-only registry as still resolving `@objectstack/spec` through `dist/` on the strength of how their replacement was SPELLED, and a dev dispatched to remediate either would have found the alias already correct, the gate still red, and the failure text prescribing the alias they already had. `asPath` now splits a template into its literal chunks and `${…}` holes, resolves each hole by the same last-literal rule, and concatenates — so `$1` survives into `String.replace` and joins the `src` chunk. Both packages' entries then shrink to exactly `['@objectstack/objectql']`; measured delta is those two dependencies and nothing else (312 -> 310 package-dependency pairs, 63 entries unchanged). Neither `vitest.config.ts` is touched: the configs are correct, the reader was what could not see them. Evaluating the config instead was measured and rejected: this gate runs dependency-free on a bare checkout in ~3s, and its own fixture tree lives in `tmpdir` with no `node_modules`, while every real config here opens with `import { defineConfig } from 'vitest/config'`. Self-test: `plugin-audit` and `service-knowledge` have now defeated three readers of vitest aliases by two different parsing assumptions — an escaped-slash regex `find` (`@fx\/core`, which hides the plain specifier from a grep census) and a template-literal `replacement`. Both spellings are pinned together in one canary fixture, plus a template landing on `dist/` (still unaliased), a template resolving through a file (still ENOTDIR), and a `${…}` hole with no literal in it (unreadable, never "aliases nothing"). The remaining unreadable-but-legal spellings — `+` concatenation, a non-literal path argument, a nested template — are enumerated in the `asPath` header so the fourth reader looks them up instead of rediscovering them. Co-authored-by: Claude <noreply@anthropic.com>
1 parent 801d952 commit 5cfbd2a

1 file changed

Lines changed: 225 additions & 8 deletions

File tree

scripts/check-test-source-alias.mjs

Lines changed: 225 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,11 @@
5858
// 3. Read the package's `vitest.config.*` and resolve each of those
5959
// specifiers the way Vite does — entries in order, FIRST MATCH WINS,
6060
// string `find` matching by PREFIX and regex `find` by `String.replace`.
61-
// A dep whose winning entry lands under `src/` is safe.
61+
// A dep whose winning entry lands under `src/` is safe. The replacement
62+
// is read statically, in the two spellings these configs use — a call
63+
// whose last argument is the path, and a template literal carrying a
64+
// `$1` back-reference inside it; `asPath` below states what that can and
65+
// cannot read, and why it does not evaluate the config.
6266
// 4. Anything left is an unaliased artifact import, and the package must be
6367
// registered in `KNOWN_UNALIASED_TEST_IMPORTS` below with EXACTLY that
6468
// set. Unregistered ⇒ red.
@@ -225,7 +229,7 @@ const KNOWN_UNALIASED_TEST_IMPORTS = {
225229
'@objectstack/platform-objects', '@objectstack/service-automation', '@objectstack/spec',
226230
'@objectstack/trigger-record-change', '@objectstack/types',
227231
],
228-
'@objectstack/plugin-audit': ['@objectstack/objectql', '@objectstack/spec'],
232+
'@objectstack/plugin-audit': ['@objectstack/objectql'],
229233
'@objectstack/plugin-auth': [
230234
'@objectstack/core', '@objectstack/driver-sql', '@objectstack/objectql',
231235
'@objectstack/platform-objects', '@objectstack/rest', '@objectstack/spec', '@objectstack/types',
@@ -291,7 +295,7 @@ const KNOWN_UNALIASED_TEST_IMPORTS = {
291295
],
292296
'@objectstack/service-i18n': ['@objectstack/core', '@objectstack/spec', '@objectstack/types'],
293297
'@objectstack/service-job': ['@objectstack/metadata-core', '@objectstack/platform-objects'],
294-
'@objectstack/service-knowledge': ['@objectstack/objectql', '@objectstack/spec'],
298+
'@objectstack/service-knowledge': ['@objectstack/objectql'],
295299
'@objectstack/service-messaging': [
296300
'@objectstack/driver-sql', '@objectstack/metadata-core', '@objectstack/objectql',
297301
'@objectstack/platform-objects', '@objectstack/spec', '@objectstack/types',
@@ -789,16 +793,118 @@ function asFind(raw) {
789793
}
790794

791795
/**
792-
* The path an alias replacement produces. `path.resolve(__dirname, '../x/src')`
793-
* carries its answer in the last string literal; a bare string literal is
794-
* itself the answer.
796+
* The last string literal in an expression — the answer for the call forms
797+
* these configs use, where the path is the final argument:
798+
* `path.resolve(__dirname, '../x/src')`, `path.join(dir, 'x/src/$1.ts')`. A
799+
* bare string literal is itself the answer.
800+
*
801+
* `context` is what the diagnostic quotes, so a failure inside a `${…}` hole
802+
* names the whole replacement rather than the fragment.
795803
*/
796-
function asPath(raw) {
804+
function lastStringLiteral(raw, context) {
797805
const strings = [...raw.matchAll(/(['"`])((?:[^\\]|\\.)*?)\1/g)].map((m) => m[2]);
798-
if (strings.length === 0) throw new UnreadableConfig(`alias replacement has no literal path: ${raw.slice(0, 60)}`);
806+
if (strings.length === 0)
807+
throw new UnreadableConfig(`alias replacement has no literal path: ${context.slice(0, 60)}`);
799808
return strings[strings.length - 1];
800809
}
801810

811+
/**
812+
* A template literal's pieces in order — literal chunks and `${…}` holes.
813+
* Escapes are consumed as they are at run time, so `\${x}` is a literal
814+
* `${x}` and not a hole.
815+
*/
816+
function splitTemplateLiteral(raw) {
817+
const body = raw.slice(1, -1);
818+
const parts = [];
819+
let literal = '';
820+
let i = 0;
821+
while (i < body.length) {
822+
const c = body[i];
823+
if (c === '\\') {
824+
literal += body[i + 1] ?? '';
825+
i += 2;
826+
continue;
827+
}
828+
if (c === '$' && body[i + 1] === '{') {
829+
const hole = balancedRegion(body, i + 1);
830+
if (hole == null) throw new UnreadableConfig(`unbalanced \`\${…}\` in alias replacement: ${raw.slice(0, 60)}`);
831+
parts.push({ literal });
832+
parts.push({ expression: hole.slice(1, -1).trim() });
833+
literal = '';
834+
i += hole.length + 1;
835+
continue;
836+
}
837+
literal += c;
838+
i++;
839+
}
840+
parts.push({ literal });
841+
return parts;
842+
}
843+
844+
/**
845+
* The path an alias replacement produces, in the two spellings these configs
846+
* use. Both are approximations of the same shape: what is returned stands in
847+
* for the resolved path, and the only questions asked of it downstream are
848+
* "does it have a `src` segment" and "does it continue past a file extension",
849+
* so a relative fragment answers exactly as well as the absolute path
850+
* `path.resolve` would really produce.
851+
*
852+
* replacement: path.resolve(__dirname, '../x/src/index.ts')
853+
* → `../x/src/index.ts` — the last string literal is the whole answer.
854+
*
855+
* replacement: `${path.resolve(__dirname, '../x/src')}/$1/index.ts`
856+
* → `../x/src/$1/index.ts` — each `${…}` hole resolves by the same rule and
857+
* the literal chunks are concatenated around it, so the `$1` back-
858+
* reference survives into `String.replace` and joins the `src` chunk.
859+
*
860+
* ⚠️ The template form is not a stylistic variant: it is how the one-rule-for-
861+
* all-namespaces subpath alias is written (`/^@objectstack\/spec\/([a-z-]+)$/`
862+
* → `…/spec/src/$1/index.ts`), because that rule needs the capture group
863+
* INSIDE the path. Reading only the last string literal returned the entire
864+
* template body — a text with no `src` SEGMENT in it (`spec/src'` is followed
865+
* by a quote, not a separator) — so a config aliasing every namespace
866+
* correctly read as aliasing nothing, and `plugin-audit` and
867+
* `service-knowledge` sat in the registry as unaliased on the strength of how
868+
* their replacement was SPELLED (#8020). Fail-closed, so never a false green —
869+
* but it over-stated the remediation list and told two compliant packages to
870+
* add the alias they already had.
871+
*
872+
* ── Spellings that are legal here and still unreadable ──────────────────────
873+
*
874+
* Each of these is fail-closed — either a loud `UnreadableConfig` naming the
875+
* config, or the same over-statement above — never a silent pass. Listed so
876+
* the next one is looked up rather than rediscovered:
877+
*
878+
* - `+` concatenation: `path.resolve(__dirname, '../x/src') + '/$1/index.ts'`
879+
* resolves to the LAST literal, `/$1/index.ts`, which has no `src` segment
880+
* — the exact #8020 shape one spelling over. Write it as a template.
881+
* - A hole or argument with no string literal in it at all (`${SPEC_SRC}`,
882+
* `path.resolve(SRC_ROOT, 'index.ts')`) — the first throws
883+
* `UnreadableConfig`; the second silently answers `index.ts`.
884+
* - A path that is not the last argument, where later arguments are
885+
* non-literal (`path.join('x/src', suffix)` reads `x/src` and drops
886+
* `suffix`).
887+
* - A template literal nested inside a `${…}` hole — the scanner treats the
888+
* first inner backtick as the outer literal's terminator.
889+
*
890+
* Evaluating the config instead of reading it was measured and rejected: this
891+
* gate runs on a bare checkout with NO `node_modules` (that is how CI reaches
892+
* it, and how its own `--self-test` fixture tree in `tmpdir` works), while
893+
* every real config here opens with `import { defineConfig } from
894+
* 'vitest/config'`. Evaluation would trade a dependency-free ~3s scan for one
895+
* that cannot run before `pnpm install`, and would make an alias list the gate
896+
* cannot see today into one it executes.
897+
*/
898+
function asPath(raw) {
899+
const literal = raw.trim();
900+
if (literal[0] === '`' && skipString(literal, 0) === literal.length - 1) {
901+
return splitTemplateLiteral(literal)
902+
.map((part) => (part.expression == null ? part.literal : lastStringLiteral(part.expression, literal)))
903+
.join('');
904+
}
905+
return lastStringLiteral(literal, literal);
906+
}
907+
802908
/**
803909
* Resolve `spec` through `entries` exactly as Vite does: entries in order,
804910
* first match wins, string `find` replacing a PREFIX and regex `find` going
@@ -1051,6 +1157,78 @@ function buildFixtureTree() {
10511157
'src/thing.test.ts': "export default 1;\n",
10521158
});
10531159

1160+
// ── (7) THE CANARY ────────────────────────────────────────────────────────
1161+
//
1162+
// A byte-for-byte copy of the shape `plugin-audit` and `service-knowledge`
1163+
// really use. Those two packages have now defeated THREE readers of vitest
1164+
// aliases, twice on the same morning, by two different parsing assumptions:
1165+
//
1166+
// - an `objectstack/core` grep census missed them because the anchored
1167+
// regex form writes the bytes `@fx\/core` — with an ESCAPED SLASH, so
1168+
// the plain specifier never appears in the file;
1169+
// - this gate missed them because the one-rule-for-all-namespaces subpath
1170+
// alias writes its replacement as a TEMPLATE LITERAL, to get the `$1`
1171+
// back-reference inside the path (#8020).
1172+
//
1173+
// Neither spelling is exotic and neither is going away — the escaped slash
1174+
// is forced by the regex literal, the template by the capture group. They
1175+
// are pinned together, in one fixture, so the fourth reader of these configs
1176+
// inherits the two assumptions that have already cost this repo twice
1177+
// instead of rediscovering them. This fixture must stay COMPLIANT: it
1178+
// aliases everything it imports, and any reader that reports it is wrong
1179+
// about the reader, not about the config.
1180+
fixture(root, 'packages/canary', {
1181+
'package.json': ARTIFACT_MANIFEST('@fx/canary'),
1182+
'src/thing.test.ts':
1183+
"import { alive } from '@fx/core';\nimport { log } from '@fx/core/logger';\nexport default alive + log;\n",
1184+
'vitest.config.ts':
1185+
"import path from 'path';\nexport default { resolve: { alias: [\n" +
1186+
// Subpath rule first: one anchored regex for every namespace, with the
1187+
// capture group INSIDE the path — which is what forces the template.
1188+
' {\n' +
1189+
' find: /^@fx\\/core\\/([a-z-]+)$/,\n' +
1190+
" replacement: `${path.resolve(__dirname, '../core/src')}/$1.ts`,\n" +
1191+
' },\n' +
1192+
" { find: /^@fx\\/core$/, replacement: path.resolve(__dirname, '../core/src/index.ts') },\n" +
1193+
'] } };\n',
1194+
});
1195+
1196+
// (8) the template form is a spelling, not a licence: one landing on `dist/`
1197+
// is still an unaliased artifact import.
1198+
fixture(root, 'packages/template-to-dist', {
1199+
'package.json': ARTIFACT_MANIFEST('@fx/template-to-dist'),
1200+
'src/thing.test.ts': "import { log } from '@fx/core/logger';\nexport default log;\n",
1201+
'vitest.config.ts':
1202+
"import path from 'path';\nexport default { resolve: { alias: [\n" +
1203+
' {\n' +
1204+
' find: /^@fx\\/core\\/([a-z-]+)$/,\n' +
1205+
" replacement: `${path.resolve(__dirname, '../core/dist')}/$1.js`,\n" +
1206+
' },\n' +
1207+
'] } };\n',
1208+
});
1209+
1210+
// (9) …and the ENOTDIR trap is still seen THROUGH a template: the chunks
1211+
// really are concatenated, rather than the whole thing waved past.
1212+
fixture(root, 'packages/template-prefix-trap', {
1213+
'package.json': ARTIFACT_MANIFEST('@fx/template-prefix-trap'),
1214+
'src/thing.test.ts': "import { log } from '@fx/core/logger';\nexport default log;\n",
1215+
'vitest.config.ts':
1216+
"import path from 'path';\nexport default { resolve: { alias: {\n" +
1217+
" '@fx/core': `${path.resolve(__dirname, '../core/src')}/index.ts`,\n" +
1218+
'} } };\n',
1219+
});
1220+
1221+
// (10) a `${…}` hole with no literal in it is UNREADABLE, not empty — the
1222+
// fail-closed half of reading templates at all.
1223+
fixture(root, 'packages/opaque-template', {
1224+
'package.json': ARTIFACT_MANIFEST('@fx/opaque-template'),
1225+
'src/thing.test.ts': "import { alive } from '@fx/core';\nexport default alive;\n",
1226+
'vitest.config.ts':
1227+
"import { SRC } from './shared';\nexport default { resolve: { alias: [\n" +
1228+
' { find: /^@fx\\/core$/, replacement: `${SRC}/index.ts` },\n' +
1229+
'] } };\n',
1230+
});
1231+
10541232
return root;
10551233
}
10561234

@@ -1072,6 +1250,45 @@ function selfTest() {
10721250
expect(has(bare.failures, 'ENOTDIR'), 'the prefix/ENOTDIR alias trap was not detected');
10731251
expect(has(bare.failures, 'cannot be read statically'), 'a config with spread aliases was read as aliasing nothing');
10741252

1253+
// ── the canary (#8020) ────────────────────────────────────────────────
1254+
// Escaped-slash regex `find` AND template-literal `replacement`, together,
1255+
// exactly as the two real configs write them. Both spellings have already
1256+
// broken a reader of these configs; neither may break this one again.
1257+
expect(
1258+
!has(bare.failures, 'packages/canary'),
1259+
'the canary config (escaped-slash regex find + template-literal replacement) was reported as unaliased',
1260+
);
1261+
// Reading a template must not degrade into waving it past: one that lands
1262+
// on `dist/` is still unaliased, and one that resolves THROUGH a file is
1263+
// still the ENOTDIR trap — both require the chunks to be really joined.
1264+
expect(
1265+
has(bare.failures, 'packages/template-to-dist'),
1266+
'a template-literal replacement resolving to `dist/` was read as aliased to source',
1267+
);
1268+
expect(
1269+
has(bare.failures, '@fx/core/logger` to `') && has(bare.failures, 'packages/template-prefix-trap'),
1270+
'the ENOTDIR trap went unseen through a template-literal replacement',
1271+
);
1272+
// Fail-closed: a hole with nothing readable in it is UNREADABLE, never a
1273+
// config that aliases nothing.
1274+
expect(
1275+
has(bare.failures, 'packages/opaque-template'),
1276+
'a template replacement interpolating a non-literal was read as aliasing nothing',
1277+
);
1278+
expect(
1279+
bare.failures.some((f) => f.includes('packages/opaque-template') && f.includes('cannot be read statically')),
1280+
'an unreadable `${…}` hole did not fail as unreadable',
1281+
);
1282+
1283+
// Both directions on the canary: registering a package the reader can now
1284+
// see through is the stale half, and it must name itself for deletion —
1285+
// this is the shape of the two real entries that came off in #8020.
1286+
const canaryRegistered = check(root, { '@fx/violator': ['@fx/core'], '@fx/canary': ['@fx/core'] });
1287+
expect(
1288+
has(canaryRegistered.failures, '@fx/canary') && has(canaryRegistered.failures, 'no longer needed'),
1289+
'a registry entry for the now-readable canary config did not fail the both-directions audit',
1290+
);
1291+
10751292
// Registered at the measured state: the violator goes quiet, nothing else does.
10761293
const registered = check(root, { '@fx/violator': ['@fx/core'] });
10771294
expect(!has(registered.failures, 'packages/violator'), 'a correctly registered package still failed');

0 commit comments

Comments
 (0)