diff --git a/.changeset/17410-generate-reserved-word-barrel-refusal.md b/.changeset/17410-generate-reserved-word-barrel-refusal.md new file mode 100644 index 0000000000..27e865a1cc --- /dev/null +++ b/.changeset/17410-generate-reserved-word-barrel-refusal.md @@ -0,0 +1,23 @@ +--- +"@objectstack/cli": minor +--- + +feat(cli)!: `os generate` refuses a name whose barrel alias no consumer could import by name (#17410) + +`os generate view class` exited **0** and wrote `export { default as class } from './class.view';`. That line parses — an ES module export clause admits a reserved word as a `ModuleExportName` — so both landed layers admitted it, each correctly by its own terms: the #16726 charset gate because every character of `class` is a lowercase letter, and the #16541 parse check because the bytes really are parseable TypeScript. The import side is not: `import { class } from './views'` needs an `ImportedBinding`, and a reserved word is not one. So the command reported success and produced a barrel entry nothing can name, with the failure deferred into the author's own file where it reads as their mistake. + +A third layer now stands behind those two. After the identifier is derived and before anything is written or previewed, the barrel alias is put through TypeScript **in the exact position a consumer must write it**, and the command refuses when the compiler will not take it — naming the constraint, showing the line that would have been written, and writing nothing. This delivers the #16726 ruling's own closing sentence, 「`os generate view class` is therefore refused at the door rather than emitting a barrel line that binds a reserved word.」, which the charset mechanism specified in that same ruling could not. + +⛔ **No third charset** — the #16726 ruling forbids one and none is added: no character is judged. ⛔ **Nothing is rewritten.** Emitting a non-reserved alias while keeping the authored name was the other option and it loses on the reasoning that already refused option B: it decouples the name the author wrote from the name that gets emitted, silently. So this refuses, and the name you author stays the name that lands. + +**What this narrows:** 46 names — the 36 always-reserved words (`class`, `new`, `enum`, `default`, `import`, …) plus the ten reserved because a module is automatically in strict mode (`let`, `yield`, `static`, `implements`, `interface`, `package`, `private`, `protected`, `public`, and `await`, reserved at a module's top level). Every one is charset-legal and every one used to reach `exit 0` for the six generators that suffix their `const` binding (`view`, `action`, `flow`, `dashboard`, `app`, `skill`). The seventh, `object`, binds the bare identifier, so the parse check already refused **some** of them there — but only the always-reserved ones: `os g object let`, `os g object yield` and `os g object static` also exited 0, because a strict-mode reservation is a semantic diagnostic and that check is syntactic. Pick a name that survives as an import binding — `os g view order_line` works, and binds `orderLine`. + +**This is an observable change to accepted input:** those 46 names exit **0** today and will exit non-zero after this lands. Every one of them produced a barrel entry no consumer could name, so this is the fix rather than a break — but if you script `os generate`, a name in that set now stops the command instead of writing an unusable file. + +**One durability note.** The refused set is decided by the TypeScript compiler, asked in position, rather than by a list this package keeps — which is why it is right in both directions today. The consequence is that a TypeScript upgrade can move it: a word that becomes reserved starts being refused, and a word that stops being reserved starts being accepted. Both are correct, neither is a regression, and neither is predicted by a changeset. + +**What this deliberately does NOT narrow:** contextual reserved words. `type`, `as`, `from`, `async`, `get`, `set`, `of`, `keyof`, `readonly`, `satisfies`, `infer`, `declare`, `namespace`, `using`, `accessor`, `undefined`, `arguments`, `eval` and the rest are legal import bindings, they generate today, and they still generate. Refusing one of them would break a name that works — the expensive failure direction, and the one a hand-written keyword list gets wrong. There is no keyword list here for exactly that reason: a list is simultaneously too narrow (it stops at the obvious 36 and ships the defect for the other ten, which a syntactic-only check cannot even see, because the compiler reports strict-mode reservations as semantic diagnostics) and too wide (it swallows the contextual set). The judge is the compiler, asked in position. + +⛔ Neither layer in front is relaxed or reordered. `os g object class` still meets the parse check's own diagnostic in the compiler's words, a name outside the charset still meets the schema's own pattern, and the new layer is asked last, so it can only narrow what all three would otherwise have admitted. + + diff --git a/packages/cli/src/commands/generate.ts b/packages/cli/src/commands/generate.ts index dd412e6b8d..c5f73a2d42 100644 --- a/packages/cli/src/commands/generate.ts +++ b/packages/cli/src/commands/generate.ts @@ -39,6 +39,7 @@ import { import { printHeader, printSuccess, printError, printInfo, printStep, createTimer, isReportedError, CLI_ALIAS } from '../utils/format.js'; import { metadataFileName } from '../utils/metadata-file-name.js'; import { findEmissionParseFailures } from '../utils/emitted-source-parses.js'; +import { findBarrelAliasRefusal } from '../utils/importable-binding.js'; // ─── Metadata Type Templates ──────────────────────────────────────── @@ -971,6 +972,95 @@ async function runMetadataGeneration(type: string, name: string, flags: { dir?: process.exit(1); } + // ⛔ REFUSE a barrel alias no consumer can IMPORT BY NAME (#17410). + // + // The two checks above are each satisfied, correctly, by a name whose + // emitted binding is still unusable — and the comment on the parse check + // says why in its own words: a reserved word "is illegal as a `const` + // binding and legal as an `export { default as … }` alias". `class` is + // inside the charset (all lowercase letters), `const classViews:` parses, + // `export { default as class } from './class.view'` parses, and + // `import { class } from './views'` is a syntax error at the call site. + // So `os g view class` exited 0 and wrote a barrel entry that can never be + // named — the failure deferred into the author's own file, where it reads + // as their mistake. + // + // The question is the CONSUMER's, which is why it is asked here and not in + // the check above: that one asks whether the bytes we write parse, this one + // asks whether the binding those bytes publish can be imported. Both ask + // the compiler; neither states a rule of its own. ⛔ Not a third charset + // (the #16726 ruling forbids one, and none is added — no character is + // judged), and ⛔ not a sanitiser: it refuses and rewrites nothing, so the + // name the author wrote stays the name that lands. + // + // Placed LAST of the three on purpose. Each layer asks a strictly narrower + // question than the one before — legal characters, then parseable bytes, + // then an importable binding — and being last means it changes the verdict + // of neither: every name the layers in front already refuse still meets + // their diagnostic, with their wording, and `os g object class` is still + // the compiler's "not allowed as a variable declaration name" rather than + // this. It only ever narrows, and only for names all three would otherwise + // have admitted. + // + // Ahead of the dry-run branch for the same reason the parse check is: a + // preview that prints an unusable barrel and exits 0 is the same defect in + // preview form. + const barrelAlias = toCamelCase(name); + const aliasRefusal = await findBarrelAliasRefusal(barrelAlias); + if (aliasRefusal) { + printError('Refusing to generate — the barrel line this would write could not be imported'); + console.log(''); + console.log(` ${chalk.dim('Name:')} ${chalk.white(name)}`); + console.log(` ${chalk.dim('Identifier:')} ${chalk.white(barrelAlias)}`); + console.log(` ${chalk.dim('Barrel:')} ${chalk.white(exportLine)}`); + console.log(''); + console.log(` ${chalk.white(path.join(dir, 'index.ts'))}`); + for (const diagnostic of aliasRefusal) { + console.log(chalk.dim(` ${diagnostic}`)); + } + console.log(''); + console.log(chalk.dim( + ` That line parses — an export clause admits a reserved word as an alias — so`, + )); + console.log(chalk.dim( + ` it would have been written. What cannot be written is the other half: a`, + )); + console.log(chalk.dim( + ` consumer has to name it, and \`import { ${barrelAlias} } from …\` is what the`, + )); + console.log(chalk.dim( + ' compiler refused above. Nothing was written.', + )); + console.log(''); + console.log(chalk.dim( + ` \`${barrelAlias}\` is a reserved word in this position. The rule is not this`, + )); + console.log(chalk.dim( + ' command\'s and it is not a charset: it is the compiler, asked whether the', + )); + console.log(chalk.dim( + ' binding your name publishes can be imported by that name. Reserved only in', + )); + console.log(chalk.dim( + ' some contexts — `type`, `as`, `from`, `async`, `get`, `set` — are accepted,', + )); + console.log(chalk.dim( + ' because a consumer can import those.', + )); + console.log(''); + console.log(chalk.dim( + // ⛔ Deliberately NOT derived from what the author typed — a suggestion + // built from the refused name is the sanitiser this layer declines to + // be, arriving one keystroke later. Same reasoning as the #16726 gate. + ` Pick a name that survives as an import binding — \`${CLI_ALIAS} g ${type} order_line\``, + )); + console.log(chalk.dim( + ' works, and binds `orderLine`.', + )); + console.log(''); + process.exit(1); + } + if (flags.dryRun) { printInfo('Dry run — no files written'); console.log(''); diff --git a/packages/cli/src/utils/importable-binding.ts b/packages/cli/src/utils/importable-binding.ts new file mode 100644 index 0000000000..d37afc3d8d --- /dev/null +++ b/packages/cli/src/utils/importable-binding.ts @@ -0,0 +1,163 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Can a consumer IMPORT the barrel alias a scaffolder is about to write? (#17410) + * + * ## The defect this exists to end + * + * `os generate ` writes two files, and the barrel is the one with + * a consumer: `export { default as } from './'`. An ES module + * export clause admits a reserved word there — `ModuleExportName` is an + * `IdentifierName`, not a `BindingIdentifier` — so the line PARSES. The import + * side does not: `import { class } from './index'` needs an `ImportedBinding`, + * and a reserved word is not one. The command therefore exited **0** and wrote + * a barrel entry no consumer can name: + * + * os generate view class exit 0 + * src/views/class.view.ts -> const classViews: UI.View = { // parses + * src/views/index.ts -> export { default as class } … // parses + * the consumer -> import { class } from './views' // SYNTAX ERROR + * + * Every layer in front is satisfied, each correctly by its own terms: `class` + * is inside the charset `packages/spec` declares for an object `name` (#16726 — + * every character is a lowercase letter), and both emitted files parse + * (#16541). The gap is exactly a name that is charset-legal AND + * emission-parseable whose emitted binding is unusable downstream, and + * `generate.ts` names it in its own words above the parse check. + * + * ## ⛔ What this is NOT + * + * **Not a third charset.** The #16726 ruling says ⛔ no third charset and this + * adds none: no character is judged here, and a name of any shape that a + * consumer can import passes. **Not a sanitiser** — it returns the compiler's + * reasons and never a repaired alias, so the name the author wrote stays the + * name that lands (option B, refused in `nameCharsetRefusal`, would decouple + * them). **Not a relaxation of anything**: it is asked AFTER both landed + * layers and can only ever refuse more, never less. + * + * ## ⛔ Why there is no list of reserved words + * + * The obvious implementation is an array of keywords, and it is wrong in both + * directions at once — which is the whole reason this question needed + * measuring rather than recalling: + * + * - **Too narrow.** The 36 always-reserved words (`class`, `new`, `enum`, …) + * are the ones everybody writes down. But modules are automatically in + * strict mode, so `let`, `yield`, `static`, `implements`, `interface`, + * `package`, `private`, `protected`, `public` are reserved **here** too, + * and `await` is reserved at the top level of a module. All ten are + * charset-legal, all ten reached `exit 0`, and a hand-picked list that + * stops at the obvious 36 ships the same defect for them. Measured: 46 + * words, not 36. + * - **Too wide.** `type`, `as`, `from`, `async`, `get`, `set`, `keyof`, + * `satisfies`, `using`, `undefined`, `arguments`, `eval` and the rest of + * the contextual set are perfectly legal import bindings. Refusing a + * merely-contextual reserved word would break names that work today — + * the failure direction that costs an author a working command. + * + * No list gets both right and stays right: the boundary moves with the + * language, and the position matters more than the word (a reserved word is + * legal as an `export { default as … }` alias and illegal as an import + * binding). So the judge is **TypeScript's own parser, asked in the exact + * position the consumer must write** — the same instrument and the same + * reasoning as {@link findEmissionParseFailures}, one question further down. + * + * ## Why the probe is a two-file module graph + * + * A lone `import { } from './m';` is not enough, and the difference is + * measurable: the strict-mode reservations above are **grammar** checks the + * compiler reports as SEMANTIC diagnostics, so a syntactic-only verdict is + * structurally blind to all ten of them. Resolving the import makes the + * semantic bucket clean enough to read — the control returns **zero** + * diagnostics — so both buckets can be required empty and the ten are seen. + * + * Asking it as a real graph buys one more thing: the probe does not depend on + * the layers in front of it. A multi-token alias (`a, b`) or one carrying an + * escape (`a } from "./x"; const y = 1; //`) parses perfectly well as a bare + * import clause — it is simply a *different* import — and is refused here + * because `typeof ` then does not hold together. So this layer stands + * on its own, exactly as the other two do. + * + * `noLib` keeps the verdict about the grammar of these bytes rather than about + * a `lib.d.ts` a scaffold has no business needing, and the module and + * resolution modes are stated rather than defaulted so the verdict does not + * drift with a compiler upgrade. + * + * ## Why `ts` arrives through a lazy import + * + * The same call, for the same reason, as `emitted-source-parses.ts`: `ts-morph` + * is already a CLI runtime dependency and re-exports the compiler namespace, + * and the parser is a heavy load that only a command which reaches this check + * should pay for. + */ + +import type { ts as TS } from 'ts-morph'; + +const BARREL = '/barrel.ts'; +const CONSUMER = '/consumer.ts'; + +/** + * The diagnostics TypeScript reports for importing `alias` by name from a + * barrel that exports it, flattened to text. + * + * Both buckets are read: the syntactic one carries the always-reserved words + * and the malformed shapes, the semantic one carries the strict-mode and + * module-level reservations. An empty array means a consumer can write + * `import { } from './…'` and refer to the result. + * + * Exported so a pin can reach the instrument the command actually uses instead + * of a second copy of it that could drift green. + */ +export function namedImportDiagnostics(ts: typeof TS, alias: string): string[] { + const sources: Record = { + // The barrel re-exports under `alias` — legal for any `IdentifierName`, + // which is precisely why the emitted line is not where this shows up. + [BARREL]: `declare const value: unknown;\nexport { value as ${alias} };\n`, + // The consumer side: name it in an import clause, then refer to it. The + // reference is load-bearing — it is what refuses an alias that merely + // parses as some *other* import clause. + [CONSUMER]: `import { ${alias} } from './barrel';\ntype Used = typeof ${alias};\nexport type { Used };\n`, + }; + const files = new Map( + Object.entries(sources).map(([name, source]) => [ + name, + ts.createSourceFile(name, source, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS), + ]), + ); + const host: TS.CompilerHost = { + getSourceFile: (requested) => files.get(requested), + getDefaultLibFileName: () => 'lib.d.ts', + writeFile: () => {}, + getCurrentDirectory: () => '/', + getCanonicalFileName: (f) => f, + useCaseSensitiveFileNames: () => true, + getNewLine: () => '\n', + fileExists: (f) => files.has(f), + readFile: (f) => sources[f], + }; + const program = ts.createProgram([CONSUMER, BARREL], { + noLib: true, + target: ts.ScriptTarget.Latest, + module: ts.ModuleKind.ESNext, + moduleResolution: ts.ModuleResolutionKind.Bundler, + }, host); + const consumer = files.get(CONSUMER)!; + return [ + ...program.getSyntacticDiagnostics(consumer), + ...program.getSemanticDiagnostics(consumer), + ].map((d) => ts.flattenDiagnosticMessageText(d.messageText, ' ')); +} + +/** + * The compiler's reasons a consumer could not import `alias` by name, or + * `null` when it can. + * + * `null` is the accept verdict and is the answer for every name that already + * produced importable output — ⛔ never that the check was skipped: `alias` is + * the same string the command interpolates into the barrel it writes. + */ +export async function findBarrelAliasRefusal(alias: string): Promise { + const { ts } = await import('ts-morph'); + const diagnostics = namedImportDiagnostics(ts, alias); + return diagnostics.length > 0 ? diagnostics : null; +} diff --git a/packages/cli/test/generate-refuses-name-outside-charset.test.ts b/packages/cli/test/generate-refuses-name-outside-charset.test.ts index a730226586..033b04ae8c 100644 --- a/packages/cli/test/generate-refuses-name-outside-charset.test.ts +++ b/packages/cli/test/generate-refuses-name-outside-charset.test.ts @@ -262,6 +262,19 @@ describe('[#16726] the gate and #16541`s parse check are DISTINCT layers', () => // rule, and the ruling's other half is ⛔ no third charset. Reported on // #16726 for the maintainer; this assertion exists so that whichever way // that is answered, the answer is a deliberate edit here. + // + // ⭐ ANSWERED (#17410) — and this is that deliberate edit. A third layer + // now refuses `os g view class`, so the ruling's sentence holds. The two + // assertions below are UNCHANGED and still true, because the third layer + // added no fourth verdict on these emissions: both still parse, and + // `class` is still inside the charset. It asks the one question neither of + // these does — whether a CONSUMER can import the barrel alias by name — so + // this row keeps measuring exactly what it always measured, which is why + // nothing here had to be weakened to make room for it. ⛔ Still no third + // charset: no character is judged. The command-level verdict is pinned in + // `generate-refuses-unimportable-alias.test.ts`, which also holds the + // three layers apart so neither of the two asserted here loses its own + // wording. expect(specVerdict('class').accepted).toBe(true); return findEmissionParseFailures(emissionsFor('view', 'class')).then((failures) => { expect(failures).toEqual([]); diff --git a/packages/cli/test/generate-refuses-unimportable-alias.test.ts b/packages/cli/test/generate-refuses-unimportable-alias.test.ts new file mode 100644 index 0000000000..ee9a57e03e --- /dev/null +++ b/packages/cli/test/generate-refuses-unimportable-alias.test.ts @@ -0,0 +1,267 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * PIN (#17410) — `os generate` must not EXIT 0 on a name whose barrel alias no + * consumer can import by name, and must write nothing when it refuses. + * + * ## Why a child process, and why this file exists next to the unit pin + * + * `importable-binding.test.ts` measures the check: given an alias, is it + * importable. It cannot measure the half this card is actually about — that the + * COMMAND consults it, before the writes, on every branch. The reported defect + * is an exit code (`os generate view class` -> exit 0 with a barrel entry + * nothing can name), and `process.exitCode` set inside a vitest worker is not + * an exit status: a CI script judges this command by `$?`. So the assertions + * here are on a real child process and on stdout, for the same two reasons + * `invocation-loudness.e2e.test.ts` documents at length — these commands print + * through `utils/format.ts`, whose `printError` writes to stdout. Spawned + * through `bin/run-dev.js` + tsx so the suite does not depend on + * `packages/cli/dist` having been built. + * + * ## ⛔ Why this file is NOT named `.e2e` + * + * Identical to its sibling `generate-refuses-unparseable-name.test.ts`: the + * BEHAVIOUR predicate in `vitest-tiers.ts` puts a spawning file in the + * `integration` project, while the NAME decides which RUN collects it — + * `*.e2e.test.ts` is nightly, everything else is the queue's. The defect + * pinned here is a command that reports success while writing an unusable + * barrel, so it is pinned in the run that gates the merge queue rather than in + * the one that reports the next morning. + * + * ## The three layers, and why this one had to be third + * + * `class` passes the #16726 charset gate (every character is a lowercase + * letter) and passes the #16541 parse check for every generator but `object` + * (`const classViews:` parses, and `export { default as class } from …` parses + * — an export clause admits a reserved word as a `ModuleExportName`). Both + * landed layers are therefore satisfied, correctly, by a name whose emitted + * binding is unusable: `import { class } from './views'` is a syntax error. + * + * So the assertions below are written to hold each layer to its OWN verdict. + * `os g object class` must still meet the parse check's wording, `order-line` + * must still meet the charset gate's, and only a name all three would + * otherwise have admitted may meet this one. ⛔ A future edit that lets this + * layer answer first reddens here, and it would be a regression: it would take + * the compiler's specific reason away from the author of `os g object class`. + * + * ## The controls are load-bearing + * + * A refusal that fired on everything would satisfy every refusal assertion in + * this file and would be a worse command than the broken one. Two controls run + * the whole path: `order_line` (an ordinary name) and `type` (⭐ contextual + * only — keyword-shaped and a perfectly legal import binding, so refusing it + * would break a name that works today). Both write their scaffold and their + * barrel, and both files are re-read from disk here, so "still works" is a + * reading rather than an exit code. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { childEnv } from './helpers/serve-process.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); + +/** oclif + tsx cold start, with every command module loaded; ~2-10 s when healthy. */ +const RUN_TIMEOUT_MS = 240_000; + +interface Run { + code: number; + stdout: string; + stderr: string; +} + +function runTsx(args: string[], cwd: string): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + args, + { cwd, maxBuffer: 8 * 1024 * 1024, env: childEnv({ NO_COLOR: '1' }) }, + (err, stdout, stderr) => { + resolvePromise({ + // `err.code` is the real exit status; null/undefined means the child + // was signalled — a different failure, never reported as 0. + code: err + ? typeof (err as { code?: unknown }).code === 'number' + ? (err as unknown as { code: number }).code + : 1 + : 0, + stdout: String(stdout), + stderr: String(stderr), + }); + }, + ); + }); +} + +let reservedDir: string; +let strictDir: string; +let dryRunDir: string; +let parseLayerDir: string; +let charsetLayerDir: string; +let controlDir: string; +let contextualDir: string; + +let reserved: Run; +let strictReserved: Run; +let dryRun: Run; +let parseLayer: Run; +let charsetLayer: Run; +let control: Run; +let contextual: Run; + +beforeAll(async () => { + reservedDir = mkdtempSync(join(tmpdir(), 'os-g-reserved-')); + strictDir = mkdtempSync(join(tmpdir(), 'os-g-strict-')); + dryRunDir = mkdtempSync(join(tmpdir(), 'os-g-alias-dryrun-')); + parseLayerDir = mkdtempSync(join(tmpdir(), 'os-g-parselayer-')); + charsetLayerDir = mkdtempSync(join(tmpdir(), 'os-g-charsetlayer-')); + controlDir = mkdtempSync(join(tmpdir(), 'os-g-alias-control-')); + contextualDir = mkdtempSync(join(tmpdir(), 'os-g-contextual-')); + + // Sequential on purpose: cold tsx starts, each loading every command module, + // in a container several agents share. + // + // The card's measured row. `view` because that generator suffixes its `const` + // binding (`classViews`), so the scaffold parses and the barrel alias is the + // only thing left that cannot be named. + reserved = await runTsx([CLI, 'generate', 'view', 'class'], reservedDir); + // A second generator, to show the refusal is not one patched call site — + // and `let`, which is reserved only because a module is in strict mode. + strictReserved = await runTsx([CLI, 'generate', 'flow', 'let'], strictDir); + dryRun = await runTsx([CLI, 'generate', 'view', 'class', '--dry-run'], dryRunDir); + // The layer in front, on the name it owns. + parseLayer = await runTsx([CLI, 'generate', 'object', 'class'], parseLayerDir); + charsetLayer = await runTsx([CLI, 'generate', 'object', 'order-line'], charsetLayerDir); + control = await runTsx([CLI, 'generate', 'view', 'order_line'], controlDir); + contextual = await runTsx([CLI, 'generate', 'view', 'type'], contextualDir); +}, RUN_TIMEOUT_MS); + +afterAll(() => { + for (const dir of [ + reservedDir, strictDir, dryRunDir, parseLayerDir, charsetLayerDir, + controlDir, contextualDir, + ]) { + rmSync(dir, { recursive: true, force: true }); + } +}); + +describe('[#17410] `os generate view class` refuses instead of exiting 0', () => { + it('exits non-zero — the reported defect was exit 0', () => { + expect(reserved.code).not.toBe(0); + expect(reserved.code).toBe(1); + }); + + it('says it is refusing, and names the value it refused', () => { + expect(reserved.stdout).toContain('Refusing to generate'); + expect(reserved.stdout).toContain('class'); + }); + + it('names the CONSTRAINT — that the barrel line could not be imported', () => { + // Triage's ask: refuse "loudly, naming the constraint". A bare "invalid + // name" would satisfy the two assertions above. + expect(reserved.stdout).toContain('could not be imported'); + expect(reserved.stdout).toContain('reserved word in this position'); + }); + + it('shows the author the exact barrel line that would have been written', () => { + // The defect is invisible without it: the line parses, so the author has + // no reason to suspect it. Printing it is what connects the refusal to the + // thing they typed. + expect(reserved.stdout).toContain("export { default as class } from './class.view';"); + }); + + it('⛔ does not rewrite the name into an importable one', () => { + // The sanitiser outcome the #16726 ruling refused, and this layer declines + // to be. No repaired alias may appear, and nothing may be reported created. + expect(reserved.stdout).not.toContain('Created'); + expect(reserved.stdout).not.toContain('classView '); + expect(reserved.stdout).not.toContain('klass'); + expect(reserved.stdout).not.toContain('class_'); + }); + + it('writes nothing — no scaffold, no barrel, no directory', () => { + expect(existsSync(join(reservedDir, 'src', 'views', 'class.view.ts'))).toBe(false); + expect(existsSync(join(reservedDir, 'src', 'views', 'index.ts'))).toBe(false); + expect(existsSync(join(reservedDir, 'src'))).toBe(false); + }); + + it('is ONE chokepoint, not one patched generator', () => { + // `flow` + `let`: a different generator and a word reserved for a + // different reason (a module is automatically in strict mode). + expect(strictReserved.code).toBe(1); + expect(strictReserved.stdout).toContain('could not be imported'); + expect(strictReserved.stdout).toContain('strict mode'); + expect(existsSync(join(strictDir, 'src'))).toBe(false); + }); + + it('fires BEFORE the preview, not only before the write', () => { + // A `--dry-run` that prints an unusable barrel and exits 0 is the same + // defect in preview form. + expect(dryRun.code).toBe(1); + expect(dryRun.stdout).toContain('could not be imported'); + expect(dryRun.stdout).not.toContain('Dry run'); + expect(existsSync(join(dryRunDir, 'src'))).toBe(false); + }); +}); + +describe('[#17410] the three layers stay DISTINCT — each keeps its own verdict', () => { + it('the parse check still answers for `os g object class`, in the compiler`s words', () => { + // ⛔ This layer must NOT have shadowed #16541's. `object` binds the bare + // identifier in a `const` position, so the compiler has a reason specific + // to that — and taking it away would be a regression even though the exit + // code is identical. + expect(parseLayer.code).toBe(1); + expect(parseLayer.stdout).toContain('does not parse'); + expect(parseLayer.stdout).toContain("'class' is not allowed as a variable declaration name."); + expect(parseLayer.stdout).not.toContain('could not be imported'); + }); + + it('the charset gate still answers for a name outside the charset', () => { + // ⛔ And #16726's is untouched: `order-line` never reaches either check + // behind it, so the author still gets the schema's own pattern. + expect(charsetLayer.code).toBe(1); + expect(charsetLayer.stdout).toContain('not a name this command accepts'); + expect(charsetLayer.stdout).not.toContain('could not be imported'); + expect(charsetLayer.stdout).not.toContain('does not parse'); + }); + + it('⛔ no third charset — the refusal asserts no character rule', () => { + // The #16726 ruling's other half, asserted on the output rather than + // trusted. Spelled as "does not print the charset gate's own lines" + // rather than "does not contain the word charset": this refusal SAYS it is + // not a charset, in prose, so the bare word is present on purpose and + // matching it would pin the disclaimer instead of the rule. + expect(reserved.stdout).not.toContain('must match pattern'); + expect(reserved.stdout).not.toContain('not a name this command accepts'); + // And it does say so, which is the half worth pinning. + expect(reserved.stdout).toContain('it is not a charset'); + }); +}); + +describe('[#17410] ⭐ CONTROL — names that work today still generate', () => { + it('an ordinary name still exits 0 and writes both files', () => { + // A refusal that fired on everything would satisfy every assertion above. + expect(control.code).toBe(0); + expect(control.stdout).toContain('Created src/views/order_line.view.ts'); + expect(control.stdout).toContain('Created src/views/index.ts'); + const barrel = readFileSync(join(controlDir, 'src', 'views', 'index.ts'), 'utf8'); + expect(barrel).toContain("export { default as orderLine } from './order_line.view';"); + }); + + it('⭐ a CONTEXTUAL reserved word still generates — the expensive direction', () => { + // `type` is keyword-shaped and a perfectly legal import binding. Refusing + // it would break a name that works today, which is the failure direction + // that costs an author a working command — and the direction a hand-picked + // keyword list gets wrong. Read from disk, not from the exit code. + expect(contextual.code).toBe(0); + const barrel = readFileSync(join(contextualDir, 'src', 'views', 'index.ts'), 'utf8'); + expect(barrel).toContain("export { default as type } from './type.view';"); + expect(existsSync(join(contextualDir, 'src', 'views', 'type.view.ts'))).toBe(true); + }); +}); diff --git a/packages/cli/test/importable-binding.test.ts b/packages/cli/test/importable-binding.test.ts new file mode 100644 index 0000000000..df6d5fb10c --- /dev/null +++ b/packages/cli/test/importable-binding.test.ts @@ -0,0 +1,221 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * PIN (#17410) — the instrument that decides whether a generated barrel alias + * can be imported by name, measured across every class of word that matters. + * + * ## What this file measures, and what it deliberately does not + * + * This is the UNIT half: given an alias, does the check refuse it. The other + * half — that the COMMAND consults it, before the writes, on every branch, and + * exits non-zero — cannot be measured here, because `process.exitCode` set in + * a vitest worker is not an exit status. That half is + * `generate-refuses-unimportable-alias.test.ts`, on a real child process, for + * the same reasons its two siblings document at length. + * + * ## ⛔ Why the rows are word CLASSES and not a list of keywords + * + * The implementation could have been an array of reserved words, and the + * reason it is not is the reason these rows are grouped this way: a list is + * wrong in **both** directions, and only a matrix that carries both failure + * directions can hold that. The classes below are therefore not decoration — + * each is a distinct way to get this wrong: + * + * - `ALWAYS_RESERVED` (36) — the words everybody writes down. Refusing only + * these is the **too narrow** failure. + * - `STRICT_MODE_RESERVED` (10) — reserved because a module is automatically + * in strict mode (`await` because it is reserved at a module's top level). + * Every one is charset-legal and every one reached `exit 0` before this + * layer, so a list stopping at the obvious 36 ships the same defect for + * them. These are the rows a SYNTACTIC-only verdict cannot see at all: the + * compiler reports them as grammar errors in the SEMANTIC bucket, which is + * why the instrument reads both and why its probe resolves its own import. + * - `CONTEXTUAL_ONLY` (31) — legal import bindings that merely look + * keyword-ish. Refusing one of these is the **too wide** failure, and it is + * the expensive direction: it breaks a name that works today. ⭐ These rows + * are the load-bearing control of this file. + * - `REAL_NAMES` (14) — the second control: ordinary authored names, + * including near-misses (`classy`, `klass`, `letter`, `statically`, + * `awaited`, `myClass`) that a substring-matching implementation would + * refuse while satisfying every assertion about `class` and `let`. + * - `MALFORMED` (8) — multi-token and escape-shaped aliases. They are + * stopped by the charset gate long before this layer, and are asserted + * here anyway so this layer is measured as standing on its own rather than + * on the one in front: a bare `import { a, b } from './m'` parses fine, it + * is simply a different import, and only asking the graph to hold together + * refuses it. ⛔ A future edit that makes this layer depend on the charset + * gate reddens here. + * + * A refusal that fired on everything would satisfy every `REFUSES` row in this + * file and would be a far worse command than the broken one, so the two + * control classes are asserted with the same instrument, in the same run. + */ + +import { describe, expect, it } from 'vitest'; +import { ts } from 'ts-morph'; +import { namedImportDiagnostics, findBarrelAliasRefusal } from '../src/utils/importable-binding.js'; + +/** Reserved in every context — illegal as an import binding, always. */ +const ALWAYS_RESERVED = [ + 'break', 'case', 'catch', 'class', 'const', 'continue', 'debugger', 'default', + 'delete', 'do', 'else', 'enum', 'export', 'extends', 'false', 'finally', + 'for', 'function', 'if', 'import', 'in', 'instanceof', 'new', 'null', + 'return', 'super', 'switch', 'this', 'throw', 'true', 'try', 'typeof', + 'var', 'void', 'while', 'with', +]; + +/** + * Reserved because the file is a MODULE, and modules are automatically in + * strict mode. Invisible to a syntactic-only verdict. + */ +const STRICT_MODE_RESERVED = [ + 'implements', 'interface', 'let', 'package', 'private', 'protected', + 'public', 'static', 'yield', 'await', +]; + +/** ⭐ CONTROL — keyword-ish, and perfectly legal as an import binding. */ +const CONTEXTUAL_ONLY = [ + 'as', 'async', 'from', 'get', 'of', 'set', 'type', 'any', 'string', 'number', + 'boolean', 'never', 'unknown', 'declare', 'namespace', 'module', 'abstract', + 'asserts', 'infer', 'is', 'keyof', 'readonly', 'require', 'satisfies', 'out', + 'accessor', 'using', 'undefined', 'globalThis', 'arguments', 'eval', +]; + +/** ⭐ CONTROL — ordinary names, plus near-misses of the refused words. */ +const REAL_NAMES = [ + 'order_line', 'orderLine', 'classViews', '_internal', 'x2', 'customer', + 'sales_order', 'lead_qual', 'myClass', 'classy', 'klass', 'letter', + 'statically', 'awaited', +]; + +/** Not a single token at all — the charset gate's territory, asserted anyway. */ +const MALFORMED = [ + 'a, b', + 'a as b', + 'a } from "./x"; const y = 1; //', + 'a\nb', + 'a.b', + 'a-b', + 'a*/b', + 'a`b', +]; + +describe('[#17410] the reserved-word classes a keyword list gets wrong', () => { + it.each(ALWAYS_RESERVED)('refuses `%s` — reserved in every context', (alias) => { + expect(namedImportDiagnostics(ts, alias).length).toBeGreaterThan(0); + }); + + it.each(STRICT_MODE_RESERVED)( + 'refuses `%s` — reserved because a module is strict mode', + (alias) => { + // ⚠️ The rows a syntactic-only check cannot see. Asserted individually + // so that narrowing the instrument back to one bucket names the word it + // stopped seeing rather than emptying a whole describe block. + expect(namedImportDiagnostics(ts, alias).length).toBeGreaterThan(0); + }, + ); + + it('names the STRICT-MODE reason, so the verdict is not a coincidence', () => { + // A refusal that fired for the wrong reason would satisfy the row above. + // These words are refused specifically because a module is strict, and the + // compiler says so — if this stops matching, the instrument has changed + // what it is measuring even though the pass/fail is unchanged. + expect(namedImportDiagnostics(ts, 'let').join(' ')).toContain( + "'let' is a reserved word in strict mode", + ); + expect(namedImportDiagnostics(ts, 'await').join(' ')).toContain( + "'await' is a reserved word at the top-level of a module", + ); + }); + + it('⛔ the always-reserved set is refused for a DIFFERENT reason than strict mode', () => { + // `class` cannot be an import binding at all, strict mode or not, and the + // compiler's reason for it carries no strict-mode clause. Pinned so the + // two classes cannot silently collapse into one code path. + const classReasons = namedImportDiagnostics(ts, 'class').join(' '); + expect(classReasons).toContain('Identifier expected'); + expect(classReasons).not.toContain('strict mode'); + }); +}); + +describe('[#17410] ⭐ CONTROL — every name that works today still works', () => { + it.each(CONTEXTUAL_ONLY)('admits `%s` — contextual only, legal as a binding', (alias) => { + // ⛔ The expensive failure direction. Refusing one of these breaks a + // working command, which is why the accept verdict is asserted per word + // rather than as a count. + expect(namedImportDiagnostics(ts, alias)).toEqual([]); + }); + + it.each(REAL_NAMES)('admits `%s` — an ordinary authored name', (alias) => { + expect(namedImportDiagnostics(ts, alias)).toEqual([]); + }); + + it('the control classes are not empty, and the two verdicts really differ', () => { + // Guards the shape of this file rather than the code: an `it.each` over an + // accidentally-empty array passes by running nothing. + expect(CONTEXTUAL_ONLY.length).toBe(31); + expect(REAL_NAMES.length).toBe(14); + expect(ALWAYS_RESERVED.length).toBe(36); + expect(STRICT_MODE_RESERVED.length).toBe(10); + // And the instrument discriminates: same call, opposite answers. + expect(namedImportDiagnostics(ts, 'class')).not.toEqual([]); + expect(namedImportDiagnostics(ts, 'classy')).toEqual([]); + }); +}); + +describe('[#17410] this layer stands on its own, not on the charset gate', () => { + it.each(MALFORMED)('refuses `%j` — not a single import binding', (alias) => { + expect(namedImportDiagnostics(ts, alias).length).toBeGreaterThan(0); + }); + + it('⛔ a bare import clause is NOT enough to refuse a multi-token alias', () => { + // The measurement that decided the probe's shape, kept as an assertion so + // the reasoning cannot be simplified away. `import { a, b } from './m'` + // parses — it is simply a *different* import — so a probe that only parsed + // an import clause would admit it. Requiring the alias to be REFERRED to + // is what refuses it. + const fileName = 'probe.ts'; + const source = `import { a, b } from './m';`; + const sourceFile = ts.createSourceFile( + fileName, source, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS, + ); + const host: ts.CompilerHost = { + getSourceFile: (requested) => (requested === fileName ? sourceFile : undefined), + getDefaultLibFileName: () => 'lib.d.ts', + writeFile: () => {}, + getCurrentDirectory: () => '/', + getCanonicalFileName: (f) => f, + useCaseSensitiveFileNames: () => true, + getNewLine: () => '\n', + fileExists: (f) => f === fileName, + readFile: (f) => (f === fileName ? source : undefined), + }; + const program = ts.createProgram( + [fileName], { noLib: true, noResolve: true, target: ts.ScriptTarget.Latest }, host, + ); + expect(program.getSyntacticDiagnostics(sourceFile)).toEqual([]); + // ⇒ and the instrument this command uses refuses it anyway. + expect(namedImportDiagnostics(ts, 'a, b').length).toBeGreaterThan(0); + }); +}); + +describe('[#17410] the async wrapper the command actually calls', () => { + it('returns the compiler`s reasons for a refused alias, and null for an accepted one', async () => { + const refused = await findBarrelAliasRefusal('class'); + expect(refused).not.toBeNull(); + expect(refused!.length).toBeGreaterThan(0); + // ⛔ `null` is the ACCEPT verdict, never "could not run": a wrapper that + // swallowed a loader failure would return null for `class` too. + expect(await findBarrelAliasRefusal('order_line')).toBeNull(); + }); + + it('agrees with the synchronous instrument it wraps', async () => { + // One question, one answer — a wrapper that drifted would let the command + // and every pin above disagree while both stayed green. + for (const alias of ['class', 'let', 'await', 'type', 'order_line']) { + const wrapped = await findBarrelAliasRefusal(alias); + const direct = namedImportDiagnostics(ts, alias); + expect(wrapped).toEqual(direct.length > 0 ? direct : null); + } + }); +});