Skip to content

Commit c60698a

Browse files
qq9340100claude
andauthored
fix(scripts): print the measured specifiers in check-test-source-alias remediation (#8256) (#8486)
The unaliased-dependency diagnostic named the bare dependency and printed one anchored-BARE alias entry for `deps[0]`. That is right for a package imported bare and a dead end for one whose reachable specifiers are all subpaths: `/^@objectstack\/spec$/` matches none of the specifiers the same message had just named, so applying the printed fix leaves the gate red with the message unchanged and no further guidance. The gate already knows the specifiers — that is how it decided the package was unaliased — so they are now carried through the scan and printed, each with where it lands today (no entry matched, or an entry that lands on `dist/`), plus one anchored entry per specifier. The replacement side cannot be a template either: `@objectstack/spec` serves every namespace from a directory while `@objectstack/platform-objects` maps `./plugin` to a file, so a single capture rule is right for one and wrong for the other — and wrong on whoever next writes that import. Each target is therefore measured against the tree, and printed as unmeasured when nothing under the dependency's `src/` answers to it. A subpath importer is also warned off the object form, which passes this gate by prefix-matching and then dies with ENOTDIR at run time. Output only: no input changes which packages the gate accepts or rejects. `--list` is byte-for-byte identical before and after (61 entries), and the full-repo run is unchanged. Claude-Session: https://claude.ai/code/session_01Jqe56GnYFddggeAyfkZFVz Co-authored-by: Claude <noreply@anthropic.com>
1 parent d2d6e4c commit c60698a

1 file changed

Lines changed: 225 additions & 7 deletions

File tree

scripts/check-test-source-alias.mjs

Lines changed: 225 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1096,6 +1096,39 @@ function aliasedSourceFile(spec, entries, configDir, root) {
10961096
return null;
10971097
}
10981098

1099+
/**
1100+
* The source file that would serve `spec`, spelled relative to `consumerDir` —
1101+
* i.e. ready to drop into `path.resolve(__dirname, …)` in that package's vitest
1102+
* config. Null when nothing under the dependency's `src/` answers to it.
1103+
*
1104+
* This is the half of the remediation hint that CANNOT be a template (#8256).
1105+
* The right-hand side of a subpath alias is not derivable from the specifier:
1106+
* measured in #8104, `@objectstack/spec` maps every namespace to a DIRECTORY
1107+
* (`src/api/index.ts`) while `@objectstack/platform-objects` maps `./plugin` to
1108+
* a FILE (`src/plugin.ts`). A single capture rule — the tempting "fix" — is
1109+
* therefore right for one and wrong for the other, and it fails on whoever NEXT
1110+
* writes that import rather than on the author of the rule. So the target is
1111+
* measured per specifier against the tree instead of being guessed from its
1112+
* shape: `resolveModulePath` tries the same file-then-`index.*` candidate list
1113+
* a bundler does, and returns only a path that really exists.
1114+
*
1115+
* Fail-soft on purpose: a dependency whose source is not laid out under `src/`
1116+
* mirroring its subpaths yields null, and the caller prints a placeholder plus
1117+
* the instruction to resolve it by hand. A hint that cannot be measured must
1118+
* say so, never invent a path — inventing one is the defect this card exists
1119+
* to remove, one layer down.
1120+
*/
1121+
function sourceTargetFor(spec, packageDirs, consumerDir) {
1122+
const scoped = spec.match(/^(@[^/]+\/[^/]+)(?:\/.*)?$/);
1123+
const bare = scoped ? scoped[1] : spec.split('/')[0];
1124+
const dir = packageDirs.get(bare);
1125+
if (!dir) return null;
1126+
const subpath = spec.slice(bare.length + 1);
1127+
const file = resolveModulePath(join(dir, 'src'), subpath === '' ? 'index' : subpath);
1128+
if (!file) return null;
1129+
return relative(consumerDir, file).replace(/\\/g, '/');
1130+
}
1131+
10991132
// ── the scan ────────────────────────────────────────────────────────────────
11001133

11011134
/**
@@ -1109,6 +1142,7 @@ function scan(root) {
11091142
const workspace = listWorkspacePackages(root);
11101143
const names = new Set(workspace.map((p) => p.name));
11111144
const artifactPackages = new Set(workspace.filter((p) => resolvesToArtifact(p.json)).map((p) => p.name));
1145+
const packageDirs = new Map(workspace.map((p) => [p.name, p.dir]));
11121146

11131147
const packages = [];
11141148
for (const pkg of workspace) {
@@ -1133,23 +1167,35 @@ function scan(root) {
11331167
if (!reachable) continue;
11341168

11351169
const unaliased = [];
1170+
/**
1171+
* dep -> the specifiers that made it unaliased, each with where it lands
1172+
* today and the source file that would serve it. The verdict is still the
1173+
* emptiness of this list, exactly as the `anyUnaliased` flag it replaces —
1174+
* see `remediationHint` for why the specifiers now have to survive the scan
1175+
* instead of being reduced to the dep's bare name here (#8256).
1176+
*/
1177+
const unaliasedSpecs = new Map();
11361178
const throughAFile = [];
11371179
for (const [dep, specs] of [...reachable.imports].sort(([a], [b]) => a.localeCompare(b))) {
11381180
if (!artifactPackages.has(dep)) continue; // resolves to source already; not an artifact
1139-
let anyUnaliased = false;
1181+
const offending = [];
11401182
for (const spec of [...specs].sort()) {
11411183
const resolved = resolveThroughAliases(spec, entries);
11421184
if (!resolved) {
1143-
anyUnaliased = true;
1185+
offending.push({ spec, landsOn: null, suggest: sourceTargetFor(spec, packageDirs, pkg.dir) });
11441186
continue;
11451187
}
11461188
if (THROUGH_A_FILE.test(resolved.result)) {
11471189
throughAFile.push({ spec, result: resolved.result, via: null });
11481190
continue;
11491191
}
1150-
if (!pointsAtSource(resolved.result)) anyUnaliased = true;
1192+
if (!pointsAtSource(resolved.result))
1193+
offending.push({ spec, landsOn: resolved.result, suggest: sourceTargetFor(spec, packageDirs, pkg.dir) });
1194+
}
1195+
if (offending.length > 0) {
1196+
unaliased.push(dep);
1197+
unaliasedSpecs.set(dep, offending);
11511198
}
1152-
if (anyUnaliased) unaliased.push(dep);
11531199
}
11541200

11551201
// Rule 5 over the specifiers that reached this config's resolution domain
@@ -1170,6 +1216,7 @@ function scan(root) {
11701216
configPath: configPath ? relative(root, configPath) : null,
11711217
unreadable,
11721218
unaliased,
1219+
unaliasedSpecs,
11731220
throughAFile,
11741221
});
11751222
}
@@ -1184,6 +1231,90 @@ function escapeForRegexLiteral(spec) {
11841231
return spec.replace(/[/\\^$*+?.()|[\]{}]/g, (c) => '\\' + c);
11851232
}
11861233

