refactor(commands): drop the free runCommand and canExecuteCommand helpers

The two convenience functions resolved the dispatcher from the current
injection context and otherwise from the CLI's root injector. After a
handler's first await, or inside a stdin handler, there is no context,
so they silently dispatched through the root and ignored the injector
the caller actually held: a per-command provider, an extension's own
scope, or a test injector.

CommandsService is a service like any other and follows the same rule:
inject() before the first await, the injector after it. The shortcut
actions dispatch through the injector their context carries, and embed
injects the contract for its prepare precondition.

Author: edusperoni

defining-commands.md

@@ -482,8 +482,8 @@ Sharing is either of two things, and neither of them is a bag:
   }
   ```
 
-- **A whole command's precondition** — `canExecuteCommand(name, args)`, which
-  asks that command itself; see [Asking another
+- **A whole command's precondition** — `CommandsService.canExecuteCommand`,
+  which asks that command itself; see [Asking another
   command](#asking-another-command).
 
 ### `setup`, when a command has one
@@ -846,21 +846,23 @@ Running a command in process
 ----------------------------
 
 The `CommandsService` contract dispatches a registered command from inside the
-process that is already running. A class command injects it like any other
-service; an inline handler or a key shortcut may use the `runCommand`
-convenience, which only resolves the contract from the current context:
+process that is already running. It is a service like any other, so it follows
+the rule every service does: `inject()` before the first `await`, the
+injector after it, and a key shortcut's action reaches it through the
+injector its context carries:
 
 ```ts
-import { CommandsService } from "../common/contracts/commands-service";
-import { runCommand } from "../common/services/command-definition-adapter";
+import { CommandsService } from "nativescript/contracts";
 
 // in a class command
 private $commandsService = inject(CommandsService);
 await this.$commandsService.runCommand("autocomplete");
 
-// in an inline handler or a shortcut action
-await runCommand("open|ios");
-await runCommand("install", ["lodash"]);
+// in an inline handler, after the first await
+await ctx.injector.get(CommandsService).runCommand("install", ["lodash"]);
+
+// in a shortcut action
+action: (ctx) => ctx.injector.get(CommandsService).runCommand("open|ios"),
 ```
 
 The command gets what a typed command line gives it, in the same order: its
@@ -887,10 +889,11 @@ declarations into it rewrites the values the host process is still running on
 — `open|ios` declares `watch: false`, which would otherwise leave an `ns start`
 out of watch mode for the rest of its life.
 
-Which injector `runCommand` dispatches through follows the rule
-`registerCommand` does: the injector of the current injection context, and the
-CLI's own outside one. The pipeline itself lives on the contract, so a plugin
-that holds an injector can call `CommandsService.runCommand` directly.
+There is deliberately no free `runCommand()` function: one that silently fell
+back to the CLI's root injector outside an injection context would dispatch
+through the wrong scope from exactly the places — after an `await`, inside a
+stdin handler — where the mistake is hardest to notice. The injector you hold
+is the one to dispatch through.
 
 ### Asking another command
 
@@ -903,15 +906,21 @@ given, whether or not it is registered, so `runCommand(prepareCommandDefinition)
 runs exactly what you hold and cannot go stale the way a string can. Its first
 name still identifies it for hooks and reporting.
 
-`CommandsService.canExecuteCommand(command, args)` — or the
-`canExecuteCommand` convenience — asks a registered command whether it *could*
-run, without running it:
+`CommandsService.canExecuteCommand(command, args)` asks a registered command
+whether it *could* run, without running it:
 
 ```ts
-import { canExecuteCommand } from "../common/services/command-definition-adapter";
+import { CommandsService } from "nativescript/contracts";
+
+private $commandsService = inject(CommandsService);
 
 async canExecute(): Promise<boolean> {
-	if (!(await canExecuteCommand("prepare", [this.args[0]]))) {
+	if (
+		!(await this.$commandsService.canExecuteCommand(
+			prepareCommandDefinition,
+			[this.args[0]],
+		))
+	) {
 		return false;
 	}
 
@@ -931,10 +940,10 @@ Pass only the arguments the child's own `arguments` policy accepts. The child
 enforces that policy before its `canExecute`, so forwarding a caller's whole
 argument list to a child that declares fewer is a rejection, not a wider check.
 
-`canExecuteCommand` is a thin call onto `CommandsService.canExecuteCommand`
-(which the deprecated `canExecuteCommandInProcess` also calls), and follows
-`runCommand` in everything else: the same injector rule, the same option priming and
-restoration.
+`canExecuteCommand` follows `runCommand` in everything else: the same option
+priming and restoration, the same routing of a parent name to its subcommand.
+The deprecated `canExecuteCommandInProcess` and `executeCommandInProcess`
+call the two methods with a name.
 
 ### Key shortcuts
 
@@ -947,7 +956,7 @@ an `action` that runs it:
 	key: "I",
 	description: "Open project in Xcode",
 	when: onPlatform("iOS"),
-	action: () => runCommand("open|ios"),
+	action: (ctx) => ctx.injector.get(CommandsService).runCommand("open|ios"),
 }
 ```
 

