From f29862a9bcff0338d9c567b54f8a9881776d6212 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 29 Jul 2026 21:59:49 -0300 Subject: [PATCH 1/8] feat(commands): add defineCommand with typed options via an ICommand adapter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commands can now be declared as plain objects: a name, an option schema built from booleanOption/stringOption/numberOption/arrayOption, and a run function whose context carries the positional args plus the declared options, typed by inference from the schema. lib/common/define-command holds the types and the pure factories only, so it stays side-effect-free and can be re-exported from nativescript/contracts. The runtime bridge lives in lib/common/services/command-definition-adapter, which compiles a definition into the ICommand the legacy registry expects and runs it inside an injection context. canExecute is emitted only when the definition supplies one or opts into arguments: "any"; CommandsService skips all parameter validation as soon as canExecute exists, so omitting it is what lets the framework reject stray positional arguments for arguments: "none". Fully additive — existing ICommand classes are untouched. --- defining-commands.md | 251 ++++++++++ lib/common/define-command.ts | 104 ++++ .../services/command-definition-adapter.ts | 120 +++++ lib/contracts/index.ts | 18 + test/define-command.ts | 461 ++++++++++++++++++ 5 files changed, 954 insertions(+) create mode 100644 defining-commands.md create mode 100644 lib/common/define-command.ts create mode 100644 lib/common/services/command-definition-adapter.ts create mode 100644 test/define-command.ts diff --git a/defining-commands.md b/defining-commands.md new file mode 100644 index 0000000000..fa0143123b --- /dev/null +++ b/defining-commands.md @@ -0,0 +1,251 @@ +Defining Commands +================= + +`defineCommand` is the declarative way to add a command to the NativeScript +CLI. A definition is a plain object: a name, an option schema, and a `run` +function. The CLI compiles it into the command shape its registry expects, so a +definition gets the same option parsing, hooks, analytics and help wiring as a +hand-written command class — without a class, a constructor, or an +`allowedParameters` array. + +This is purely additive. The legacy `ICommand` classes registered through +`$injector.registerCommand` keep working exactly as before, and the two styles +coexist in the same registry. + +At a glance +----------- + +```ts +import { + defineCommand, + booleanOption, + stringOption, +} from "nativescript/contracts"; + +export default defineCommand({ + name: "widget|add", + description: "Adds a widget to the project", + options: { + verbose: booleanOption({ default: false }), + output: stringOption({ alias: "o" }), + }, + arguments: "any", + async run(ctx) { + // ctx.args -> string[] of positional arguments + // ctx.options -> { verbose: boolean; output: string } + if (ctx.options.verbose) { + console.log(`adding ${ctx.args.join(", ")} to ${ctx.options.output}`); + } + }, +}); +``` + +`defineCommand` returns the definition, tagged with a marker symbol so that any +copy of the CLI can recognise it (`isCommandDefinition(value)` is the exported +predicate). It does not register anything by itself — see +[Registering a definition](#registering-a-definition). + +Names and the command hierarchy +------------------------------- + +`name` is either a single string or an array of strings, in which case every +entry becomes an alias for the same command. + +The CLI's command registry is flat; the hierarchy the user types on the command +line is encoded in the name with a `|` separator. `"widget|add"` is the command +invoked as `ns widget add`, and `"widget|template|list"` is `ns widget template +list`. Registering a hierarchical name automatically synthesises the parent +dispatcher (`widget`), which routes to the right subcommand or prints help. + +A leading `*` on the last segment marks a **default subcommand**: `"widget|*add"` +runs both for `ns widget add` and for a bare `ns widget`. This is the convention +the CLI's own commands use (`run|*all`, `debug|*all`); the encoding is +user-visible because it feeds shell autocompletion and generated help. + +Options +------- + +`options` is a schema keyed by the long option name — `verbose` is passed as +`--verbose`. Declare each entry with one of the four helpers, which fix the +value type: + +| Helper | Value type on `ctx.options` | +| --------------- | --------------------------- | +| `booleanOption` | `boolean` | +| `stringOption` | `string` | +| `numberOption` | `number` | +| `arrayOption` | `string[]` | + +Each helper takes an optional spec: + +```ts +options: { + // --release, absent means false + release: booleanOption({ default: false }), + // --output , also accepted as -o + output: stringOption({ alias: "o", description: "Output directory" }), + // --retries + retries: numberOption({ default: 3 }), + // --file a.ts --file b.ts + file: arrayOption(), + // kept out of analytics and logs + token: stringOption({ hasSensitiveValue: true }), +} +``` + +- `default` — value used when the flag is absent. +- `alias` — single-dash shorthand, or an array of them. +- `hasSensitiveValue` — defaults to `false`; set it for anything that must not + be recorded. There is no reason not to be explicit about credentials, paths + containing user directories, and tokens. +- `description` — shown in generated help. + +The schema types `ctx.options` and nothing else: `ctx.options` carries exactly +the declared keys, with the types the helpers imply. Values that the CLI parses +globally (`--path`, `--log`, …) are not exposed there; resolve the `options` +service if you need them. + +### How validation behaves + +Option validation is the CLI's existing behaviour, not something the definition +opts into. Before a command runs, the parser is re-primed with that command's +declared options and the command line is re-parsed: + +- Declared options are accepted and appear on `ctx.options`. +- An option the CLI does not know — neither global nor declared by this command + — is a **hard error**: the command does not run, and the CLI prints + `The option '' is not supported.` followed by a help suggestion. + +So adding an option is exactly a matter of adding a schema entry; forgetting to +declare one that users pass is a failure, not a silent `undefined`. + +Positional arguments +-------------------- + +`arguments` declares whether the command takes positional arguments at all: + +- `"none"` (the default) — the command accepts no positional arguments. Passing + any is rejected with `This command doesn't accept parameters.` +- `"any"` — positional arguments are accepted and handed to `run` as + `ctx.args`. + +Anything finer than that belongs in `canExecute`. + +### `canExecute` owns validation + +```ts +defineCommand({ + name: "widget|add", + arguments: "any", + async canExecute(ctx) { + return ctx.args.length === 1; + }, + async run(ctx) { + /* ... */ + }, +}); +``` + +`canExecute` receives the same context as `run` and returns a boolean (or a +promise of one). Returning `false` aborts the command and prints a help +suggestion; throwing surfaces your own error message, which is usually the +friendlier choice. + +There is one rule to internalise: **the moment a command supplies +`canExecute`, it owns argument validation completely.** The framework returns +that verdict and skips every built-in parameter check, including the +"no parameters" rule implied by `arguments: "none"`. A definition with a +`canExecute` that only inspects options will therefore accept stray positional +arguments unless it checks `ctx.args` itself. If you do not need custom +validation, omit `canExecute` and let `arguments` do the work. + +The run context +--------------- + +`run(ctx)` receives: + +- `ctx.args` — `string[]`, the positional arguments left after the command name + (including any subcommand segments) has been consumed. +- `ctx.options` — the current value of each declared option, read at the moment + the command executes. + +`run` may be synchronous or `async`; the CLI awaits the result and treats a +rejection as a command failure. + +`run` starts inside a dependency-injection context, so `inject()` works +directly: + +```ts +import { defineCommand, inject } from "nativescript/contracts"; +import { DoctorService } from "nativescript/contracts"; + +export default defineCommand({ + name: "widget|check", + async run() { + const doctorService = inject(DoctorService); + await doctorService.printWarnings(); + }, +}); +``` + +The injection context is synchronous: `inject()` is valid up to the first +`await` in `run`, and not after it. Capture what you need at the top of `run`, +or inject the `Injector` itself and use `injector.get()` for late lookups. See +`dependency-injection.md`. + +Other flags +----------- + +- `disableAnalytics: true` — skips analytics tracking for this command. +- `enableHooks: false` — skips the before/after hooks that normally run around + the command. Hooks are enabled by default. + +Both are simply passed through to the command the CLI executes; omitting them +leaves the CLI's defaults in place. + +Registering a definition +------------------------ + +Inside the CLI, a definition is registered with `registerCommandDefinition`: + +```ts +import { registerCommandDefinition } from "../common/services/command-definition-adapter"; +import addWidgetCommand from "./add-widget"; + +registerCommandDefinition(addWidgetCommand); +``` + +It registers the definition under every name it declares, on the CLI's injector +by default; pass a second argument to target a different injector (tests do +this). The command instance is created on first resolution and cached. + +`registerCommandDefinition` lives in +`lib/common/services/command-definition-adapter` rather than in +`nativescript/contracts`, because it reaches into the CLI runtime — the +side-effect-free contracts entry point deliberately does not pull it in. +`defineCommand`, the option helpers and all the types are exported from both +`nativescript/contracts` and `lib/common/define-command`. + +Declaring commands from an extension manifest, so that an extension does not +have to call a registration function at load time, is being added separately. +Until then, extensions register definitions the same way the CLI does. + +Relationship to `ICommand` +-------------------------- + +A definition is compiled into an ordinary `ICommand`, so nothing downstream — +the registry, the router, hooks, help, analytics — knows the difference. The +mapping is: + +| Definition | `ICommand` | +| --------------------------------- | ------------------------------------------ | +| `options` | `dashedOptions` | +| `run` | `execute`, wrapped in an injection context | +| `arguments`, `canExecute` | `canExecute` (see the rule above) | +| — | `allowedParameters`, always `[]` | +| `disableAnalytics`, `enableHooks` | passed through unchanged | + +Existing command classes need no migration. Reach for a definition when a +command is mostly "parse these flags and do this"; a class still makes sense +when a command needs constructor-injected collaborators shared across several +methods, custom `ICommandParameter` validators, or a `postCommandAction`. diff --git a/lib/common/define-command.ts b/lib/common/define-command.ts new file mode 100644 index 0000000000..b197709ca9 --- /dev/null +++ b/lib/common/define-command.ts @@ -0,0 +1,104 @@ +/** + * The declarative command API. Types and pure factories only — this module is + * re-exported from `nativescript/contracts` and must stay side-effect-free, so + * it may not import lib/common/yok (whose import creates global.$injector). + * The runtime bridge onto the legacy registry lives in + * lib/common/services/command-definition-adapter. + */ + +/** + * Symbol.for so that a definition produced by one copy of the CLI is still + * recognised by another — extensions bundle their own node_modules. + */ +export const COMMAND_DEFINITION_MARKER = Symbol.for( + "nativescript:cli:commandDefinition", +); + +export type CommandOptionType = "boolean" | "string" | "number" | "array"; + +export interface ICommandOptionSpec { + type: CommandOptionType; + /** Value used when the flag is absent from the command line. */ + default?: TValue; + /** Single-dash shorthand, e.g. `-o` for `--output`. */ + alias?: string | string[]; + /** Keeps the value out of analytics and logs. Defaults to false. */ + hasSensitiveValue?: boolean; + description?: string; +} + +/** The parts of an option spec a caller supplies; `type` comes from the helper. */ +export type CommandOptionSpecInit = Omit< + ICommandOptionSpec, + "type" +>; + +export interface ICommandOptionsSchema { + [optionName: string]: ICommandOptionSpec; +} + +export type CommandOptionValues = { + [K in keyof TSchema]: TSchema[K] extends ICommandOptionSpec + ? TValue + : any; +}; + +export interface ICommandContext< + TSchema extends ICommandOptionsSchema = ICommandOptionsSchema, +> { + /** Positional arguments, after the command name has been consumed. */ + args: string[]; + /** Current value of every option declared in the schema, and nothing else. */ + options: CommandOptionValues; +} + +export interface ICommandDefinition< + TSchema extends ICommandOptionsSchema = ICommandOptionsSchema, +> { + /** `"widget|add"`; `|` separates hierarchy levels. Several names alias one command. */ + name: string | string[]; + description?: string; + options?: TSchema; + /** + * `"none"` (the default) rejects positional arguments; `"any"` accepts them. + * Anything finer belongs in `canExecute`. + */ + arguments?: "none" | "any"; + canExecute?(context: ICommandContext): Promise | boolean; + disableAnalytics?: boolean; + enableHooks?: boolean; + run(context: ICommandContext): Promise | void; +} + +const optionSpec = ( + type: CommandOptionType, + init: CommandOptionSpecInit, +): ICommandOptionSpec => ({ ...init, type }); + +export const booleanOption = ( + init: CommandOptionSpecInit = {}, +): ICommandOptionSpec => optionSpec("boolean", init); + +export const stringOption = ( + init: CommandOptionSpecInit = {}, +): ICommandOptionSpec => optionSpec("string", init); + +export const numberOption = ( + init: CommandOptionSpecInit = {}, +): ICommandOptionSpec => optionSpec("number", init); + +export const arrayOption = ( + init: CommandOptionSpecInit = {}, +): ICommandOptionSpec => optionSpec("array", init); + +export function defineCommand( + definition: ICommandDefinition, +): ICommandDefinition { + const marked: ICommandDefinition = { ...definition }; + (marked)[COMMAND_DEFINITION_MARKER] = true; + return marked; +} + +export function isCommandDefinition(value: any): value is ICommandDefinition { + return !!value && (value)[COMMAND_DEFINITION_MARKER] === true; +} diff --git a/lib/common/services/command-definition-adapter.ts b/lib/common/services/command-definition-adapter.ts new file mode 100644 index 0000000000..0956aa1db7 --- /dev/null +++ b/lib/common/services/command-definition-adapter.ts @@ -0,0 +1,120 @@ +import { OptionType } from "../enums"; +import { injector } from "../yok"; +import { runInInjectionContext } from "../di/inject"; +import { IDictionary, IDashedOption } from "../declarations"; +import { IInjector } from "../definitions/yok"; +import { ICommand } from "../definitions/commands"; +import { + CommandOptionType, + ICommandContext, + ICommandDefinition, + ICommandOptionsSchema, +} from "../define-command"; + +const OPTION_TYPES: IDictionary = { + boolean: OptionType.Boolean, + string: OptionType.String, + number: OptionType.Number, + array: OptionType.Array, +}; + +/** + * Wraps a declarative definition in the ICommand shape the legacy registry and + * CommandsService expect. + * + * Constraint worth knowing before changing the canExecute mapping: the moment + * an ICommand exposes `canExecute`, CommandsService returns its verdict and + * skips `allowedParameters` validation entirely. So `canExecute` is emitted + * only when the definition supplies one or opts into `arguments: "any"` — the + * adapter then owns argument validation wholesale. With `arguments: "none"` + * and no user `canExecute` the property is omitted, which lets the framework's + * own empty-`allowedParameters` check reject stray positional arguments. + */ +export function createCommandFromDefinition< + TSchema extends ICommandOptionsSchema, +>( + definition: ICommandDefinition, + targetInjector: IInjector = injector, +): ICommand { + const schema = definition.options || {}; + const optionNames = Object.keys(schema); + + const dashedOptions: IDictionary = {}; + for (const optionName of optionNames) { + const spec = schema[optionName]; + const dashedOption: IDashedOption = { + type: OPTION_TYPES[spec.type], + hasSensitiveValue: spec.hasSensitiveValue === true, + }; + + if (spec.default !== undefined) { + dashedOption.default = spec.default; + } + + if (spec.alias !== undefined) { + dashedOption.alias = spec.alias; + } + + if (spec.description !== undefined) { + dashedOption.describe = spec.description; + } + + dashedOptions[optionName] = dashedOption; + } + + // Resolved per call: the options service only holds parsed values once + // validateOptions has run for this command. + const buildContext = (args: string[]): ICommandContext => { + const optionsService: any = targetInjector.resolve("options"); + const options: any = {}; + for (const optionName of optionNames) { + options[optionName] = optionsService + ? optionsService[optionName] + : undefined; + } + + return { args, options }; + }; + + const command: ICommand = { + allowedParameters: [], + dashedOptions, + execute: async (args: string[]): Promise => { + // runInInjectionContext is synchronous, so inject() is available while + // run() executes up to its first await, and not after it. + await runInInjectionContext((targetInjector).di, () => + definition.run(buildContext(args)), + ); + }, + }; + + if (definition.canExecute || definition.arguments === "any") { + command.canExecute = async (args: string[]): Promise => + definition.canExecute + ? await definition.canExecute(buildContext(args)) + : true; + } + + if (definition.disableAnalytics !== undefined) { + command.disableAnalytics = definition.disableAnalytics; + } + + if (definition.enableHooks !== undefined) { + command.enableHooks = definition.enableHooks; + } + + return command; +} + +export function registerCommandDefinition< + TSchema extends ICommandOptionsSchema, +>( + definition: ICommandDefinition, + targetInjector: IInjector = injector, +): void { + // An arrow function resolver is invoked as a factory rather than new-ed, and + // its result is cached as the command instance. + targetInjector.registerCommand(definition.name, () => + createCommandFromDefinition(definition, targetInjector), + ); +} diff --git a/lib/contracts/index.ts b/lib/contracts/index.ts index c13a9b9996..efbdc504f6 100644 --- a/lib/contracts/index.ts +++ b/lib/contracts/index.ts @@ -25,3 +25,21 @@ export type { export { DoctorService } from "./doctor-service"; export { ProjectNameService } from "./project-name-service"; + +export { + defineCommand, + isCommandDefinition, + booleanOption, + stringOption, + numberOption, + arrayOption, +} from "../common/define-command"; +export type { + ICommandDefinition, + ICommandContext, + ICommandOptionSpec, + ICommandOptionsSchema, + CommandOptionSpecInit, + CommandOptionType, + CommandOptionValues, +} from "../common/define-command"; diff --git a/test/define-command.ts b/test/define-command.ts new file mode 100644 index 0000000000..011ff59f31 --- /dev/null +++ b/test/define-command.ts @@ -0,0 +1,461 @@ +import { assert } from "chai"; +import { Yok, injector as globalInjector } from "../lib/common/yok"; +import { IInjector } from "../lib/common/definitions/yok"; +import { inject } from "../lib/common/di"; +import { CommandsService } from "../lib/common/services/commands-service"; +import { LoggerStub, HooksServiceStub } from "./stubs"; +import { + arrayOption, + booleanOption, + defineCommand, + isCommandDefinition, + numberOption, + stringOption, +} from "../lib/common/define-command"; +import { + createCommandFromDefinition, + registerCommandDefinition, +} from "../lib/common/services/command-definition-adapter"; + +type IsExact = + (() => T extends A ? 1 : 2) extends () => T extends B ? 1 : 2 + ? true + : false; + +/** Fails to compile unless the schema inferred exactly the expected type. */ +const expectExactType = (): void => undefined; + +const createTestInjector = (options: any = {}): IInjector => { + const testInjector = new Yok(); + testInjector.register("options", options); + return testInjector; +}; + +describe("defineCommand", () => { + it("marks definitions so a duplicated CLI copy still recognises them", () => { + const definition = defineCommand({ + name: "dctest-marker", + run: () => undefined, + }); + + assert.isTrue(isCommandDefinition(definition)); + assert.isTrue( + (definition)[Symbol.for("nativescript:cli:commandDefinition")], + ); + assert.isFalse(isCommandDefinition({ name: "dctest-marker" })); + assert.isFalse(isCommandDefinition(null)); + }); + + describe("registration", () => { + it("round-trips through the legacy command registry", () => { + const definition = defineCommand({ + name: "dctestwidget|add", + description: "Adds a widget", + run: () => undefined, + }); + + // The parent dispatcher is synthesized onto the global facade by the + // legacy registry, so registration happens there too. + registerCommandDefinition(definition, globalInjector); + + const command = globalInjector.resolveCommand("dctestwidget|add"); + assert.isFunction(command.execute); + assert.deepEqual(command.allowedParameters, []); + + const parent = globalInjector.resolveCommand("dctestwidget"); + assert.isTrue(parent.isHierarchicalCommand); + + assert.include( + globalInjector.getRegisteredCommandsNames(false), + "dctestwidget|add", + ); + }); + + it("caches one command instance per registered name", () => { + const testInjector = createTestInjector(); + registerCommandDefinition( + defineCommand({ name: "dctestflat", run: () => undefined }), + testInjector, + ); + + assert.strictEqual( + testInjector.resolveCommand("dctestflat"), + testInjector.resolveCommand("dctestflat"), + ); + }); + + it("registers every alias of a multi-name definition", () => { + const testInjector = createTestInjector(); + registerCommandDefinition( + defineCommand({ + name: ["dctestalias", "dctestalias2"], + run: () => undefined, + }), + testInjector, + ); + + assert.isFunction(testInjector.resolveCommand("dctestalias").execute); + assert.isFunction(testInjector.resolveCommand("dctestalias2").execute); + }); + }); + + describe("execute", () => { + it("passes args and the declared options through, inside an injection context", async () => { + const testInjector = createTestInjector({ + verbose: true, + output: "dist", + undeclared: "ignored", + }); + testInjector.register("dcTestGreeter", { greet: () => "hello" }); + + let capturedArgs: string[]; + let capturedOptions: any; + let greeting: string; + + const command = createCommandFromDefinition( + defineCommand({ + name: "dctestexec", + options: { + verbose: booleanOption(), + output: stringOption(), + }, + run(context) { + greeting = inject("dcTestGreeter").greet(); + capturedArgs = context.args; + capturedOptions = context.options; + }, + }), + testInjector, + ); + + await command.execute(["one", "two"]); + + assert.deepEqual(capturedArgs, ["one", "two"]); + assert.deepEqual(capturedOptions, { verbose: true, output: "dist" }); + assert.strictEqual(greeting, "hello"); + }); + + it("reads option values at execution time", async () => { + const optionsService: any = { verbose: false }; + const testInjector = createTestInjector(optionsService); + + let seen: boolean; + const command = createCommandFromDefinition( + defineCommand({ + name: "dctestlate", + options: { verbose: booleanOption() }, + run: (context) => { + seen = context.options.verbose; + }, + }), + testInjector, + ); + + optionsService.verbose = true; + await command.execute([]); + + assert.isTrue(seen); + }); + + it("awaits an asynchronous run", async () => { + const testInjector = createTestInjector(); + let finished = false; + + const command = createCommandFromDefinition( + defineCommand({ + name: "dctestasync", + run: async () => { + await new Promise((resolve) => setTimeout(resolve, 1)); + finished = true; + }, + }), + testInjector, + ); + + await command.execute([]); + + assert.isTrue(finished); + }); + + it("infers the option value types on the run context", async () => { + const testInjector = createTestInjector({ + verbose: true, + output: "dist", + retries: 3, + files: ["a.ts"], + }); + + let verbose: boolean; + let output: string; + let retries: number; + let files: string[]; + + const command = createCommandFromDefinition( + defineCommand({ + name: "dctesttypes", + options: { + verbose: booleanOption(), + output: stringOption(), + retries: numberOption(), + files: arrayOption(), + }, + run: (context) => { + expectExactType>(); + expectExactType>(); + expectExactType>(); + expectExactType>(); + + verbose = context.options.verbose; + output = context.options.output; + retries = context.options.retries; + files = context.options.files; + }, + }), + testInjector, + ); + + await command.execute([]); + + assert.isTrue(verbose); + assert.strictEqual(output, "dist"); + assert.strictEqual(retries, 3); + assert.deepEqual(files, ["a.ts"]); + }); + }); + + describe("dashedOptions", () => { + it("compiles the schema into the shape the option parser expects", () => { + const command = createCommandFromDefinition( + defineCommand({ + name: "dctestdashed", + options: { + verbose: booleanOption({ default: false }), + output: stringOption({ alias: "o" }), + retries: numberOption({ default: 3 }), + files: arrayOption(), + token: stringOption({ + hasSensitiveValue: true, + description: "Auth token", + }), + }, + run: () => undefined, + }), + createTestInjector(), + ); + + assert.deepEqual(command.dashedOptions, { + verbose: { type: "boolean", hasSensitiveValue: false, default: false }, + output: { type: "string", hasSensitiveValue: false, alias: "o" }, + retries: { type: "number", hasSensitiveValue: false, default: 3 }, + files: { type: "array", hasSensitiveValue: false }, + token: { + type: "string", + hasSensitiveValue: true, + describe: "Auth token", + }, + }); + }); + + it("is empty when no options are declared", () => { + const command = createCommandFromDefinition( + defineCommand({ name: "dctestnoopts", run: () => undefined }), + createTestInjector(), + ); + + assert.deepEqual(command.dashedOptions, {}); + }); + }); + + describe("canExecute", () => { + it("is omitted for argument-less commands so the framework rejects parameters", () => { + const testInjector = createTestInjector(); + + const implicit = createCommandFromDefinition( + defineCommand({ name: "dctestnone", run: () => undefined }), + testInjector, + ); + const explicit = createCommandFromDefinition( + defineCommand({ + name: "dctestnone2", + arguments: "none", + run: () => undefined, + }), + testInjector, + ); + + assert.isUndefined(implicit.canExecute); + assert.isUndefined(explicit.canExecute); + assert.deepEqual(implicit.allowedParameters, []); + assert.deepEqual(explicit.allowedParameters, []); + }); + + it("accepts anything when arguments are 'any'", async () => { + const command = createCommandFromDefinition( + defineCommand({ + name: "dctestany", + arguments: "any", + run: () => undefined, + }), + createTestInjector(), + ); + + assert.isFunction(command.canExecute); + assert.isTrue(await command.canExecute(["whatever", "else"])); + }); + + it("hands the context to a user canExecute and honours its verdict", async () => { + const testInjector = createTestInjector({ force: true }); + let capturedContext: any; + + const build = (verdict: boolean) => + createCommandFromDefinition( + defineCommand({ + name: "dctestverdict", + arguments: "any", + options: { force: booleanOption() }, + canExecute: (context) => { + capturedContext = context; + return verdict; + }, + run: () => undefined, + }), + testInjector, + ); + + assert.isTrue(await build(true).canExecute(["android"])); + assert.deepEqual(capturedContext.args, ["android"]); + assert.deepEqual(capturedContext.options, { force: true }); + + assert.isFalse(await build(false).canExecute(["android"])); + }); + + it("is emitted for a user canExecute even when arguments are 'none'", async () => { + const command = createCommandFromDefinition( + defineCommand({ + name: "dctestnonecan", + arguments: "none", + canExecute: async () => true, + run: () => undefined, + }), + createTestInjector(), + ); + + assert.isFunction(command.canExecute); + assert.isTrue(await command.canExecute([])); + }); + }); + + describe("command flags", () => { + it("passes disableAnalytics and enableHooks through", () => { + const command = createCommandFromDefinition( + defineCommand({ + name: "dctestflags", + disableAnalytics: true, + enableHooks: false, + run: () => undefined, + }), + createTestInjector(), + ); + + assert.isTrue(command.disableAnalytics); + assert.isFalse(command.enableHooks); + }); + + it("leaves both absent when the definition omits them", () => { + const command = createCommandFromDefinition( + defineCommand({ name: "dctestnoflags", run: () => undefined }), + createTestInjector(), + ); + + assert.isFalse("disableAnalytics" in command); + assert.isFalse("enableHooks" in command); + }); + }); + + describe("end to end through CommandsService", () => { + let validatedOptions: any; + + const createCommandsServiceInjector = (options: any): IInjector => { + const testInjector = new Yok(); + testInjector.register("errors", { + beginCommand: async (action: () => Promise) => action(), + failWithHelp: (message: string) => { + throw new Error(message); + }, + fail: (message: string) => { + throw new Error(message); + }, + }); + testInjector.register("hooksService", HooksServiceStub); + testInjector.register("logger", LoggerStub); + testInjector.register("staticConfig", { + disableAnalytics: true, + disableCommandHooks: true, + }); + testInjector.register("extensibilityService", {}); + testInjector.register("optionsTracker", {}); + testInjector.register("options", { + ...options, + validateOptions: (dashedOptions: any) => { + validatedOptions = dashedOptions; + }, + }); + testInjector.register("commandsService", CommandsService); + return testInjector; + }; + + beforeEach(() => { + validatedOptions = undefined; + }); + + it("validates the declared options and runs the command", async () => { + const testInjector = createCommandsServiceInjector({ verbose: true }); + let ran: any; + + registerCommandDefinition( + defineCommand({ + name: "dctest-e2e", + options: { verbose: booleanOption({ default: false }) }, + arguments: "any", + run: (context) => { + ran = context; + }, + }), + testInjector, + ); + + const commandsService: ICommandsService = + testInjector.resolve("commandsService"); + await commandsService.tryExecuteCommand("dctest-e2e", ["alpha"]); + + assert.deepEqual(validatedOptions, { + verbose: { type: "boolean", hasSensitiveValue: false, default: false }, + }); + assert.deepEqual(ran.args, ["alpha"]); + assert.deepEqual(ran.options, { verbose: true }); + }); + + it("lets the framework reject parameters when arguments are 'none'", async () => { + const testInjector = createCommandsServiceInjector({}); + let ran = false; + + registerCommandDefinition( + defineCommand({ + name: "dctest-e2e-none", + run: () => { + ran = true; + }, + }), + testInjector, + ); + + const commandsService: ICommandsService = + testInjector.resolve("commandsService"); + await assert.isRejected( + commandsService.tryExecuteCommand("dctest-e2e-none", ["stray"]), + /doesn't accept parameters/, + ); + assert.isFalse(ran); + }); + }); +}); From 15d95d26730aaeeaa8eaf9ee4c52c776996ccb62 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 29 Jul 2026 22:07:33 -0300 Subject: [PATCH 2/8] fix(commands): don't resolve the options service for option-less definitions A definition with no declared options must be executable in a container that has no options service registered - manifest-loaded extension commands run in exactly that situation. --- lib/common/services/command-definition-adapter.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/common/services/command-definition-adapter.ts b/lib/common/services/command-definition-adapter.ts index 0956aa1db7..c56577775f 100644 --- a/lib/common/services/command-definition-adapter.ts +++ b/lib/common/services/command-definition-adapter.ts @@ -65,12 +65,16 @@ export function createCommandFromDefinition< // Resolved per call: the options service only holds parsed values once // validateOptions has run for this command. const buildContext = (args: string[]): ICommandContext => { - const optionsService: any = targetInjector.resolve("options"); const options: any = {}; - for (const optionName of optionNames) { - options[optionName] = optionsService - ? optionsService[optionName] - : undefined; + // Only a definition that declares options may depend on the options + // service being registered - a bare command must work without one. + if (optionNames.length) { + const optionsService: any = targetInjector.resolve("options"); + for (const optionName of optionNames) { + options[optionName] = optionsService + ? optionsService[optionName] + : undefined; + } } return { args, options }; From 7ee7b34764fb2ae63cf14ff13e7d0bb387e00722 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 29 Jul 2026 22:39:13 -0300 Subject: [PATCH 3/8] test(commands): register hierarchical definitions on a local injector The parent-dispatcher leak onto the module-level injector is fixed in the base branch, so the round-trip test no longer needs the global facade. --- test/define-command.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/test/define-command.ts b/test/define-command.ts index 011ff59f31..da866802d2 100644 --- a/test/define-command.ts +++ b/test/define-command.ts @@ -1,5 +1,5 @@ import { assert } from "chai"; -import { Yok, injector as globalInjector } from "../lib/common/yok"; +import { Yok } from "../lib/common/yok"; import { IInjector } from "../lib/common/definitions/yok"; import { inject } from "../lib/common/di"; import { CommandsService } from "../lib/common/services/commands-service"; @@ -54,19 +54,18 @@ describe("defineCommand", () => { run: () => undefined, }); - // The parent dispatcher is synthesized onto the global facade by the - // legacy registry, so registration happens there too. - registerCommandDefinition(definition, globalInjector); + const testInjector = createTestInjector(); + registerCommandDefinition(definition, testInjector); - const command = globalInjector.resolveCommand("dctestwidget|add"); + const command = testInjector.resolveCommand("dctestwidget|add"); assert.isFunction(command.execute); assert.deepEqual(command.allowedParameters, []); - const parent = globalInjector.resolveCommand("dctestwidget"); + const parent = testInjector.resolveCommand("dctestwidget"); assert.isTrue(parent.isHierarchicalCommand); assert.include( - globalInjector.getRegisteredCommandsNames(false), + testInjector.getRegisteredCommandsNames(false), "dctestwidget|add", ); }); From 1c6f26013d10021ce9462e7ad13c3c924e83842d Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 29 Jul 2026 22:44:15 -0300 Subject: [PATCH 4/8] refactor(commands): use the typed di bridge on IInjector --- lib/common/services/command-definition-adapter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/common/services/command-definition-adapter.ts b/lib/common/services/command-definition-adapter.ts index c56577775f..5b1c3da997 100644 --- a/lib/common/services/command-definition-adapter.ts +++ b/lib/common/services/command-definition-adapter.ts @@ -86,7 +86,7 @@ export function createCommandFromDefinition< execute: async (args: string[]): Promise => { // runInInjectionContext is synchronous, so inject() is available while // run() executes up to its first await, and not after it. - await runInInjectionContext((targetInjector).di, () => + await runInInjectionContext(targetInjector.di, () => definition.run(buildContext(args)), ); }, From e005adf73147bbf15240deef24d764f8f7f2113e Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 29 Jul 2026 23:27:00 -0300 Subject: [PATCH 5/8] refactor(commands): the target injector IS the injection context Yok extends Injector on the base branch; the di bridge is gone. --- lib/common/services/command-definition-adapter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/common/services/command-definition-adapter.ts b/lib/common/services/command-definition-adapter.ts index 5b1c3da997..e92e5a88c8 100644 --- a/lib/common/services/command-definition-adapter.ts +++ b/lib/common/services/command-definition-adapter.ts @@ -86,7 +86,7 @@ export function createCommandFromDefinition< execute: async (args: string[]): Promise => { // runInInjectionContext is synchronous, so inject() is available while // run() executes up to its first await, and not after it. - await runInInjectionContext(targetInjector.di, () => + await runInInjectionContext(targetInjector, () => definition.run(buildContext(args)), ); }, From bca21ffd699042742b98d12c32f9b5df184f7ea9 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 5 Aug 2026 15:50:01 -0300 Subject: [PATCH 6/8] feat(commands): validate definitions at define time and compose the argument policy Reworks the declarative command API after the design review: - the new public types drop the `I` prefix, and `defineCommand` returns a `DefinedCommand` branded with the marker `isCommandDefinition` narrows to. `registerCommandDefinition` requires that brand, so nothing reaches the registry without having been validated. - an option is `T` only when its spec declares a `default`; without one it is `T | undefined`, which is what the command line actually produces. Asserted by test/type-fixtures, compiled under strict mode because this build has strictNullChecks off. - `defineCommand` validates the definition and throws naming the command and the accepted form, instead of failing deep and unattributed later. - `arguments` is enforced before the definition's `canExecute` runs, so the two compose: a command that leaves `arguments` at "none" rejects stray positional arguments whether or not it refines further. - `canExecute` runs in an injection context, like `run`. - registration goes through the `CommandRegistry` facet the target injector provides rather than the injector itself. - a schema entry shadowing a CLI-wide option warns naming the collision. --- lib/common/define-command.ts | 318 ++++++++-- .../services/command-definition-adapter.ts | 203 ++++--- lib/contracts/index.ts | 10 +- test/define-command.ts | 542 ++++++++++++++++-- test/type-fixtures/define-command-types.ts | 82 +++ test/type-fixtures/tsconfig.json | 13 + tsconfig.json | 6 +- 7 files changed, 1002 insertions(+), 172 deletions(-) create mode 100644 test/type-fixtures/define-command-types.ts create mode 100644 test/type-fixtures/tsconfig.json diff --git a/lib/common/define-command.ts b/lib/common/define-command.ts index b197709ca9..53327119e6 100644 --- a/lib/common/define-command.ts +++ b/lib/common/define-command.ts @@ -8,15 +8,16 @@ /** * Symbol.for so that a definition produced by one copy of the CLI is still - * recognised by another — extensions bundle their own node_modules. + * recognised by another — extensions bundle their own node_modules. `unique + * symbol` so the marker can also be spelled in the branded return type. */ -export const COMMAND_DEFINITION_MARKER = Symbol.for( +export const COMMAND_DEFINITION_MARKER: unique symbol = Symbol.for( "nativescript:cli:commandDefinition", ); export type CommandOptionType = "boolean" | "string" | "number" | "array"; -export interface ICommandOptionSpec { +export interface CommandOptionSpec { type: CommandOptionType; /** Value used when the flag is absent from the command line. */ default?: TValue; @@ -24,81 +25,318 @@ export interface ICommandOptionSpec { alias?: string | string[]; /** Keeps the value out of analytics and logs. Defaults to false. */ hasSensitiveValue?: boolean; + /** Reserved for generated help; nothing renders it yet. */ description?: string; } +/** + * A spec whose `default` is required. The required property is what + * `CommandOptionValues` keys off to drop `| undefined` from the value type, so + * it may not be relaxed to an optional one. + */ +export interface DefaultedCommandOptionSpec< + TValue = any, +> extends CommandOptionSpec { + default: TValue; +} + /** The parts of an option spec a caller supplies; `type` comes from the helper. */ export type CommandOptionSpecInit = Omit< - ICommandOptionSpec, + CommandOptionSpec, "type" >; -export interface ICommandOptionsSchema { - [optionName: string]: ICommandOptionSpec; +export interface CommandOptionsSchema { + [optionName: string]: CommandOptionSpec; } -export type CommandOptionValues = { - [K in keyof TSchema]: TSchema[K] extends ICommandOptionSpec - ? TValue +/** + * An option the command line omitted is absent at runtime, so only a spec that + * declares a `default` yields a value that is always there. + */ +type CommandOptionValue = + TSpec extends CommandOptionSpec + ? TSpec extends { default: any } + ? TValue + : TValue | undefined : any; + +export type CommandOptionValues = { + [K in keyof TSchema]: CommandOptionValue; }; -export interface ICommandContext< - TSchema extends ICommandOptionsSchema = ICommandOptionsSchema, -> { +export interface CommandContext { /** Positional arguments, after the command name has been consumed. */ args: string[]; /** Current value of every option declared in the schema, and nothing else. */ options: CommandOptionValues; } -export interface ICommandDefinition< - TSchema extends ICommandOptionsSchema = ICommandOptionsSchema, -> { +export interface CommandDefinition { /** `"widget|add"`; `|` separates hierarchy levels. Several names alias one command. */ name: string | string[]; description?: string; options?: TSchema; /** * `"none"` (the default) rejects positional arguments; `"any"` accepts them. - * Anything finer belongs in `canExecute`. + * Anything finer belongs in `canExecute`, which runs after this policy. */ arguments?: "none" | "any"; - canExecute?(context: ICommandContext): Promise | boolean; + canExecute?(context: CommandContext): Promise | boolean; disableAnalytics?: boolean; enableHooks?: boolean; - run(context: ICommandContext): Promise | void; + run(context: CommandContext): Promise | void; +} + +/** + * What `defineCommand` returns: a definition carrying the marker in its type, + * so `registerCommandDefinition` can require a definition that went through + * define-time validation rather than any object of the right shape. + */ +export type DefinedCommand = + CommandDefinition & { + readonly [COMMAND_DEFINITION_MARKER]: true; + }; + +interface IOptionHelper { + ( + init: CommandOptionSpecInit & { default: TValue }, + ): DefaultedCommandOptionSpec; + (init?: CommandOptionSpecInit): CommandOptionSpec; } -const optionSpec = ( - type: CommandOptionType, - init: CommandOptionSpecInit, -): ICommandOptionSpec => ({ ...init, type }); +const optionHelper = (type: CommandOptionType): IOptionHelper => + >((init: CommandOptionSpecInit = {}) => ({ + ...init, + type, + })); + +export const booleanOption = optionHelper("boolean"); +export const stringOption = optionHelper("string"); +export const numberOption = optionHelper("number"); +export const arrayOption = optionHelper("array"); + +const DEFINITION_FIELDS = [ + "name", + "description", + "options", + "arguments", + "canExecute", + "disableAnalytics", + "enableHooks", + "run", +]; + +const OPTION_SPEC_FIELDS = [ + "type", + "default", + "alias", + "hasSensitiveValue", + "description", +]; + +const OPTION_TYPES: CommandOptionType[] = [ + "boolean", + "string", + "number", + "array", +]; + +const ACCEPTED_FORM = + 'defineCommand({ name: "widget|add", run(ctx) { ... } }) — with the ' + + "optional fields description, options, arguments, canExecute, " + + "disableAnalytics and enableHooks."; + +const describeDefinition = (definition: any): string => { + const name = definition && definition.name; + if (typeof name === "string" && name.length) { + return `'${name}'`; + } + + if (Array.isArray(name) && typeof name[0] === "string" && name[0].length) { + return `'${name[0]}'`; + } + + return "an unnamed command"; +}; + +const invalid = (definition: any, problem: string): never => { + throw new Error( + `Invalid command definition for ${describeDefinition(definition)}: ` + + `${problem}. Accepted form: ${ACCEPTED_FORM}`, + ); +}; + +const isPlainObject = (value: any): boolean => + !!value && typeof value === "object" && !Array.isArray(value); + +const validateName = (definition: any): void => { + const name = definition.name; + const isUsableName = (value: any) => + typeof value === "string" && value.trim().length > 0; + + if (isUsableName(name)) { + return; + } + + if (Array.isArray(name) && name.length && name.every(isUsableName)) { + return; + } + + invalid( + definition, + "'name' must be a non-empty string, or an array of non-empty strings for a command with aliases", + ); +}; + +const validateOptionSpec = ( + definition: any, + optionName: string, + spec: any, +): void => { + if (!isPlainObject(spec)) { + invalid( + definition, + `option '${optionName}' must be declared with one of booleanOption(), stringOption(), numberOption() or arrayOption()`, + ); + } + + if (OPTION_TYPES.indexOf(spec.type) === -1) { + invalid( + definition, + `option '${optionName}' has type '${spec.type}'; the supported types are ${OPTION_TYPES.join( + ", ", + )} — declare it with one of booleanOption(), stringOption(), numberOption() or arrayOption()`, + ); + } + + const unknownFields = Object.keys(spec).filter( + (field) => OPTION_SPEC_FIELDS.indexOf(field) === -1, + ); + if (unknownFields.length) { + invalid( + definition, + `option '${optionName}' has unknown field(s) ${unknownFields + .map((field) => `'${field}'`) + .join(", ")}; an option spec accepts ${OPTION_SPEC_FIELDS.join(", ")}`, + ); + } + + const aliasIsUsable = + spec.alias === undefined || + typeof spec.alias === "string" || + (Array.isArray(spec.alias) && + spec.alias.length > 0 && + spec.alias.every((entry: any) => typeof entry === "string")); + if (!aliasIsUsable) { + invalid( + definition, + `option '${optionName}' declares an 'alias' that is neither a string nor a non-empty array of strings`, + ); + } -export const booleanOption = ( - init: CommandOptionSpecInit = {}, -): ICommandOptionSpec => optionSpec("boolean", init); + if ( + spec.hasSensitiveValue !== undefined && + typeof spec.hasSensitiveValue !== "boolean" + ) { + invalid( + definition, + `option '${optionName}' declares a non-boolean 'hasSensitiveValue'`, + ); + } -export const stringOption = ( - init: CommandOptionSpecInit = {}, -): ICommandOptionSpec => optionSpec("string", init); + if (spec.description !== undefined && typeof spec.description !== "string") { + invalid( + definition, + `option '${optionName}' declares a non-string 'description'`, + ); + } +}; + +const validateDefinition = (definition: any): void => { + if (!isPlainObject(definition)) { + invalid(definition, "expected an object"); + } + + const unknownFields = Object.keys(definition).filter( + (field) => DEFINITION_FIELDS.indexOf(field) === -1, + ); + if (unknownFields.length) { + invalid( + definition, + `unknown field(s) ${unknownFields + .map((field) => `'${field}'`) + .join(", ")}; a definition accepts ${DEFINITION_FIELDS.join(", ")}`, + ); + } + + validateName(definition); + + if (typeof definition.run !== "function") { + invalid(definition, "'run' must be a function"); + } + + if ( + definition.arguments !== undefined && + definition.arguments !== "none" && + definition.arguments !== "any" + ) { + invalid( + definition, + `'arguments' is '${definition.arguments}'; it must be "none" or "any"`, + ); + } -export const numberOption = ( - init: CommandOptionSpecInit = {}, -): ICommandOptionSpec => optionSpec("number", init); + if ( + definition.canExecute !== undefined && + typeof definition.canExecute !== "function" + ) { + invalid(definition, "'canExecute' must be a function"); + } + + for (const flag of ["disableAnalytics", "enableHooks"]) { + if ( + definition[flag] !== undefined && + typeof definition[flag] !== "boolean" + ) { + invalid(definition, `'${flag}' must be a boolean`); + } + } + + if (definition.description !== undefined) { + if (typeof definition.description !== "string") { + invalid(definition, "'description' must be a string"); + } + } + + if (definition.options !== undefined) { + if (!isPlainObject(definition.options)) { + invalid( + definition, + "'options' must be an object keyed by the long option name", + ); + } + + for (const optionName of Object.keys(definition.options)) { + validateOptionSpec( + definition, + optionName, + definition.options[optionName], + ); + } + } +}; -export const arrayOption = ( - init: CommandOptionSpecInit = {}, -): ICommandOptionSpec => optionSpec("array", init); +export function defineCommand( + definition: CommandDefinition, +): DefinedCommand { + validateDefinition(definition); -export function defineCommand( - definition: ICommandDefinition, -): ICommandDefinition { - const marked: ICommandDefinition = { ...definition }; - (marked)[COMMAND_DEFINITION_MARKER] = true; + const marked: any = { ...definition }; + marked[COMMAND_DEFINITION_MARKER] = true; return marked; } -export function isCommandDefinition(value: any): value is ICommandDefinition { +export function isCommandDefinition(value: any): value is DefinedCommand { return !!value && (value)[COMMAND_DEFINITION_MARKER] === true; } diff --git a/lib/common/services/command-definition-adapter.ts b/lib/common/services/command-definition-adapter.ts index e92e5a88c8..e26fa5009f 100644 --- a/lib/common/services/command-definition-adapter.ts +++ b/lib/common/services/command-definition-adapter.ts @@ -4,11 +4,14 @@ import { runInInjectionContext } from "../di/inject"; import { IDictionary, IDashedOption } from "../declarations"; import { IInjector } from "../definitions/yok"; import { ICommand } from "../definitions/commands"; +import { CommandRegistry } from "../contracts/command-registry"; import { + CommandContext, + CommandDefinition, CommandOptionType, - ICommandContext, - ICommandDefinition, - ICommandOptionsSchema, + CommandOptionsSchema, + DefinedCommand, + isCommandDefinition, } from "../define-command"; const OPTION_TYPES: IDictionary = { @@ -18,29 +21,12 @@ const OPTION_TYPES: IDictionary = { array: OptionType.Array, }; -/** - * Wraps a declarative definition in the ICommand shape the legacy registry and - * CommandsService expect. - * - * Constraint worth knowing before changing the canExecute mapping: the moment - * an ICommand exposes `canExecute`, CommandsService returns its verdict and - * skips `allowedParameters` validation entirely. So `canExecute` is emitted - * only when the definition supplies one or opts into `arguments: "any"` — the - * adapter then owns argument validation wholesale. With `arguments: "none"` - * and no user `canExecute` the property is omitted, which lets the framework's - * own empty-`allowedParameters` check reject stray positional arguments. - */ -export function createCommandFromDefinition< - TSchema extends ICommandOptionsSchema, ->( - definition: ICommandDefinition, - targetInjector: IInjector = injector, -): ICommand { - const schema = definition.options || {}; - const optionNames = Object.keys(schema); - +const compileOptions = ( + schema: CommandOptionsSchema, +): IDictionary => { const dashedOptions: IDictionary = {}; - for (const optionName of optionNames) { + + for (const optionName of Object.keys(schema)) { const spec = schema[optionName]; const dashedOption: IDashedOption = { type: OPTION_TYPES[spec.type], @@ -62,63 +48,150 @@ export function createCommandFromDefinition< dashedOptions[optionName] = dashedOption; } - // Resolved per call: the options service only holds parsed values once - // validateOptions has run for this command. - const buildContext = (args: string[]): ICommandContext => { + return dashedOptions; +}; + +/** + * A command option that shadows a CLI-wide one wins the re-parse for this + * command only, so the two spellings mean different things depending on what + * the user typed first. Warned rather than rejected while the policy is open. + */ +const warnOnCliOptionCollisions = ( + targetInjector: IInjector, + definition: CommandDefinition, + optionNames: string[], + optionsService: any, +): void => { + const cliOptions = optionsService && optionsService.options; + if (!cliOptions) { + return; + } + + const collisions = optionNames.filter((optionName) => cliOptions[optionName]); + if (!collisions.length) { + return; + } + + const logger = targetInjector.get("logger", { optional: true }); + if (!logger) { + return; + } + + const commandName = Array.isArray(definition.name) + ? definition.name[0] + : definition.name; + logger.warn( + `Command '${commandName}' declares option(s) ${collisions + .map((name) => `'--${name}'`) + .join(", ")} that the CLI already defines globally. The command's ` + + `declaration wins while the command runs; rename them to avoid it.`, + ); +}; + +/** + * Wraps a declarative definition in the ICommand shape the legacy registry and + * CommandsService expect. + * + * The compiled command always exposes `canExecute`, because CommandsService + * skips `allowedParameters` entirely once it is present: the adapter enforces + * the declared `arguments` policy itself and only then consults the + * definition's own `canExecute`, so the two fields compose. + */ +export function createCommandFromDefinition< + TSchema extends CommandOptionsSchema, +>( + definition: CommandDefinition, + targetInjector: IInjector = injector, +): ICommand { + const schema = definition.options || {}; + const optionNames = Object.keys(schema); + const dashedOptions = compileOptions(schema); + + // Only a definition that declares options may depend on the options service + // being registered - a bare command must work without one. + const optionsService: any = optionNames.length + ? targetInjector.resolve("options") + : null; + + warnOnCliOptionCollisions( + targetInjector, + definition, + optionNames, + optionsService, + ); + + // Read per call rather than snapshotted here: the options service only holds + // this command's parsed values once validateOptions has run for it. + const buildContext = (args: string[]): CommandContext => { const options: any = {}; - // Only a definition that declares options may depend on the options - // service being registered - a bare command must work without one. - if (optionNames.length) { - const optionsService: any = targetInjector.resolve("options"); - for (const optionName of optionNames) { - options[optionName] = optionsService - ? optionsService[optionName] - : undefined; - } + for (const optionName of optionNames) { + options[optionName] = optionsService[optionName]; } return { args, options }; }; - const command: ICommand = { + const acceptsArguments = definition.arguments === "any"; + + return { allowedParameters: [], dashedOptions, + ...(definition.disableAnalytics === undefined + ? {} + : { disableAnalytics: definition.disableAnalytics }), + ...(definition.enableHooks === undefined + ? {} + : { enableHooks: definition.enableHooks }), + canExecute: async (args: string[]): Promise => { + if (!acceptsArguments && args.length) { + targetInjector + .resolve("errors") + .failWithHelp("This command doesn't accept parameters."); + return false; + } + + const refine = definition.canExecute; + if (!refine) { + return true; + } + + // Same first-await rule as execute: runInInjectionContext is + // synchronous, so inject() is available up to the first await. + return await runInInjectionContext(targetInjector, () => + refine.call(definition, buildContext(args)), + ); + }, execute: async (args: string[]): Promise => { - // runInInjectionContext is synchronous, so inject() is available while - // run() executes up to its first await, and not after it. await runInInjectionContext(targetInjector, () => definition.run(buildContext(args)), ); }, }; +} - if (definition.canExecute || definition.arguments === "any") { - command.canExecute = async (args: string[]): Promise => - definition.canExecute - ? await definition.canExecute(buildContext(args)) - : true; +export function registerCommandDefinition( + definition: DefinedCommand, + targetInjector: IInjector = injector, +): void { + if (!isCommandDefinition(definition)) { + throw new Error( + "registerCommandDefinition() takes the result of defineCommand(); " + + "the value passed carries no command-definition marker.", + ); } - if (definition.disableAnalytics !== undefined) { - command.disableAnalytics = definition.disableAnalytics; - } + // The registry facet rather than the injector itself, so a child injector + // that provides its own CommandRegistry receives the registration. + const registry = targetInjector.get(CommandRegistry); + const names = Array.isArray(definition.name) + ? definition.name + : [definition.name]; - if (definition.enableHooks !== undefined) { - command.enableHooks = definition.enableHooks; + for (const name of names) { + // A prototype-less zero-parameter function registers as a useFactory + // provider, so the command is built on first resolution and cached. + registry.registerCommand(name, () => + createCommandFromDefinition(definition, targetInjector), + ); } - - return command; -} - -export function registerCommandDefinition< - TSchema extends ICommandOptionsSchema, ->( - definition: ICommandDefinition, - targetInjector: IInjector = injector, -): void { - // An arrow function resolver is invoked as a factory rather than new-ed, and - // its result is cached as the command instance. - targetInjector.registerCommand(definition.name, () => - createCommandFromDefinition(definition, targetInjector), - ); } diff --git a/lib/contracts/index.ts b/lib/contracts/index.ts index efbdc504f6..0120e5301a 100644 --- a/lib/contracts/index.ts +++ b/lib/contracts/index.ts @@ -35,10 +35,12 @@ export { arrayOption, } from "../common/define-command"; export type { - ICommandDefinition, - ICommandContext, - ICommandOptionSpec, - ICommandOptionsSchema, + CommandDefinition, + DefinedCommand, + CommandContext, + CommandOptionSpec, + DefaultedCommandOptionSpec, + CommandOptionsSchema, CommandOptionSpecInit, CommandOptionType, CommandOptionValues, diff --git a/test/define-command.ts b/test/define-command.ts index da866802d2..9bcf16a5b3 100644 --- a/test/define-command.ts +++ b/test/define-command.ts @@ -1,8 +1,13 @@ import { assert } from "chai"; +import { spawnSync } from "child_process"; +import * as path from "path"; import { Yok } from "../lib/common/yok"; import { IInjector } from "../lib/common/definitions/yok"; import { inject } from "../lib/common/di"; +import { CommandRegistry } from "../lib/common/contracts/command-registry"; import { CommandsService } from "../lib/common/services/commands-service"; +import { Options } from "../lib/options"; +import { Errors } from "../lib/common/errors"; import { LoggerStub, HooksServiceStub } from "./stubs"; import { arrayOption, @@ -17,17 +22,15 @@ import { registerCommandDefinition, } from "../lib/common/services/command-definition-adapter"; -type IsExact = - (() => T extends A ? 1 : 2) extends () => T extends B ? 1 : 2 - ? true - : false; - -/** Fails to compile unless the schema inferred exactly the expected type. */ -const expectExactType = (): void => undefined; - const createTestInjector = (options: any = {}): IInjector => { const testInjector = new Yok(); testInjector.register("options", options); + testInjector.register("logger", LoggerStub); + testInjector.register("errors", { + failWithHelp: (message: string) => { + throw new Error(message); + }, + }); return testInjector; }; @@ -35,7 +38,7 @@ describe("defineCommand", () => { it("marks definitions so a duplicated CLI copy still recognises them", () => { const definition = defineCommand({ name: "dctest-marker", - run: () => undefined, + run: (): void => undefined, }); assert.isTrue(isCommandDefinition(definition)); @@ -46,12 +49,195 @@ describe("defineCommand", () => { assert.isFalse(isCommandDefinition(null)); }); + it("keeps the marker on a spread-derived copy", () => { + const derived = { + ...defineCommand({ name: "dctest-spread", run: (): void => undefined }), + name: "dctest-spread-derived", + }; + + assert.isTrue(isCommandDefinition(derived)); + }); + + describe("define-time validation", () => { + const rejects = (definition: any, expected: RegExp) => + assert.throws(() => defineCommand(definition), expected); + + it("names the command and the accepted form in every message", () => { + rejects( + { name: "dctest-bad", run: 42 }, + /Invalid command definition for 'dctest-bad'.*'run' must be a function.*Accepted form: defineCommand/s, + ); + }); + + it("rejects a missing or unusable name", () => { + rejects( + { run: (): void => undefined }, + /an unnamed command.*'name' must be/s, + ); + rejects({ name: "", run: (): void => undefined }, /'name' must be/); + rejects({ name: [], run: (): void => undefined }, /'name' must be/); + rejects( + { name: ["ok", ""], run: (): void => undefined }, + /'name' must be/, + ); + rejects({ name: 7, run: (): void => undefined }, /'name' must be/); + }); + + it("rejects a missing run", () => { + rejects({ name: "dctest-norun" }, /'run' must be a function/); + }); + + it("rejects a typo'd definition field", () => { + rejects( + { + name: "dctest-typo", + handler: (): void => undefined, + run: (): void => undefined, + }, + /unknown field\(s\) 'handler'/, + ); + }); + + it("rejects an unusable arguments policy", () => { + rejects( + { name: "dctest-args", arguments: "one", run: (): void => undefined }, + /'arguments' is 'one'; it must be "none" or "any"/, + ); + }); + + it("rejects a non-function canExecute and non-boolean flags", () => { + rejects( + { name: "dctest-can", canExecute: true, run: (): void => undefined }, + /'canExecute' must be a function/, + ); + rejects( + { + name: "dctest-flag", + disableAnalytics: "yes", + run: (): void => undefined, + }, + /'disableAnalytics' must be a boolean/, + ); + rejects( + { name: "dctest-flag2", enableHooks: 1, run: (): void => undefined }, + /'enableHooks' must be a boolean/, + ); + }); + + it("rejects an option with an unsupported type", () => { + rejects( + { + name: "dctest-opt", + options: { verbose: { type: "bool" } }, + run: (): void => undefined, + }, + /option 'verbose' has type 'bool'; the supported types are boolean, string, number, array/, + ); + }); + + it("rejects an option that is not a spec at all", () => { + rejects( + { + name: "dctest-opt2", + options: { verbose: true }, + run: (): void => undefined, + }, + /option 'verbose' must be declared with one of booleanOption/, + ); + }); + + it("rejects a typo'd option-spec field", () => { + rejects( + { + name: "dctest-opt3", + options: { verbose: { type: "boolean", describe: "no" } }, + run: (): void => undefined, + }, + /option 'verbose' has unknown field\(s\) 'describe'/, + ); + }); + + it("rejects unusable alias, hasSensitiveValue and description entries", () => { + rejects( + { + name: "dctest-opt4", + options: { verbose: { type: "boolean", alias: 1 } }, + run: (): void => undefined, + }, + /option 'verbose' declares an 'alias'/, + ); + rejects( + { + name: "dctest-opt5", + options: { verbose: { type: "boolean", hasSensitiveValue: "yes" } }, + run: (): void => undefined, + }, + /non-boolean 'hasSensitiveValue'/, + ); + rejects( + { + name: "dctest-opt6", + options: { verbose: { type: "boolean", description: 5 } }, + run: (): void => undefined, + }, + /non-string 'description'/, + ); + }); + + it("accepts every documented field", () => { + assert.doesNotThrow(() => + defineCommand({ + name: ["dctest-full", "dctest-full-alias"], + description: "Everything at once", + options: { + verbose: booleanOption({ default: false }), + output: stringOption({ alias: ["o", "out"], description: "Dir" }), + retries: numberOption({ default: 1 }), + files: arrayOption({ hasSensitiveValue: true }), + }, + arguments: "any", + canExecute: () => true, + disableAnalytics: true, + enableHooks: false, + run: (): void => undefined, + }), + ); + }); + }); + + describe("option value types", () => { + it("types default-less options as possibly undefined", () => { + // The repo builds without strictNullChecks, which erases the very + // `| undefined` under test, so the assertions live in their own + // strict project. + const project = path.join( + __dirname, + "..", + "..", + "test", + "type-fixtures", + "tsconfig.json", + ); + const result = spawnSync( + process.execPath, + [require.resolve("typescript/bin/tsc"), "-p", project], + { encoding: "utf8" }, + ); + + assert.strictEqual( + result.status, + 0, + `${result.stdout || ""}${result.stderr || ""}`, + ); + }); + }); + describe("registration", () => { it("round-trips through the legacy command registry", () => { const definition = defineCommand({ name: "dctestwidget|add", description: "Adds a widget", - run: () => undefined, + run: (): void => undefined, }); const testInjector = createTestInjector(); @@ -73,7 +259,7 @@ describe("defineCommand", () => { it("caches one command instance per registered name", () => { const testInjector = createTestInjector(); registerCommandDefinition( - defineCommand({ name: "dctestflat", run: () => undefined }), + defineCommand({ name: "dctestflat", run: (): void => undefined }), testInjector, ); @@ -88,7 +274,7 @@ describe("defineCommand", () => { registerCommandDefinition( defineCommand({ name: ["dctestalias", "dctestalias2"], - run: () => undefined, + run: (): void => undefined, }), testInjector, ); @@ -96,6 +282,62 @@ describe("defineCommand", () => { assert.isFunction(testInjector.resolveCommand("dctestalias").execute); assert.isFunction(testInjector.resolveCommand("dctestalias2").execute); }); + + it("refuses a value that did not come from defineCommand", () => { + assert.throws( + () => + registerCommandDefinition( + { name: "dctestraw", run: (): void => undefined }, + createTestInjector(), + ), + /carries no command-definition marker/, + ); + }); + + it("registers through the CommandRegistry the target injector provides", () => { + const testInjector = createTestInjector(); + const registered: string[] = []; + testInjector.register({ + provide: CommandRegistry, + useValue: { + registerCommand: (name: string) => registered.push(name), + }, + }); + + registerCommandDefinition( + defineCommand({ + name: ["dctestfacet", "dctestfacet2"], + run: (): void => undefined, + }), + testInjector, + ); + + assert.deepEqual(registered, ["dctestfacet", "dctestfacet2"]); + assert.isNull(testInjector.resolveCommand("dctestfacet")); + }); + + it("keeps a registered command when a subcommand would shadow it", () => { + const testInjector = createTestInjector(); + registerCommandDefinition( + defineCommand({ name: "dctestowned", run: (): void => undefined }), + testInjector, + ); + + registerCommandDefinition( + defineCommand({ name: "dctestowned|sub", run: (): void => undefined }), + testInjector, + ); + + const owner = testInjector.resolveCommand("dctestowned"); + assert.isUndefined(owner.isHierarchicalCommand); + assert.isFunction(testInjector.resolveCommand("dctestowned|sub").execute); + + const logger: LoggerStub = testInjector.resolve("logger"); + assert.match( + logger.warnOutput, + /'dctestowned' is already registered as a command of its own.*'dctestowned\|sub' cannot be reached/, + ); + }); }); describe("execute", () => { @@ -176,7 +418,7 @@ describe("defineCommand", () => { assert.isTrue(finished); }); - it("infers the option value types on the run context", async () => { + it("carries the declared option values onto the run context", async () => { const testInjector = createTestInjector({ verbose: true, output: "dist", @@ -184,11 +426,7 @@ describe("defineCommand", () => { files: ["a.ts"], }); - let verbose: boolean; - let output: string; - let retries: number; - let files: string[]; - + let seen: any; const command = createCommandFromDefinition( defineCommand({ name: "dctesttypes", @@ -199,15 +437,7 @@ describe("defineCommand", () => { files: arrayOption(), }, run: (context) => { - expectExactType>(); - expectExactType>(); - expectExactType>(); - expectExactType>(); - - verbose = context.options.verbose; - output = context.options.output; - retries = context.options.retries; - files = context.options.files; + seen = context.options; }, }), testInjector, @@ -215,10 +445,12 @@ describe("defineCommand", () => { await command.execute([]); - assert.isTrue(verbose); - assert.strictEqual(output, "dist"); - assert.strictEqual(retries, 3); - assert.deepEqual(files, ["a.ts"]); + assert.deepEqual(seen, { + verbose: true, + output: "dist", + retries: 3, + files: ["a.ts"], + }); }); }); @@ -237,7 +469,7 @@ describe("defineCommand", () => { description: "Auth token", }), }, - run: () => undefined, + run: (): void => undefined, }), createTestInjector(), ); @@ -257,35 +489,75 @@ describe("defineCommand", () => { it("is empty when no options are declared", () => { const command = createCommandFromDefinition( - defineCommand({ name: "dctestnoopts", run: () => undefined }), + defineCommand({ name: "dctestnoopts", run: (): void => undefined }), createTestInjector(), ); assert.deepEqual(command.dashedOptions, {}); }); + + it("warns when a declared option shadows a CLI-wide one", () => { + const testInjector = createTestInjector({ + options: { verbose: { type: "boolean" } }, + }); + + createCommandFromDefinition( + defineCommand({ + name: "dctestshadow", + options: { verbose: booleanOption(), fresh: booleanOption() }, + run: (): void => undefined, + }), + testInjector, + ); + + const logger: LoggerStub = testInjector.resolve("logger"); + assert.match( + logger.warnOutput, + /Command 'dctestshadow' declares option\(s\) '--verbose' that the CLI already defines globally/, + ); + assert.notInclude(logger.warnOutput, "--fresh"); + }); }); describe("canExecute", () => { - it("is omitted for argument-less commands so the framework rejects parameters", () => { - const testInjector = createTestInjector(); + it("rejects positional arguments before consulting the definition", async () => { + let refined = false; + const command = createCommandFromDefinition( + defineCommand({ + name: "dctestnone", + canExecute: () => { + refined = true; + return true; + }, + run: (): void => undefined, + }), + createTestInjector(), + ); - const implicit = createCommandFromDefinition( - defineCommand({ name: "dctestnone", run: () => undefined }), - testInjector, + await assert.isRejected( + command.canExecute(["stray"]), + /doesn't accept parameters/, ); - const explicit = createCommandFromDefinition( + assert.isFalse(refined); + assert.isTrue(await command.canExecute([])); + assert.isTrue(refined); + }); + + it("rejects positional arguments with no definition canExecute at all", async () => { + const command = createCommandFromDefinition( defineCommand({ name: "dctestnone2", arguments: "none", - run: () => undefined, + run: (): void => undefined, }), - testInjector, + createTestInjector(), ); - assert.isUndefined(implicit.canExecute); - assert.isUndefined(explicit.canExecute); - assert.deepEqual(implicit.allowedParameters, []); - assert.deepEqual(explicit.allowedParameters, []); + await assert.isRejected( + command.canExecute(["stray"]), + /doesn't accept parameters/, + ); + assert.isTrue(await command.canExecute([])); }); it("accepts anything when arguments are 'any'", async () => { @@ -293,16 +565,15 @@ describe("defineCommand", () => { defineCommand({ name: "dctestany", arguments: "any", - run: () => undefined, + run: (): void => undefined, }), createTestInjector(), ); - assert.isFunction(command.canExecute); assert.isTrue(await command.canExecute(["whatever", "else"])); }); - it("hands the context to a user canExecute and honours its verdict", async () => { + it("hands the context to a definition canExecute and honours its verdict", async () => { const testInjector = createTestInjector({ force: true }); let capturedContext: any; @@ -316,7 +587,7 @@ describe("defineCommand", () => { capturedContext = context; return verdict; }, - run: () => undefined, + run: (): void => undefined, }), testInjector, ); @@ -328,19 +599,21 @@ describe("defineCommand", () => { assert.isFalse(await build(false).canExecute(["android"])); }); - it("is emitted for a user canExecute even when arguments are 'none'", async () => { + it("runs the definition canExecute inside an injection context", async () => { + const testInjector = createTestInjector(); + testInjector.register("dcTestPolicy", { allowed: true }); + const command = createCommandFromDefinition( defineCommand({ - name: "dctestnonecan", - arguments: "none", - canExecute: async () => true, - run: () => undefined, + name: "dctestcaninject", + arguments: "any", + canExecute: () => inject("dcTestPolicy").allowed, + run: (): void => undefined, }), - createTestInjector(), + testInjector, ); - assert.isFunction(command.canExecute); - assert.isTrue(await command.canExecute([])); + assert.isTrue(await command.canExecute(["anything"])); }); }); @@ -351,7 +624,7 @@ describe("defineCommand", () => { name: "dctestflags", disableAnalytics: true, enableHooks: false, - run: () => undefined, + run: (): void => undefined, }), createTestInjector(), ); @@ -362,7 +635,7 @@ describe("defineCommand", () => { it("leaves both absent when the definition omits them", () => { const command = createCommandFromDefinition( - defineCommand({ name: "dctestnoflags", run: () => undefined }), + defineCommand({ name: "dctestnoflags", run: (): void => undefined }), createTestInjector(), ); @@ -371,10 +644,83 @@ describe("defineCommand", () => { }); }); + describe("option validation with the real options service", () => { + interface IValidationRun { + failures: string[]; + options: any; + } + + // The options service parses process.argv in its constructor, so each run + // gets its own injector and its own instance. + const validate = (definition: any, argv: string[]): IValidationRun => { + const failures: string[] = []; + const testInjector = new Yok(); + testInjector.register("staticConfig", { CLIENT_NAME: "" }); + testInjector.register("hostInfo", {}); + testInjector.register("settingsService", { + setSettings: (): any => undefined, + getProfileDir: () => "profileDir", + }); + testInjector.register("logger", LoggerStub); + + const errors = new Errors(testInjector); + errors.failWithHelp = ((message: string) => failures.push(message)); + errors.fail = ((message: string) => failures.push(message)); + testInjector.register("errors", errors); + testInjector.register("options", Options); + + const originalArgv = process.argv; + process.argv = [originalArgv[0], originalArgv[1], ...argv]; + try { + const command = createCommandFromDefinition(definition, testInjector); + const options: any = testInjector.resolve("options"); + options.validateOptions(command.dashedOptions); + return { failures, options }; + } finally { + process.argv = originalArgv; + } + }; + + beforeEach(() => { + process.env.NS_STRICT_OPTIONS = "error"; + }); + + afterEach(() => { + delete process.env.NS_STRICT_OPTIONS; + }); + + it("accepts an option declared with an array of aliases, under any spelling", () => { + const definition = defineCommand({ + name: "dctest-alias", + options: { outputDir: stringOption({ alias: ["o", "out"] }) }, + run: (): void => undefined, + }); + + for (const spelling of ["--output-dir", "--outputDir", "-o", "--out"]) { + const run = validate(definition, [spelling, "dist"]); + assert.deepEqual(run.failures, [], `rejected ${spelling}`); + assert.strictEqual(run.options.outputDir, "dist"); + } + }); + + it("still rejects an option the definition did not declare", () => { + const definition = defineCommand({ + name: "dctest-alias2", + options: { outputDir: stringOption({ alias: ["o", "out"] }) }, + run: (): void => undefined, + }); + + const run = validate(definition, ["--outputdirr", "dist"]); + + assert.lengthOf(run.failures, 1); + assert.match(run.failures[0], /'outputdirr' is not supported/); + }); + }); + describe("end to end through CommandsService", () => { let validatedOptions: any; - const createCommandsServiceInjector = (options: any): IInjector => { + const createCommandsServiceInjector = (options: any = {}): IInjector => { const testInjector = new Yok(); testInjector.register("errors", { beginCommand: async (action: () => Promise) => action(), @@ -434,8 +780,8 @@ describe("defineCommand", () => { assert.deepEqual(ran.options, { verbose: true }); }); - it("lets the framework reject parameters when arguments are 'none'", async () => { - const testInjector = createCommandsServiceInjector({}); + it("rejects parameters when arguments are 'none'", async () => { + const testInjector = createCommandsServiceInjector(); let ran = false; registerCommandDefinition( @@ -456,5 +802,77 @@ describe("defineCommand", () => { ); assert.isFalse(ran); }); + + it("rejects parameters even when the definition supplies a canExecute", async () => { + const testInjector = createCommandsServiceInjector(); + let ran = false; + + registerCommandDefinition( + defineCommand({ + name: "dctest-e2e-refine", + canExecute: () => true, + run: () => { + ran = true; + }, + }), + testInjector, + ); + + const commandsService: ICommandsService = + testInjector.resolve("commandsService"); + await assert.isRejected( + commandsService.tryExecuteCommand("dctest-e2e-refine", ["stray"]), + /doesn't accept parameters/, + ); + assert.isFalse(ran); + }); + + it("dispatches a subcommand through the parent name", async () => { + const testInjector = createCommandsServiceInjector(); + let ran: any; + + registerCommandDefinition( + defineCommand({ + name: "dctest-widget|add", + arguments: "any", + run: (context) => { + ran = context; + }, + }), + testInjector, + ); + + const commandsService: ICommandsService = + testInjector.resolve("commandsService"); + await commandsService.tryExecuteCommand("dctest-widget", [ + "add", + "alpha", + ]); + + assert.deepEqual(ran.args, ["alpha"]); + }); + + it("dispatches the default subcommand, named or bare", async () => { + const testInjector = createCommandsServiceInjector(); + const runs: string[][] = []; + + registerCommandDefinition( + defineCommand({ + name: "dctest-gadget|*all", + arguments: "any", + run: (context) => { + runs.push(context.args); + }, + }), + testInjector, + ); + + const commandsService: ICommandsService = + testInjector.resolve("commandsService"); + await commandsService.tryExecuteCommand("dctest-gadget", ["all", "beta"]); + await commandsService.tryExecuteCommand("dctest-gadget", []); + + assert.deepEqual(runs, [["beta"], []]); + }); }); }); diff --git a/test/type-fixtures/define-command-types.ts b/test/type-fixtures/define-command-types.ts new file mode 100644 index 0000000000..7bb50de7cb --- /dev/null +++ b/test/type-fixtures/define-command-types.ts @@ -0,0 +1,82 @@ +/** + * Type-level assertions for the defineCommand schema, compiled by + * test/define-command.ts through this directory's tsconfig. It is kept out of + * the repo's own build because that build runs without strictNullChecks, which + * erases the `| undefined` these assertions exist to pin — and because the + * @ts-expect-error directives below only hold under strict mode. + */ + +import { + arrayOption, + booleanOption, + defineCommand, + numberOption, + stringOption, +} from "../../lib/common/define-command"; + +type IsExact = + (() => T extends A ? 1 : 2) extends () => T extends B ? 1 : 2 + ? true + : false; + +const expectExactType = (): void => undefined; + +// A declared option is `T` only when the schema supplies a default; without +// one the flag may simply be absent from the command line. +defineCommand({ + name: "typefixture|values", + options: { + verbose: booleanOption(), + release: booleanOption({ default: false }), + output: stringOption({ alias: "o" }), + target: stringOption({ default: "dist" }), + retries: numberOption(), + attempts: numberOption({ default: 3 }), + files: arrayOption(), + tags: arrayOption({ default: [] }), + }, + run(ctx) { + expectExactType>(); + expectExactType>(); + expectExactType>(); + expectExactType>(); + expectExactType>(); + expectExactType>(); + expectExactType>(); + expectExactType>(); + expectExactType>(); + + // @ts-expect-error - the schema types ctx.options and nothing else + ctx.options.undeclared; + }, +}); + +defineCommand({ + name: "typefixture|no-options", + run(ctx) { + expectExactType>(); + + // @ts-expect-error - nothing is declared, so any access is a typo + ctx.options.anything; + }, +}); + +defineCommand({ + name: "typefixture|refine", + options: { force: booleanOption({ default: false }) }, + canExecute(ctx) { + expectExactType>(); + return ctx.args.length === 1; + }, + run: () => undefined, +}); + +// @ts-expect-error - `run` is the required handler field +defineCommand({ name: "typefixture|no-run" }); + +defineCommand({ + name: "typefixture|bad-arguments", + // @ts-expect-error - `arguments` is a closed set + arguments: "one", + run: () => undefined, +}); diff --git a/test/type-fixtures/tsconfig.json b/test/type-fixtures/tsconfig.json new file mode 100644 index 0000000000..254bd447c0 --- /dev/null +++ b/test/type-fixtures/tsconfig.json @@ -0,0 +1,13 @@ +{ + "compilerOptions": { + "target": "ES2018", + "module": "commonjs", + "noEmit": true, + "strict": true, + "skipLibCheck": true, + "noUnusedLocals": false, + "lib": ["ESNext"], + "types": [] + }, + "files": ["define-command-types.ts"] +} diff --git a/tsconfig.json b/tsconfig.json index 1e9b8deda9..45bf605cf7 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -18,5 +18,9 @@ "lib": ["ESNext"], "strict": false }, - "include": ["lib/", "test/"] + // test/type-fixtures has its own strict tsconfig and is compiled by the test + // that asserts on it; building it here would emit a testless file into dist + // and drop the strictness those assertions depend on + "include": ["lib/", "test/"], + "exclude": ["test/type-fixtures/"] } From 816082250836189ae34274c0f1c06a5a23d1aa52 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 5 Aug 2026 15:50:01 -0300 Subject: [PATCH 7/8] docs(commands): match defining-commands.md to the shipped behavior Unknown options warn and only fail under NS_STRICT_OPTIONS=error; `description` reaches the parser but nothing renders it; `canExecute` gets a context of the same shape as run's, not the same one. Replaces the "canExecute owns validation" rule with how the two fields compose, renames the flagship example's option off the CLI-wide `verbose`, and documents option value types, array aliases, the parent-name collision and `satisfies` for shared schemas. --- defining-commands.md | 159 +++++++++++++++++++++++++++++++------------ 1 file changed, 114 insertions(+), 45 deletions(-) diff --git a/defining-commands.md b/defining-commands.md index fa0143123b..cfadf9cd27 100644 --- a/defining-commands.md +++ b/defining-commands.md @@ -26,25 +26,45 @@ export default defineCommand({ name: "widget|add", description: "Adds a widget to the project", options: { - verbose: booleanOption({ default: false }), + overwrite: booleanOption({ default: false }), output: stringOption({ alias: "o" }), }, arguments: "any", async run(ctx) { // ctx.args -> string[] of positional arguments - // ctx.options -> { verbose: boolean; output: string } - if (ctx.options.verbose) { + // ctx.options -> { overwrite: boolean; output: string | undefined } + if (ctx.options.output) { console.log(`adding ${ctx.args.join(", ")} to ${ctx.options.output}`); } }, }); ``` -`defineCommand` returns the definition, tagged with a marker symbol so that any -copy of the CLI can recognise it (`isCommandDefinition(value)` is the exported -predicate). It does not register anything by itself — see +`defineCommand` validates the definition and returns it, tagged with a marker +symbol so that any copy of the CLI can recognise it. `isCommandDefinition(value)` +is the exported check, and it narrows to `DefinedCommand`. The tag survives a +spread, so `{ ...baseDefinition, name: "widget|add2" }` is still recognised. + +`defineCommand` does not register anything by itself — see [Registering a definition](#registering-a-definition). +Validation happens where you can see it +--------------------------------------- + +A definition is checked at the moment `defineCommand` is called, not when the +command eventually runs. A misspelled field, a missing `run`, an option +declared with something other than the four helpers, an `arguments` value +outside `"none" | "any"` — each throws immediately, naming the command and the +accepted form: + +``` +Invalid command definition for 'widget|add': unknown field(s) 'handler'; a +definition accepts name, description, options, arguments, canExecute, +disableAnalytics, enableHooks, run. Accepted form: defineCommand({ name: +"widget|add", run(ctx) { ... } }) — with the optional fields description, +options, arguments, canExecute, disableAnalytics and enableHooks. +``` + Names and the command hierarchy ------------------------------- @@ -62,19 +82,29 @@ runs both for `ns widget add` and for a bare `ns widget`. This is the convention the CLI's own commands use (`run|*all`, `debug|*all`); the encoding is user-visible because it feeds shell autocompletion and generated help. +A parent name cannot also be a command of its own. If `widget` is already +registered as a flat command, registering `widget|add` leaves that command in +place, warns naming both, and creates no dispatcher — so `ns widget add` will +not route until one of the two is renamed. + Options ------- -`options` is a schema keyed by the long option name — `verbose` is passed as -`--verbose`. Declare each entry with one of the four helpers, which fix the +`options` is a schema keyed by the long option name — `output` is passed as +`--output`. Declare each entry with one of the four helpers, which fix the value type: -| Helper | Value type on `ctx.options` | -| --------------- | --------------------------- | -| `booleanOption` | `boolean` | -| `stringOption` | `string` | -| `numberOption` | `number` | -| `arrayOption` | `string[]` | +| Helper | Declared with `default` | Declared without | +| --------------- | ----------------------- | ----------------------- | +| `booleanOption` | `boolean` | `boolean \| undefined` | +| `stringOption` | `string` | `string \| undefined` | +| `numberOption` | `number` | `number \| undefined` | +| `arrayOption` | `string[]` | `string[] \| undefined` | + +The two columns are the whole story of the option types: a flag the user did +not pass is absent at runtime, so only a `default` makes the value on +`ctx.options` always present. Declare a default whenever there is a sensible +one and the `| undefined` disappears from the type. Each helper takes an optional spec: @@ -94,17 +124,39 @@ options: { ``` - `default` — value used when the flag is absent. -- `alias` — single-dash shorthand, or an array of them. +- `alias` — single-dash shorthand, or an array of them (`alias: ["o", "out"]`). - `hasSensitiveValue` — defaults to `false`; set it for anything that must not be recorded. There is no reason not to be explicit about credentials, paths containing user directories, and tokens. -- `description` — shown in generated help. +- `description` — reserved for generated help. It reaches the option parser but + nothing renders it yet. The schema types `ctx.options` and nothing else: `ctx.options` carries exactly -the declared keys, with the types the helpers imply. Values that the CLI parses +the declared keys, and a typo is a compile error. Values that the CLI parses globally (`--path`, `--log`, …) are not exposed there; resolve the `options` service if you need them. +### Sharing a schema between commands + +Extract the schema with `satisfies` rather than a type annotation. An +annotation widens every entry back to the general spec type and the `default` +information — and with it the non-optional value types — is lost: + +```ts +const buildOptions = { + release: booleanOption({ default: false }), + output: stringOption({ alias: "o" }), +} satisfies CommandOptionsSchema; +``` + +### Do not shadow a CLI-wide option + +`--verbose`, `--path`, `--log`, `--release`, `--env` and friends are declared by +the CLI itself. Declaring one of those names in a command's schema makes the +command's declaration win for the duration of that command, which means the +same flag means different things depending on which command is running. The CLI +warns at registration naming the collision; pick another name. + ### How validation behaves Option validation is the CLI's existing behaviour, not something the definition @@ -113,11 +165,17 @@ declared options and the command line is re-parsed: - Declared options are accepted and appear on `ctx.options`. - An option the CLI does not know — neither global nor declared by this command - — is a **hard error**: the command does not run, and the CLI prints - `The option '' is not supported.` followed by a help suggestion. - -So adding an option is exactly a matter of adding a schema entry; forgetting to -declare one that users pass is a failure, not a silent `undefined`. + — produces a warning: `The option '' is not supported. This will become +an error in a future release.` The command still runs. Set + `NS_STRICT_OPTIONS=error` to preview the hard failure, which is what a future + release will do by default. +- The same staging applies to value-shape violations: a string option passed + with no value, an array option passed nothing, a single-valued option passed + twice. + +So adding an option is a matter of adding a schema entry; forgetting to declare +one that users pass is a warning today and a failure later, never a silent +`undefined`. Positional arguments -------------------- @@ -131,7 +189,7 @@ Positional arguments Anything finer than that belongs in `canExecute`. -### `canExecute` owns validation +### `canExecute` refines, it does not replace ```ts defineCommand({ @@ -146,18 +204,20 @@ defineCommand({ }); ``` -`canExecute` receives the same context as `run` and returns a boolean (or a -promise of one). Returning `false` aborts the command and prints a help -suggestion; throwing surfaces your own error message, which is usually the -friendlier choice. +The two fields compose. The declared `arguments` policy is enforced first, and +`canExecute` is consulted only for command lines that already satisfy it — so a +definition that leaves `arguments` at `"none"` still rejects stray positional +arguments even when it supplies a `canExecute`, and a `canExecute` that only +inspects options cannot accidentally widen what the command accepts. + +`canExecute` receives a context of the same shape as `run`'s — the same +`args` and the same declared options, built freshly for the call — and returns +a boolean (or a promise of one). Returning `false` aborts the command and +prints a help suggestion; throwing surfaces your own error message, which is +usually the friendlier choice. -There is one rule to internalise: **the moment a command supplies -`canExecute`, it owns argument validation completely.** The framework returns -that verdict and skips every built-in parameter check, including the -"no parameters" rule implied by `arguments: "none"`. A definition with a -`canExecute` that only inspects options will therefore accept stray positional -arguments unless it checks `ctx.args` itself. If you do not need custom -validation, omit `canExecute` and let `arguments` do the work. +`canExecute` runs inside a dependency-injection context, on the same terms as +`run`: `inject()` is valid up to the first `await`. The run context --------------- @@ -170,7 +230,9 @@ The run context the command executes. `run` may be synchronous or `async`; the CLI awaits the result and treats a -rejection as a command failure. +rejection as a command failure. Throwing is how a definition fails a command; +`$errors.failWithHelp` from the injected `errors` service adds the help +suggestion. `run` starts inside a dependency-injection context, so `inject()` works directly: @@ -215,9 +277,12 @@ import addWidgetCommand from "./add-widget"; registerCommandDefinition(addWidgetCommand); ``` -It registers the definition under every name it declares, on the CLI's injector -by default; pass a second argument to target a different injector (tests do -this). The command instance is created on first resolution and cached. +It takes a `DefinedCommand` — the result of `defineCommand`, marker and all — +and rejects a bare object of the right shape, so a definition can never reach +the registry without having been validated. It registers under every name the +definition declares, through the `CommandRegistry` the target injector provides; +pass a second argument to target a different injector (tests do this). The +command instance is built by a factory on first resolution and cached. `registerCommandDefinition` lives in `lib/common/services/command-definition-adapter` rather than in @@ -237,13 +302,17 @@ A definition is compiled into an ordinary `ICommand`, so nothing downstream — the registry, the router, hooks, help, analytics — knows the difference. The mapping is: -| Definition | `ICommand` | -| --------------------------------- | ------------------------------------------ | -| `options` | `dashedOptions` | -| `run` | `execute`, wrapped in an injection context | -| `arguments`, `canExecute` | `canExecute` (see the rule above) | -| — | `allowedParameters`, always `[]` | -| `disableAnalytics`, `enableHooks` | passed through unchanged | +| Definition | `ICommand` | +| --------------------------------- | -------------------------------------------------- | +| `options` | `dashedOptions` | +| `run` | `execute`, wrapped in an injection context | +| `arguments`, `canExecute` | `canExecute`: policy enforced, then the refinement | +| — | `allowedParameters`, always `[]` | +| `disableAnalytics`, `enableHooks` | passed through unchanged | + +The compiled command always exposes `canExecute`, because `CommandsService` +stops consulting `allowedParameters` as soon as a command has one — the adapter +therefore enforces the `arguments` policy itself. Existing command classes need no migration. Reach for a definition when a command is mostly "parse these flags and do this"; a class still makes sense From d712a4e545f51d360a711a514871d0319c3fd3c0 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 5 Aug 2026 16:18:14 -0300 Subject: [PATCH 8/8] feat(commands): add ctx.fail and warn on colliding option aliases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ctx.fail(message)` is the failure verb on the command context, in both `run` and `canExecute`. It maps to the errors service's `failWithHelp`, so a command failure carries the usage suggestion, and returns `never` so it can end a branch without a return. The message is validated like the define-time errors are, naming the command. Throwing keeps working unchanged — fail() is sugar over it, not a replacement. Commands get no `skip()`: warn-and-continue has no meaning inside run(). The CLI-wide option collision warning now covers aliases on both sides, so an `alias: "p"` that shadows `--path`'s shorthand is reported the same way a `verbose` option shadowing `--verbose` is, naming both sides. --- defining-commands.md | 47 +++++-- lib/common/define-command.ts | 2 + .../services/command-definition-adapter.ts | 74 ++++++++--- test/define-command.ts | 117 +++++++++++++++++- test/type-fixtures/define-command-types.ts | 2 + 5 files changed, 209 insertions(+), 33 deletions(-) diff --git a/defining-commands.md b/defining-commands.md index cfadf9cd27..b6f589cf9a 100644 --- a/defining-commands.md +++ b/defining-commands.md @@ -155,7 +155,11 @@ const buildOptions = { the CLI itself. Declaring one of those names in a command's schema makes the command's declaration win for the duration of that command, which means the same flag means different things depending on which command is running. The CLI -warns at registration naming the collision; pick another name. +warns at registration naming both sides of the collision; pick another name. + +Aliases count too, in both directions: an `alias: "p"` collides with `--path`'s +shorthand just as `output: stringOption()` would collide with a CLI-wide +`--output`. ### How validation behaves @@ -211,10 +215,10 @@ arguments even when it supplies a `canExecute`, and a `canExecute` that only inspects options cannot accidentally widen what the command accepts. `canExecute` receives a context of the same shape as `run`'s — the same -`args` and the same declared options, built freshly for the call — and returns -a boolean (or a promise of one). Returning `false` aborts the command and -prints a help suggestion; throwing surfaces your own error message, which is -usually the friendlier choice. +`args`, the same declared options and the same `fail` — built freshly for the +call, and returns a boolean (or a promise of one). Returning `false` aborts the +command and prints a bare help suggestion; `ctx.fail(message)` aborts it with +your own message, which is usually the friendlier choice. `canExecute` runs inside a dependency-injection context, on the same terms as `run`: `inject()` is valid up to the first `await`. @@ -228,11 +232,38 @@ The run context (including any subcommand segments) has been consumed. - `ctx.options` — the current value of each declared option, read at the moment the command executes. +- `ctx.fail(message)` — fails the command with `message` and a usage help + suggestion. `run` may be synchronous or `async`; the CLI awaits the result and treats a -rejection as a command failure. Throwing is how a definition fails a command; -`$errors.failWithHelp` from the injected `errors` service adds the help -suggestion. +rejection as a command failure. + +### Failing a command + +`ctx.fail(message)` is the idiomatic way to stop a command: + +```ts +defineCommand({ + name: "widget|add", + arguments: "any", + options: { output: stringOption() }, + async run(ctx) { + if (!ctx.options.output) { + ctx.fail("--output is required."); + } + + /* ... */ + }, +}); +``` + +It is available on the `canExecute` context as well, and it returns `never`, so +it can end a branch without a `return`. The message must be a non-empty string. + +Throwing is equivalent and keeps working — `ctx.fail` is sugar over the +`errors` service's `failWithHelp`, which is what adds the "Run `ns widget add +--help`" line. Throw when you already have an `Error` to propagate; call +`ctx.fail` when you are writing the message. `run` starts inside a dependency-injection context, so `inject()` works directly: diff --git a/lib/common/define-command.ts b/lib/common/define-command.ts index 53327119e6..c5cf28e5fe 100644 --- a/lib/common/define-command.ts +++ b/lib/common/define-command.ts @@ -70,6 +70,8 @@ export interface CommandContext { args: string[]; /** Current value of every option declared in the schema, and nothing else. */ options: CommandOptionValues; + /** Fails the command with `message` and the usage help suggestion. */ + fail(message: string): never; } export interface CommandDefinition { diff --git a/lib/common/services/command-definition-adapter.ts b/lib/common/services/command-definition-adapter.ts index e26fa5009f..92c85429b1 100644 --- a/lib/common/services/command-definition-adapter.ts +++ b/lib/common/services/command-definition-adapter.ts @@ -1,7 +1,7 @@ import { OptionType } from "../enums"; import { injector } from "../yok"; import { runInInjectionContext } from "../di/inject"; -import { IDictionary, IDashedOption } from "../declarations"; +import { IDictionary, IDashedOption, IErrors } from "../declarations"; import { IInjector } from "../definitions/yok"; import { ICommand } from "../definitions/commands"; import { CommandRegistry } from "../contracts/command-registry"; @@ -51,15 +51,18 @@ const compileOptions = ( return dashedOptions; }; +const aliasList = (alias: string | string[]): string[] => + alias === undefined ? [] : Array.isArray(alias) ? alias : [alias]; + /** * A command option that shadows a CLI-wide one wins the re-parse for this - * command only, so the two spellings mean different things depending on what - * the user typed first. Warned rather than rejected while the policy is open. + * command only, so the same spelling means different things depending on which + * command is running. Warned rather than rejected while the policy is open. */ const warnOnCliOptionCollisions = ( targetInjector: IInjector, definition: CommandDefinition, - optionNames: string[], + schema: CommandOptionsSchema, optionsService: any, ): void => { const cliOptions = optionsService && optionsService.options; @@ -67,7 +70,32 @@ const warnOnCliOptionCollisions = ( return; } - const collisions = optionNames.filter((optionName) => cliOptions[optionName]); + // Every spelling the CLI already answers to, mapped to the option owning it. + const cliSpellings: IDictionary = {}; + for (const cliName of Object.keys(cliOptions)) { + cliSpellings[cliName] = cliName; + for (const alias of aliasList(cliOptions[cliName].alias)) { + cliSpellings[alias] = cliName; + } + } + + const collisions: string[] = []; + for (const optionName of Object.keys(schema)) { + if (cliSpellings[optionName]) { + collisions.push( + `'--${optionName}' with the CLI option '--${cliSpellings[optionName]}'`, + ); + } + + for (const alias of aliasList(schema[optionName].alias)) { + if (cliSpellings[alias]) { + collisions.push( + `alias '-${alias}' of '--${optionName}' with the CLI option '--${cliSpellings[alias]}'`, + ); + } + } + } + if (!collisions.length) { return; } @@ -81,10 +109,9 @@ const warnOnCliOptionCollisions = ( ? definition.name[0] : definition.name; logger.warn( - `Command '${commandName}' declares option(s) ${collisions - .map((name) => `'--${name}'`) - .join(", ")} that the CLI already defines globally. The command's ` + - `declaration wins while the command runs; rename them to avoid it.`, + `Command '${commandName}' declares options that collide with CLI-wide ` + + `ones: ${collisions.join("; ")}. The command's declaration wins while ` + + `the command runs; rename them to avoid it.`, ); }; @@ -113,12 +140,22 @@ export function createCommandFromDefinition< ? targetInjector.resolve("options") : null; - warnOnCliOptionCollisions( - targetInjector, - definition, - optionNames, - optionsService, - ); + warnOnCliOptionCollisions(targetInjector, definition, schema, optionsService); + + const commandName = Array.isArray(definition.name) + ? definition.name[0] + : definition.name; + + const fail = (message: string): never => { + if (typeof message !== "string" || !message.trim()) { + throw new Error( + `ctx.fail() for command '${commandName}' requires a non-empty message.`, + ); + } + + const errors: IErrors = targetInjector.resolve("errors"); + return errors.failWithHelp(message); + }; // Read per call rather than snapshotted here: the options service only holds // this command's parsed values once validateOptions has run for it. @@ -128,7 +165,7 @@ export function createCommandFromDefinition< options[optionName] = optionsService[optionName]; } - return { args, options }; + return { args, options, fail }; }; const acceptsArguments = definition.arguments === "any"; @@ -144,10 +181,7 @@ export function createCommandFromDefinition< : { enableHooks: definition.enableHooks }), canExecute: async (args: string[]): Promise => { if (!acceptsArguments && args.length) { - targetInjector - .resolve("errors") - .failWithHelp("This command doesn't accept parameters."); - return false; + fail("This command doesn't accept parameters."); } const refine = definition.canExecute; diff --git a/test/define-command.ts b/test/define-command.ts index 9bcf16a5b3..f249669849 100644 --- a/test/define-command.ts +++ b/test/define-command.ts @@ -496,26 +496,58 @@ describe("defineCommand", () => { assert.deepEqual(command.dashedOptions, {}); }); - it("warns when a declared option shadows a CLI-wide one", () => { + it("warns when a declared option or alias shadows a CLI-wide one", () => { const testInjector = createTestInjector({ - options: { verbose: { type: "boolean" } }, + options: { + verbose: { type: "boolean" }, + path: { type: "string", alias: "p" }, + }, }); createCommandFromDefinition( defineCommand({ name: "dctestshadow", - options: { verbose: booleanOption(), fresh: booleanOption() }, + options: { + verbose: booleanOption(), + output: stringOption({ alias: ["p", "o"] }), + fresh: booleanOption({ alias: "f" }), + }, run: (): void => undefined, }), testInjector, ); const logger: LoggerStub = testInjector.resolve("logger"); - assert.match( + assert.include( + logger.warnOutput, + "'--verbose' with the CLI option '--verbose'", + ); + assert.include( logger.warnOutput, - /Command 'dctestshadow' declares option\(s\) '--verbose' that the CLI already defines globally/, + "alias '-p' of '--output' with the CLI option '--path'", ); assert.notInclude(logger.warnOutput, "--fresh"); + assert.notInclude(logger.warnOutput, "'-o'"); + }); + + it("stays quiet when nothing collides", () => { + const testInjector = createTestInjector({ + options: { path: { type: "string", alias: "p" } }, + }); + + createCommandFromDefinition( + defineCommand({ + name: "dctestnoshadow", + options: { output: stringOption({ alias: ["o", "out"] }) }, + run: (): void => undefined, + }), + testInjector, + ); + + assert.strictEqual( + (testInjector.resolve("logger")).warnOutput, + "", + ); }); }); @@ -617,6 +649,81 @@ describe("defineCommand", () => { }); }); + describe("ctx.fail", () => { + const createFailInjector = (): IInjector => { + const testInjector = createTestInjector(); + testInjector.register("errors", { + failWithHelp: (message: string) => { + throw new Error(`with help: ${message}`); + }, + }); + return testInjector; + }; + + it("fails the command from run, through failWithHelp", async () => { + const command = createCommandFromDefinition( + defineCommand({ + name: "dctestfailrun", + run: (ctx) => ctx.fail("no project found"), + }), + createFailInjector(), + ); + + await assert.isRejected( + command.execute([]), + /with help: no project found/, + ); + }); + + it("fails the command from canExecute, through failWithHelp", async () => { + const command = createCommandFromDefinition( + defineCommand({ + name: "dctestfailcan", + arguments: "any", + canExecute: (ctx) => + ctx.args.length === 1 || ctx.fail("expected one argument"), + run: (): void => undefined, + }), + createFailInjector(), + ); + + assert.isTrue(await command.canExecute(["one"])); + await assert.isRejected( + command.canExecute([]), + /with help: expected one argument/, + ); + }); + + it("rejects a message that carries nothing", async () => { + const command = createCommandFromDefinition( + defineCommand({ + name: "dctestfailempty", + run: (ctx) => ctx.fail(" "), + }), + createFailInjector(), + ); + + await assert.isRejected( + command.execute([]), + /ctx.fail\(\) for command 'dctestfailempty' requires a non-empty message/, + ); + }); + + it("still lets a thrown error through unchanged", async () => { + const command = createCommandFromDefinition( + defineCommand({ + name: "dctestthrow", + run: () => { + throw new Error("raw failure"); + }, + }), + createFailInjector(), + ); + + await assert.isRejected(command.execute([]), /^raw failure$/); + }); + }); + describe("command flags", () => { it("passes disableAnalytics and enableHooks through", () => { const command = createCommandFromDefinition( diff --git a/test/type-fixtures/define-command-types.ts b/test/type-fixtures/define-command-types.ts index 7bb50de7cb..e8d1f80d49 100644 --- a/test/type-fixtures/define-command-types.ts +++ b/test/type-fixtures/define-command-types.ts @@ -55,6 +55,8 @@ defineCommand({ name: "typefixture|no-options", run(ctx) { expectExactType>(); + // `never` is what lets fail() end a branch without a return. + expectExactType, never>>(); // @ts-expect-error - nothing is declared, so any access is a typo ctx.options.anything;