1234+
/** The bare package name a specifier belongs to. */
1235+
function barePackageOf(spec) {
1236+
const scoped = spec.match(/^(@[^/]+\/[^/]+)(?:\/.*)?$/);
1237+
return scoped ? scoped[1] : spec.split('/')[0];
1238+
}
1239+
1240+
/** Does this specifier reach a subpath export rather than the package entry? */
1241+
function isSubpathSpecifier(spec) {
1242+
return spec.length > barePackageOf(spec).length;
1243+
}
1244+
1245+
/** Printed where a replacement could not be measured — never a fabricated path. */
1246+
const UNMEASURED_TARGET = '<relative>/src/…';
1247+
1248+
/**
1249+
* The remediation block for a set of unaliased dependencies: the specifiers the
1250+
* gate ACTUALLY measured, and one anchored entry per specifier.
1251+
*
1252+
* ⛔ Deliberately not a template (#8256). What stood here printed the dep's
1253+
* bare NAME and one anchored-bare entry for `deps[0]` — correct only for a
1254+
* package imported bare. For an importer whose reachable specifiers are all
1255+
* subpaths (`@objectstack/spec/api`, `/data`, `/system`), `/^@objectstack\/spec$/`
1256+
* matches NONE of the specifiers the same message had just named: the reader
1257+
* applies the printed fix, the gate stays red, and the message repeats itself
1258+
* with no further guidance. Worse, the obvious next guess is the object form,
1259+
* which makes this gate pass while matching by PREFIX and dying with ENOTDIR at
1260+
* run time (#7778) — a wrong turn this block now warns against by name, because
1261+
* the case where it is tempting is exactly the case detected here.
1262+
*
1263+
* The one thing this must NOT do is answer with a different template: a single
1264+
* capture rule is safe for a package with a uniform export map and wrong for
1265+
* one that maps a subpath to a file, and it would fail on the next author
1266+
* rather than on its own. Everything printed here is measured — the specifiers
1267+
* from the walk, the targets from the tree — or explicitly marked unmeasured.
1268+
*/
1269+
function remediationHint(pkg, deps) {
1270+
const rows = deps.flatMap((dep) => pkg.unaliasedSpecs.get(dep) ?? []);
1271+
if (rows.length === 0) return '';
1272+
const width = Math.max(...rows.map((r) => r.spec.length));
1273+
const subpath = rows.find((r) => isSubpathSpecifier(r.spec));
1274+
1275+
const lines = [
1276+
' Measured — the specifiers these tests really import, and where each one lands today:',
1277+
...rows.map(
1278+
(r) =>
1279+
` ${r.spec.padEnd(width)} ` +
1280+
(r.landsOn ? `aliased, but lands on \`${r.landsOn}\` — an artifact` : 'no alias entry matches it'),
1281+
),
1282+
" Add ONE ANCHORED entry per specifier above to this package's vitest.config.* (array form).",
1283+
' Anchoring is what makes the entries order-independent and stops a bare key from swallowing',
1284+
' the subpaths:',
1285+
' alias: [',
1286+
...rows.map(
1287+
(r) =>
1288+
` { find: /^${escapeForRegexLiteral(r.spec)}$/, ` +
1289+
`replacement: path.resolve(__dirname, '${r.suggest ?? UNMEASURED_TARGET}') },`,
1290+
),
1291+
' ]',
1292+
];
1293+
1294+
if (rows.some((r) => r.suggest))
1295+
lines.push(
1296+
' Each replacement above names a file that EXISTS in this checkout' +
1297+
(subpath
1298+
? "; confirm it is what that\n package's `exports` entry for the subpath is built from — this gate measures the tree, it\n does not read the export map."
1299+
: '.'),
1300+
);
1301+
if (rows.some((r) => !r.suggest))
1302+
lines.push(
1303+
` \`${UNMEASURED_TARGET}\` marks a specifier with no counterpart under that dependency's \`src/\`:`,
1304+
' resolve that one against the package\'s own `exports` map by hand. This gate prints no path',
1305+
' that it could not measure.',
1306+
);
1307+
if (subpath)
1308+
lines.push(
1309+
` ⛔ Do NOT collapse the subpath entries into the object form \`{ '${barePackageOf(subpath.spec)}': … }\`.`,
1310+
` It matches by PREFIX, so \`${subpath.spec}\` resolves to \`…/src/index.ts/${subpath.spec.slice(barePackageOf(subpath.spec).length + 1)}\` —`,
1311+
' ENOTDIR at run time, in a config that reads as correct. This gate fails that as the',
1312+
' alias-through-a-file rule, and it is the trap this hint exists to keep you out of.',
1313+
);
1314+
1315+
return lines.join('\n');
1316+
}
1317+
11871318
function check(root, registry) {
11881319
const failures = [];
11891320
const { packages, artifactPackages, totalPackages } = scan(root);
@@ -1228,8 +1359,8 @@ function check(root, registry) {
12281359
` ${deps.join(', ')}\n` +
12291360
' Every verdict in this package is currently a function of build state, not of the source in the\n' +
12301361
' checkout — and the dangerous case is SILENT (a dist merely behind the source runs GREEN against\n' +
1231-
' old behaviour). Add the aliases to its vitest.config.ts, anchored-regex/array form:\n' +
1232-
` alias: [{ find: /^${escapeForRegexLiteral(deps[0])}$/, replacement: path.resolve(__dirname, '<relative>/src/index.ts') }]`,
1362+
' old behaviour).\n' +
1363+
remediationHint(pkg, deps),
12331364
);
12341365
continue;
12351366
}
@@ -1238,7 +1369,13 @@ function check(root, registry) {
12381369
if (added.length > 0)
12391370
failures.push(
12401371
`${name}: NEW unaliased artifact import(s) since this entry was measured: ${added.join(', ')}.\n` +
1241-
' Alias them in the package\'s vitest.config.* — widening the registry entry is not the fix.',
1372+
" Alias them in the package's vitest.config.* — widening the registry entry is not the fix.\n" +
1373+
// Same defect, same fix: this branch also named bare packages and left
1374+
// the reader to guess the specifier shape (#8256).
1375+
remediationHint(
1376+
packages.find((p) => p.name === name),
1377+
added,
1378+
),
12421379
);
12431380
if (gone.length > 0)
12441381
failures.push(
@@ -1297,10 +1434,15 @@ function buildFixtureTree() {
12971434
mkdirSync(join(root, 'packages'), { recursive: true });
12981435

12991436
// The stale-able dependency every fixture imports.
1437+
// `logger` is a subpath served by a FILE and `nested` one served by a
1438+
// DIRECTORY — the non-uniformity that decides whether a remediation hint can
1439+
// be a template at all (#8256; measured on the real `@objectstack/spec` vs
1440+
// `@objectstack/platform-objects` in #8104).
13001441
fixture(root, 'packages/core', {
13011442
'package.json': ARTIFACT_MANIFEST('@fx/core'),
13021443
'src/index.ts': 'export const alive = 1;\n',
13031444
'src/logger.ts': 'export const log = 1;\n',
1445+
'src/nested/index.ts': 'export const nested = 1;\n',
13041446
});
13051447

13061448
// (1) violating: tests import the artifact, no config at all.
@@ -1310,6 +1452,23 @@ function buildFixtureTree() {
13101452
'src/thing.test.ts': "import { thing } from './thing';\nexport default thing;\n",
13111453
});
13121454

1455+
// (1b) THE SUBPATH-ONLY IMPORTER (#8256) — the shape the old hint could not
1456+
// serve. Not one of its specifiers is the bare package name, so the anchored
1457+
// BARE entry the diagnostic used to print (`/^@fx\/core$/`) matches NONE of
1458+
// them: the reader applied the printed fix and the gate stayed red, with the
1459+
// same message and no further guidance. All three subpaths are here because
1460+
// their remediations differ and no single rule covers them: `logger` is a
1461+
// file, `nested` is a directory, and `ghost` has no counterpart under `src/`
1462+
// at all — which must print as unmeasured rather than as an invented path.
1463+
fixture(root, 'packages/subpath-only', {
1464+
'package.json': ARTIFACT_MANIFEST('@fx/subpath-only'),
1465+
'src/thing.test.ts':
1466+
"import { log } from '@fx/core/logger';\n" +
1467+
"import { nested } from '@fx/core/nested';\n" +
1468+
"import { ghost } from '@fx/core/ghost';\n" +
1469+
'export default log + nested + ghost;\n',
1470+
});
1471+
13131472
// (2) compliant: anchored array-form alias to source.
13141473
fixture(root, 'packages/compliant', {
13151474
'package.json': ARTIFACT_MANIFEST('@fx/compliant'),
@@ -1543,6 +1702,65 @@ function selfTest() {
15431702
expect(has(bare.failures, 'ENOTDIR'), 'the prefix/ENOTDIR alias trap was not detected');
15441703
expect(has(bare.failures, 'cannot be read statically'), 'a config with spread aliases was read as aliasing nothing');
15451704

1705+
// ── the remediation hint is MEASURED, not a template (#8256) ──────────
1706+
//
1707+
// The old text named the bare dependency and printed one anchored-BARE
1708+
// entry for it. Following that verbatim fixes nothing for an importer that
1709+
// only ever writes subpaths, and the message then repeats unchanged. Each
1710+
// assertion below pins one fact the hint must carry from the measurement
1711+
// rather than from a shape guess.
1712+
const subpathOnly = bare.failures.find((f) => f.startsWith('packages/subpath-only')) ?? '';
1713+
expect(
1714+
subpathOnly.includes('@fx/core/logger') &&
1715+
subpathOnly.includes('@fx/core/nested') &&
1716+
subpathOnly.includes('@fx/core/ghost'),
1717+
'the hint did not print the specifiers the gate measured — it named the bare dependency only',
1718+
);
1719+
expect(
1720+
subpathOnly.includes('find: /^@fx\\/core\\/nested$/'),
1721+
'the hint emitted no anchored entry for a measured subpath specifier (the old `deps[0]`-only template)',
1722+
);
1723+
// The counterexample that rules a one-size capture rule out: same package,
1724+
// same shape of specifier, two different targets. A rule deriving the path
1725+
// from the specifier gets exactly one of these two right.
1726+
expect(
1727+
subpathOnly.includes("'../core/src/logger.ts'"),
1728+
'a subpath served by a FILE was not measured to that file — a capture rule would say `logger/index.ts`',
1729+
);
1730+
expect(
1731+
subpathOnly.includes("'../core/src/nested/index.ts'"),
1732+
'a subpath served by a DIRECTORY was not measured through its index — the same rule cannot do both',
1733+
);
1734+
// Fail-soft: unmeasurable must print as unmeasurable.
1735+
expect(
1736+
subpathOnly.includes(UNMEASURED_TARGET) && !subpathOnly.includes('src/ghost'),
1737+
'a subpath with no counterpart under `src/` was given an invented replacement path',
1738+
);
1739+
// The wrong turn this card exists to stop (#7778): the object form passes
1740+
// this gate by prefix-matching and dies with ENOTDIR at run time.
1741+
expect(
1742+
subpathOnly.includes('matches by PREFIX') && subpathOnly.includes('ENOTDIR'),
1743+
'a subpath-only importer was not warned off the object form, the next guess that survives review',
1744+
);
1745+
// …and the warning is scoped to the case where it applies. A package
1746+
// imported only bare cannot hit prefix-matching, and telling it about the
1747+
// trap anyway is how a diagnostic becomes noise nobody reads.
1748+
const bareOnly = bare.failures.find((f) => f.startsWith('packages/violator')) ?? '';
1749+
expect(
1750+
bareOnly.includes("find: /^@fx\\/core$/") && bareOnly.includes("'../core/src/index.ts'"),
1751+
'the bare importer lost the anchored-bare entry, which was right for it all along',
1752+
);
1753+
expect(
1754+
!bareOnly.includes('matches by PREFIX'),
1755+
'the object-form warning was printed for an importer with no subpath specifier',
1756+
);
1757+
// The two reasons a specifier lands in the ledger are different repairs:
1758+
// no entry matched it at all, versus an entry matched and points at `dist/`.
1759+
expect(
1760+
(bare.failures.find((f) => f.startsWith('packages/template-to-dist')) ?? '').includes('lands on'),
1761+
'a specifier whose alias lands on `dist/` was reported as having no alias entry at all',
1762+
);
1763+
15461764
// ── the canary (#8020) ────────────────────────────────────────────────
15471765
// Escaped-slash regex `find` AND template-literal `replacement`, together,
15481766
// exactly as the two real configs write them. Both spellings have already

0 commit comments

Comments
 (0)