lib/commands/embedding/embed.ts

@@ -5,7 +5,7 @@ import { IProjectConfigService, IProjectData } from "../../definitions/project";
 import { Command } from "../../common/define-command";
 import { IFileSystem } from "../../common/declarations";
 import { inject } from "../../common/di";
-import { canExecuteCommand } from "../../common/services/command-definition-adapter";
+import { CommandsService } from "../../common/contracts/commands-service";
 import { platformArgument } from "../command-base";
 import {
 	prepareCommandDefinition,
@@ -35,6 +35,7 @@ export class EmbedCommand extends Command({
 		{ name: "hostProjectModuleName" },
 	],
 }) {
+	private $commandsService = inject(CommandsService);
 	private $fs = inject<IFileSystem>("fs");
 	private $logger = inject<ILogger>("logger");
 	private $options = inject<IOptions>("options");
@@ -57,7 +58,7 @@ export class EmbedCommand extends Command({
 		// `prepare` takes the platform alone; the host project arguments are this
 		// command's own and it would reject them.
 		if (
-			!(await canExecuteCommand(
+			!(await this.$commandsService.canExecuteCommand(
 				prepareCommandDefinition,
 				this.args.slice(0, 1),
 			))

lib/common/services/command-definition-adapter.ts

@@ -31,7 +31,6 @@ import {
 	CommandOptionSpec,
 	CommandOptionType,
 	CommandOptionsSchema,
-	CommandReference,
 	DefinedCommand,
 	RegisterableCommand,
 	defineCommand,
@@ -572,31 +571,6 @@ const namesOf = (definition: DefinedCommand<any, any, any>): string[] =>
 const contextInjector = (): Injector =>
 	getCurrentInjector() || <Injector>(<any>getRootInjector());
 
-/**
- * Convenience over `CommandsService.runCommand` for code that has no injected
- * service at hand, such as a key shortcut action or an inline handler; the
- * contract is the API, this only resolves it from the current context.
- */
-export async function runCommand(
-	command: CommandReference,
-	args: string[] = [],
-): Promise<void> {
-	await contextInjector().get(CommandsService).runCommand(command, args);
-}
-
-/**
- * Convenience over `CommandsService.canExecuteCommand`, resolved from the
- * current context the way `runCommand` is.
- */
-export async function canExecuteCommand(
-	command: CommandReference,
-	args: string[] = [],
-): Promise<boolean> {
-	return contextInjector()
-		.get(CommandsService)
-		.canExecuteCommand(command, args);
-}
-
 /**
  * Registers a command with the CLI. Takes a Command() class, the result of
  * defineCommand(), or a bare definition, which it defines on the caller's

lib/services/key-shortcuts.ts

@@ -13,7 +13,7 @@ import {
 	KeyShortcutRegistration,
 	KeyShortcutRegistry,
 } from "../common/contracts/key-shortcuts";
-import { runCommand } from "../common/services/command-definition-adapter";
+import { CommandsService } from "../common/contracts/commands-service";
 import { injector } from "../common/yok";
 import { IProjectDataService } from "../definitions/project";
 import { IStartService } from "../definitions/start-service";
@@ -318,7 +318,10 @@ export function openIdeShortcut(
 		description,
 		group: platform,
 		when: onPlatform(platform),
-		action: () => runCommand(`open|${platform.toLowerCase()}`),
+		action: (ctx) =>
+			ctx.injector
+				.get(CommandsService)
+				.runCommand(`open|${platform.toLowerCase()}`),
 	};
 }
 
@@ -367,7 +370,7 @@ export function keyShortcuts(): KeyShortcut<NsKeyContext>[] {
 			key: "n",
 			description: "Install dependencies",
 			group: WORKFLOW_GROUP,
-			action: () => runCommand("install"),
+			action: (ctx) => ctx.injector.get(CommandsService).runCommand("install"),
 		},
 	];
 }

test/define-command.ts

@@ -30,7 +30,6 @@ import {
 	stringOption,
 } from "../lib/common/define-command";
 import {
-	canExecuteCommand,
 	createCommandFromDefinition,
 	registerBuiltInCommand,
 	registerCommand,
@@ -1511,12 +1510,12 @@ describe("defineCommand", () => {
 			});
 
 			const verdicts = [
-				await runInInjectionContext(testInjector, () =>
-					canExecuteCommand("dctest-can-yes", ["ok"]),
-				),
-				await runInInjectionContext(testInjector, () =>
-					canExecuteCommand("dctest-can-yes", ["nope"]),
-				),
+				await testInjector
+					.resolve("commandsService")
+					.canExecuteCommand("dctest-can-yes", ["ok"]),
+				await testInjector
+					.resolve("commandsService")
+					.canExecuteCommand("dctest-can-yes", ["nope"]),
 			];
 
 			assert.deepEqual(verdicts, [true, false]);
@@ -1616,9 +1615,9 @@ describe("defineCommand", () => {
 			);
 
 			await assert.isRejected(
-				runInInjectionContext(testInjector, () =>
-					canExecuteCommand("dctest-can-none", ["stray"]),
-				),
+				testInjector
+					.resolve("commandsService")
+					.canExecuteCommand("dctest-can-none", ["stray"]),
 				/doesn't accept parameters/,
 			);
 			assert.isFalse(consulted);
@@ -1642,19 +1641,19 @@ describe("defineCommand", () => {
 			);
 
 			assert.isTrue(
-				await runInInjectionContext(testInjector, () =>
-					canExecuteCommand("dctest-can-setup"),
-				),
+				await testInjector
+					.resolve("commandsService")
+					.canExecuteCommand("dctest-can-setup"),
 			);
 		});
 
 		it("fails by name for a command that is not registered", async () => {
 			const testInjector = createInProcessInjector();
 
 			await assert.isRejected(
-				runInInjectionContext(testInjector, () =>
-					canExecuteCommand("dctest-can-missing"),
-				),
+				testInjector
+					.resolve("commandsService")
+					.canExecuteCommand("dctest-can-missing"),
 				/Unknown command 'dctest-can-missing'/,
 			);
 		});

test/services/key-shortcuts.ts

@@ -2,9 +2,7 @@ import { assert } from "chai";
 import { EventEmitter } from "events";
 import { RunOnDeviceEvents } from "../../lib/constants";
 import { getContractName } from "../../lib/common/di/contract";
-import { runInInjectionContext } from "../../lib/common/di/inject";
 import { Injector } from "../../lib/common/di/injector";
-import { runCommand } from "../../lib/common/services/command-definition-adapter";
 import { KeyShortcutRegistryService } from "../../lib/services/key-shortcut-registry";
 import {
 	findShortcut,
@@ -213,7 +211,7 @@ describe("key shortcuts", () => {
 			]);
 		});
 
-		it("routes the IDE shortcuts through the open commands", async () => {
+		it("routes the IDE shortcuts through the context's commands service", async () => {
 			const invoked: string[] = [];
 			const dispatcher = fakeInjector(
 				new Map<any, any>([
@@ -226,13 +224,11 @@ describe("key shortcuts", () => {
 					],
 				]),
 			);
-			const ctx = context();
+			const ctx = context({ injector: dispatcher });
 			const resolved = resolveShortcuts(keyShortcuts(), ctx);
 
 			for (const key of ["A", "I", "V", "n"]) {
-				await runInInjectionContext(dispatcher, () =>
-					findShortcut(resolved, key).action(ctx),
-				);
+				await findShortcut(resolved, key).action(ctx);
 			}
 
 			assert.deepEqual(invoked, [
@@ -952,57 +948,6 @@ describe("key shortcuts", () => {
 			);
 		});
 	});
-
-	describe("runCommand", () => {
-		it("dispatches through the commands service of the current context", async () => {
-			const dispatched: { name: string; args: string[] }[] = [];
-			const dispatcher = fakeInjector(
-				new Map<any, any>([
-					[
-						"commandsService",
-						{
-							runCommand: async (name: string, args: string[]): Promise<void> =>
-								void dispatched.push({ name, args }),
-						},
-					],
-				]),
-			);
-
-			await runInInjectionContext(dispatcher, () =>
-				runCommand("open|ios", ["--verbose"]),
-			);
-			await runInInjectionContext(dispatcher, () => runCommand("install"));
-
-			assert.deepEqual(dispatched, [
-				{ name: "open|ios", args: ["--verbose"] },
-				{ name: "install", args: [] },
-			]);
-		});
-
-		it("lets a failure reach the caller", async () => {
-			const dispatcher = fakeInjector(
-				new Map<any, any>([
-					[
-						"commandsService",
-						{
-							runCommand: async (): Promise<void> => {
-								throw new Error("Unable to execute command 'open ios'.");
-							},
-						},
-					],
-				]),
-			);
-
-			let raised: Error = null;
-			try {
-				await runInInjectionContext(dispatcher, () => runCommand("open|ios"));
-			} catch (err) {
-				raised = err;
-			}
-
-			assert.equal(raised.message, "Unable to execute command 'open ios'.");
-		});
-	});
 });
 
 function noop(): void {