diff --git a/Cargo.lock b/Cargo.lock index 5e954b3..219b62f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,4 +4,64 @@ version = 4 [[package]] name = "cmdkit" -version = "0.2.0" +version = "0.3.0" +dependencies = [ + "futures-channel", + "futures-executor", +] + +[[package]] +name = "futures-channel" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "07bbe89c50d7a535e539b8c17bc0b49bdb77747034daa8087407d655f3f7cc1d" +dependencies = [ + "futures-core", +] + +[[package]] +name = "futures-core" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e3450815272ef58cec6d564423f6e755e25379b217b0bc688e295ba24df6b1d" + +[[package]] +name = "futures-executor" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "baf29c38818342a3b26b5b923639e7b1f4a61fc5e76102d4b1981c6dc7a7579d" +dependencies = [ + "futures-core", + "futures-task", + "futures-util", +] + +[[package]] +name = "futures-task" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "037711b3d59c33004d3856fbdc83b99d4ff37a24768fa1be9ce3538a1cde4393" + +[[package]] +name = "futures-util" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "389ca41296e6190b48053de0321d02a77f32f8a5d2461dd38762c0593805c6d6" +dependencies = [ + "futures-core", + "futures-task", + "pin-project-lite", + "slab", +] + +[[package]] +name = "pin-project-lite" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" + +[[package]] +name = "slab" +version = "0.4.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c790de23124f9ab44544d7ac05d60440adc586479ce501c1d6d7da3cd8c9cf5" diff --git a/Cargo.toml b/Cargo.toml index 494db72..2deb021 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,13 +1,19 @@ [package] name = "cmdkit" -version = "0.2.0" +version = "0.3.0" edition = "2024" -description = "Core library for CLI tools, providing common functionality and utilities for building command-line applications." +description = "Deterministic command execution runtime for structured command graphs with configurable execution topology." license = "Apache-2.0" repository = "https://github.com/JackLanger/CMDkit" documentation = "https://docs.rs/cmdkit" -keywords = ["cli", "command", "framework", "parser"] -categories = ["command-line-interface", "command-line-utilities"] +keywords = [ + "runtime", + "dispatch", + "string-based", + "deterministic", + "orchestration", +] +categories = ["asynchronous", "concurrency", "development-tools"] readme = "README.md" [[bin]] @@ -16,3 +22,7 @@ path = "src/bin/wrapper_probe.rs" [dependencies] +futures-channel = "0.3" + +[dev-dependencies] +futures-executor = "0.3" diff --git a/README.md b/README.md index 5266c14..5359122 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # CMDkit -CMDkit is a small, implementation-first Rust framework for building command-line tools. +CMDkit is a deterministic command-execution runtime that separates command definition from invocation parsing and execution orchestration, while enabling full runtime configuration during setup. It is designed around three ideas: @@ -19,7 +19,7 @@ cargo add cmdkit ## Highlights - Register commands with `Command::new(...)` or fluent `command(...).build()`. -- Attach handlers as structs (`CommandStrategy`) or closures (`handler_fn` / `Command::from_fn`). +- Attach handlers as structs (`CommandStrategy`) or closures (`handler_fn` / `handler_fn_with_context` / `Command::from_fn` / `Command::from_fn_with_context`), all using `(&ExecutionContext, InvocationArgs)`. - Compose nested command hierarchies with subcommands. - Parse command input into three channels: - `options: Vec` for switch/flag inputs @@ -32,29 +32,29 @@ cargo add cmdkit ### Runtime -- `CliCore::new()` creates a runtime with default configuration. -- `CliCore::builder()` starts a fluent builder for registering commands before building the runtime. -- `CliCore::create(config)` uses custom `CoreConfig`. +- `CMDKit::new()` creates a runtime with default configuration. +- `CMDKit::builder()` starts a fluent builder for registering commands before building the runtime. +- `CMDKit::create(config)` uses custom `CoreConfig`. - `register`, `get`, and `get_all` manage command registration on a runtime instance. - `try_run_from_args(&[String])` is ideal for tests and embedding. - `run_with_commands` and `try_run_with_commands` are convenience wrappers. -Each `CliCore` instance owns its own registry. Runtime state is not shared across instances. +Each `CMDKit` instance owns its own registry. Runtime state is not shared across instances. ### Architecture Contract The runtime model follows a strict build-then-dispatch lifecycle: -- Mutation is builder-only: command registration and config changes happen in `CliCoreBuilder`. -- `build()` is the freeze boundary: once built, `CliCore` has no runtime mutation API. -- No process-global mutable state: each `CliCore` instance owns an isolated registry and config. +- Mutation is builder-only: command registration and config changes happen in `CMDKitBuilder`. +- `build()` is the freeze boundary: once built, `CMDKit` has no runtime mutation API. +- No process-global mutable state: each `CMDKit` instance owns an isolated registry and config. - Runtime operations are read-only: dispatch and lookup use immutable access to core state. - Dispatch is deterministic: `try_run_from_args` takes explicit argv input and returns structured errors. Invariants: -- A built `CliCore` never mutates its registry or config during runtime. -- Two distinct `CliCore` instances do not share mutable state and cannot affect each other. +- A built `CMDKit` never mutates its registry or config during runtime. +- Two distinct `CMDKit` instances do not share mutable state and cannot affect each other. ### Command Construction @@ -63,6 +63,7 @@ Invariants: - `command(name, description)` fluent builder: - `.handler(...)` - `.handler_fn(...)` + - `.handler_fn_with_context(...)` - `.subcommand(...)` - `.with_usage(...)` - `.with_long_description(...)` @@ -84,17 +85,19 @@ Both support aliases. ## Quick Start ```rust -use cmdkit::{argument, command, switch, Argument, CliCore, CommandStrategy, StrategyError, Switch}; +use cmdkit::{argument, command, switch, CMDKit, CommandStrategy, InvocationArgs, StrategyError}; struct CreateProject; impl CommandStrategy for CreateProject { fn execute( &self, - options: Vec, - arguments: Vec, - _params: Vec, + _context: &cmdkit::ExecutionContext, + invocation: InvocationArgs, ) -> Result<(), StrategyError> { + let options = invocation.switches; + let arguments = invocation.args; + let name = arguments .iter() .find(|arg| arg.name == "name") @@ -116,7 +119,7 @@ impl CommandStrategy for CreateProject { fn main() { - let core = CliCore::builder() + let core = CMDKit::builder() .register( command("create", "Create a new project") .handler(CreateProject) @@ -139,22 +142,26 @@ fn main() { Nested trees can be built directly with the fluent builder: ```rust -use cmdkit::{command, CliCore}; +use cmdkit::{command, CMDKit}; fn main () { - let core = CliCore::builder() + let core = CMDKit::builder() .register( command("project", "Project commands") .subcommand( - command("create", "Create a project").handler_fn(|options, arguments, _| { + command("create", "Create a project").handler_fn(|_, invocation| { + let options = invocation.switches; + let arguments = invocation.args; println!("options={options:?} arguments={arguments:?}"); Ok(()) }), ) .subcommand( - command("delete", "Delete a project").handler_fn(|_, arguments, params| { + command("delete", "Delete a project").handler_fn(|_, invocation| { + let arguments = invocation.args; + let params = invocation.params; println!("arguments={arguments:?} params={params:?}"); Ok(()) - }), + }), ) .build(), ).build(); @@ -164,6 +171,36 @@ fn main () { Routing commands forward execution to leaf commands. The selected leaf strategy receives parsed input. +## Logger Access in Strategies + +Strategies receive an `ExecutionContext` during execution and can use the configured logger without globals. + +```rust +use cmdkit::{command, CoreConfig, ExecutionContext, LogLevel, LogSink, StrategyError}; + +struct StdoutLogger; +impl LogSink for StdoutLogger { + fn log(&self, level: LogLevel, message: &str) { + println!("[{level:?}] {message}"); + } +} + +fn main() { + let core = cmdkit::CMDKit::builder() + .with_config(CoreConfig::new().with_logger(StdoutLogger)) + .register( + command("run", "run command").handler_fn_with_context( + |ctx: &ExecutionContext, _invocation| { + ctx.logger.info("run called"); + Ok::<(), StrategyError>(()) + }, + ).build(), + ) + .build(); +} + +``` + ## Parser Behavior For an invocation like: @@ -229,21 +266,68 @@ impl HelpRenderer for JsonHelp { ````rust -use cmdkit::{CliCore, CoreConfig}; +use cmdkit::{CMDKit, CoreConfig}; fn main() { let config = CoreConfig::new(); - let core = CliCore::builder().with_config(config).build(); + let core = CMDKit::builder().with_config(config).build(); } ```` Use `CoreConfig` to customize runtime behavior such as the help renderer. -The registry is owned per `CliCore` instance and does not rely on lock-poison handling. +The registry is owned per `CMDKit` instance and does not rely on lock-poison handling. + +## Implementing Extensions + +CMDkit exposes two main extension points: `HelpRenderer` and `ArgumentInterpreter`. + +### Custom Help Renderer + +Implement `HelpRenderer` when you want to replace the default plain-text help output: + +```rust +use cmdkit::{Command, HelpRenderer}; + +struct CompactHelp; + +impl HelpRenderer for CompactHelp { + fn render(&self, caller: &str, commands: &[Command]) -> String { + format!("{}: {} commands available", caller, commands.len()) + } +} +``` + +### Custom Argument Interpreter + +Implement `ArgumentInterpreter` when you want to control how raw input is turned into invocation data: + +```rust +use cmdkit::{ArgumentInterpreter, CMDKitError, Command, InvocationArgs}; + +struct FixedCommandInterpreter; + +impl ArgumentInterpreter for FixedCommandInterpreter { + fn interpret( + &self, + _arg: &[String], + _registered_commands: &[Command], + ) -> Result { + Ok(InvocationArgs { + name: "status".to_string(), + args: Vec::new(), + switches: Vec::new(), + params: Vec::new(), + order: Vec::new(), + subcommand: None, + }) + } +} +``` ## Error Model -- `CliCoreError` for dispatch/runtime-level failures: +- `CMDKitError` for dispatch/runtime-level failures: - `MissingCommand` - `UnknownCommand` - `StrategyExecution` @@ -252,17 +336,17 @@ The registry is owned per `CliCore` instance and does not rely on lock-poison ha - `Execution` - `Internal` -`CliCoreError::StrategyExecution` preserves the originating `StrategyError` as source. +`CMDKitError::StrategyExecution` preserves the originating `StrategyError` as source. ## Testing and Embedding Use `try_run_from_args` to test dispatch deterministically: ```rust -use cmdkit::{CliCore, CliCoreError}; +use cmdkit::{CMDKit, CMDKitError}; -fn run_embedded(args: Vec) -> Result<(), CliCoreError> { - let core = CliCore::new(); +fn run_embedded(args: Vec) -> Result<(), CMDKitError> { + let core = CMDKit::builder().build(); core.try_run_from_args(&args) } ``` diff --git a/TODO.txt b/TODO.txt index b04efea..1888337 100644 --- a/TODO.txt +++ b/TODO.txt @@ -7,7 +7,7 @@ Priority legend: P1 [x] Remove shared-runtime coupling from root-level wrappers. - - Status: done. global_core was removed and wrappers now instantiate fresh CliCore runtimes per call. + - Status: done. global_core was removed and wrappers now instantiate fresh CMDKit runtimes per call. - Scope: src/core.rs wrapper functions + root wrappers in src/lib.rs. - Acceptance met: no shared process-global runtime is used for wrapper execution. @@ -63,9 +63,9 @@ P1 - Acceptance met: examples align with current APIs (`Command`, `CommandMetaData`, `CommandStrategy`, `CoreConfig`, macro usage) and stale trait methods were removed. [x] Introduce a forward-compatible runtime configuration object. - - Status: done. `CoreConfig` is now the runtime-owned config object, injected at `CliCore::create(...)` time, with `CliCore::new()` preserved as the backward-compatible default constructor. - - Scope: src/core.rs public API (`CliCore` construction/configuration) + tests. - - Acceptance met: runtime configuration is no longer scattered across ad hoc setters, and changing config requires constructing a new `CliCore`. + - Status: done. `CoreConfig` is now the runtime-owned config object, injected at `CMDKit::create(...)` time, with `CMDKit::new()` preserved as the backward-compatible default constructor. + - Scope: src/core.rs public API (`CMDKit` construction/configuration) + tests. + - Acceptance met: runtime configuration is no longer scattered across ad hoc setters, and changing config requires constructing a new `CMDKit`. [x] Naming lacks in precision the User might not understand what Functionality is and why he needs it - Status: done. Renamed `Functionality`/`CommandSpec` usage to explicit `Command` + `CommandMetaData`, and updated APIs/tests accordingly. @@ -106,3 +106,40 @@ P3 - Acceptance: 1. Argument Parser is reworked to use metadata values rather creating new Arguments. 2. All tests pass + +Async runtime follow-up + +P2 +[ ] Add a config-level preflight phase to detect and apply overrides before dispatch. + - Problem: runtime override intent (for example mode toggles or global switches) is currently mixed into normal dispatch input and is not resolved centrally before interpretation/execution. + - Scope: src/core.rs CoreConfig + CMDKit::try_run_from_args + CMDKitMaster::try_run_from_args + tests/core tests + tests/clicore_suite.rs. + - Acceptance: preflight runs before command dispatch, can normalize/filter args, can return runtime directives, and behavior is identical across CMDKit and CMDKitMaster paths. + +[ ] Add an explicit shutdown/join lifecycle for CMDKitMaster workers. + - Problem: worker threads are spawned, but the public API does not expose a clean way to stop them or wait for termination. + - Scope: src/core.rs CMDKitMaster worker management + tests/core tests. + - Acceptance: callers can close the executor, stop workers, and observe deterministic shutdown behavior without leaking background threads. + +P3 +[ ] Align public naming and docs with the broader runtime model. + - Problem: some user-facing wording still implies a CLI-only framing even though the crate now models general command dispatch and orchestration. + - Scope: README.md + Cargo.toml metadata + public API docs in src/lib.rs and src/core.rs. + - Acceptance: public docs and metadata describe the runtime consistently without over-constraining the use case to CLI-only workflows. + +P4 (Optional) +[ ] Wrap the execution handle in a more ergonomic awaitable type. + - Problem: the public handle currently exposes the underlying oneshot result shape, which is correct but a bit awkward for callers. + - Scope: src/core.rs public async handle API + lib exports + tests. + - Acceptance: callers can await a single-layer result type or helper wrapper without manually handling the internal oneshot cancellation shape. + + +-- Inspect + +1. +core-parse-command-clones — src/core.rs parse_command still clones commands, tokens, arguments, and switches while building InvocationArgs. +2. +strategy-router-dispatch-clones — src/cli/strategy.rs SubcommandRouter::resolve and recursive dispatch clone Command/InvocationArgs pieces during routing. +3. +registry-lookup-clones — src/cli/registry.rs get() and get_all() clone Command values; worth checking whether borrowed lookup is possible. +4. +catalog-merge-clones — src/cli/strategy.rs FallbackSubcommandStrategy::subcommands() clones and merges command catalogs; likely the biggest remaining aggregation hotspot. \ No newline at end of file diff --git a/src/bin/wrapper_probe.rs b/src/bin/wrapper_probe.rs index bf73613..14ea51a 100644 --- a/src/bin/wrapper_probe.rs +++ b/src/bin/wrapper_probe.rs @@ -1,13 +1,12 @@ -use cmdkit::{Argument, Command, CommandStrategy, StrategyError, Switch, core}; +use cmdkit::{CMDKit, Command, CommandStrategy, StrategyError, core::InvocationArgs}; struct ProbeStrategy; impl CommandStrategy for ProbeStrategy { fn execute( &self, - _options: Vec, - _arguments: Vec, - _params: Vec, + _context: &cmdkit::ExecutionContext, + _invocation: InvocationArgs, ) -> Result<(), StrategyError> { Ok(()) } @@ -15,7 +14,7 @@ impl CommandStrategy for ProbeStrategy { fn main() { println!("--FIRST--"); - core::CliCore::run_with_commands(&[Command::new("alpha", "alpha command", ProbeStrategy)]); + CMDKit::run_with_commands(&[Command::new("alpha", "alpha command", ProbeStrategy)]); println!("--SECOND--"); - core::CliCore::run_with_commands(&[Command::new("beta", "beta command", ProbeStrategy)]); + CMDKit::run_with_commands(&[Command::new("beta", "beta command", ProbeStrategy)]); } diff --git a/src/cli/command.rs b/src/cli/command.rs index 0acfcb0..44ce940 100644 --- a/src/cli/command.rs +++ b/src/cli/command.rs @@ -1,3 +1,5 @@ +use crate::core::{ExecutionContext, InvocationArgs}; +use std::fmt::Debug; use std::{option::Option, sync::Arc}; use super::strategy::FallbackSubcommandStrategy; @@ -7,7 +9,7 @@ use super::{ /// Declarative value-taking option metadata. #[derive(Clone, Debug, PartialEq, Eq)] -pub struct Switch { +pub struct SwitchDefinition { /// Canonical option name, for example: "path". pub name: String, /// Human-readable description for help output. @@ -16,7 +18,7 @@ pub struct Switch { pub aliases: Vec, } -impl Switch { +impl SwitchDefinition { /// Creates a value-taking option declaration. pub fn new(name: impl Into, description: impl Into) -> Self { Self { @@ -34,7 +36,7 @@ impl Switch { } #[derive(Clone, Debug, PartialEq, Eq)] -pub struct Argument { +pub struct ArgumentDefinition { /// Canonical /// Argument name, for example: "verbose". pub name: String, @@ -42,22 +44,57 @@ pub struct Argument { pub description: String, /// Alternative spellings accepted during parsing. pub aliases: Vec, - /// Numeric payload that can be mapped to an enum or bit mask. - pub value: Option, /// Whether this argument is required or optional. pub required: bool, + + pub value_type: ValueType, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum ValueType { + String, + Bool, + Float, + Int, } -impl Argument { +#[derive(Clone, Debug, PartialEq)] +pub enum ArgumentValue { + String(String), + Int(i64), + Bool(bool), + Float(f64), +} + +impl Eq for ArgumentValue {} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Argument { + pub name: String, + pub value: ArgumentValue, +} + +impl ArgumentDefinition { /// Creates a /// Argument declaration with the given numeric payload. - pub fn new(name: impl Into, description: impl Into) -> Self { + pub fn new( + name: impl Into, + description: impl Into, + value_type: ValueType, + ) -> Self { Self { name: name.into(), description: description.into(), aliases: Vec::new(), - value: None, required: false, + value_type, + } + } + + pub fn set_value(&self, value: ArgumentValue) -> Argument { + Argument { + name: self.name.clone(), + value, } } @@ -68,12 +105,6 @@ impl Argument { self } - /// Sets the value for this argument. - pub fn set_value(mut self, value: impl Into) -> Self { - self.value = Some(value.into()); - self - } - /// Sets required to true, indicating this argument is required. pub fn set_required(mut self) -> Self { self.required = true; @@ -95,10 +126,10 @@ pub struct CommandMetaData { /// Optional command examples shown in help output. pub examples: Vec, /// Optional option/flag descriptions shown in help output. - pub options: Vec, + pub options: Vec, /// Optional /// Argument/flag descriptions shown in help output. - pub arguments: Vec, + pub arguments: Vec, /// Optional aliases accepted by command discovery layers. pub aliases: Vec, } @@ -137,14 +168,14 @@ impl CommandMetaData { } /// Adds value-taking option definitions for this command. - pub fn with_options(mut self, options: Vec) -> Self { + pub fn with_options(mut self, options: Vec) -> Self { self.options = options; self } /// Adds /// Argument/flag definitions for this command. - pub fn with_arguments(mut self, arguments: Vec) -> Self { + pub fn with_arguments(mut self, arguments: Vec) -> Self { self.arguments = arguments; self } @@ -176,13 +207,23 @@ impl Command { } } - /// Executes this command after parsing raw argv-style arguments into the strategy contract. - pub fn execute(&self, args: Vec) -> Result<(), StrategyError> { - let invocation = parser::ArgumentParser::parse(args, &self.metadata, |token| { - self.matches_subcommand(token) - })?; - self.strategy - .execute(invocation.options, invocation.arguments, invocation.params) + pub(crate) fn execute( + &self, + context: &ExecutionContext, + mut invocation: InvocationArgs, + ) -> Result<(), StrategyError> { + match invocation.subcommand.take() { + Some(subcommand) => self + .resolve_subcommand(&subcommand.name) + .ok_or_else(|| { + StrategyError::invalid_arguments(format!( + "unknown subcommand '{}'", + subcommand.name + )) + })? + .execute(context, *subcommand), + None => self.strategy.execute(context, invocation), + } } /// Returns the optional subcommand catalog exposed by the underlying strategy. @@ -193,7 +234,7 @@ impl Command { /// Creates a command specification directly from a function or closure handler. pub fn from_fn(name: impl Into, description: impl Into, runner: F) -> Self where - F: Fn(Vec, Vec, Vec) -> Result<(), StrategyError> + F: Fn(&ExecutionContext, InvocationArgs) -> Result<(), StrategyError> + Send + Sync + 'static, @@ -201,14 +242,29 @@ impl Command { Self::new(name, description, FunctionStrategy::new(runner)) } + /// Alias for [`Command::from_fn`] kept for compatibility. + pub fn from_fn_with_context( + name: impl Into, + description: impl Into, + runner: F, + ) -> Self + where + F: Fn(&ExecutionContext, InvocationArgs) -> Result<(), StrategyError> + + Send + + Sync + + 'static, + { + Self::from_fn(name, description, runner) + } + /// Creates a fluent command builder. pub fn builder(name: impl Into, description: impl Into) -> CommandBuilder { CommandBuilder::new(name, description) } - fn matches_subcommand(&self, token: &str) -> bool { - self.subcommand_catalog().is_some_and(|catalog| { - catalog.subcommands().into_iter().any(|command| { + pub(crate) fn resolve_subcommand(&self, token: &str) -> Option { + self.subcommand_catalog().and_then(|catalog| { + catalog.subcommands().into_iter().find(|command| { command.metadata.name == token || command.metadata.aliases.iter().any(|alias| alias == token) }) @@ -216,25 +272,27 @@ impl Command { } } -struct ParsedInvocation { - options: Vec, - arguments: Vec, - params: Vec, -} - /// Creates a fluent command builder. pub fn command(name: impl Into, description: impl Into) -> CommandBuilder { CommandBuilder::new(name, description) } /// creates a value-taking option declaration. -pub fn argument(name: impl Into, description: impl Into) -> Argument { - Argument::new(name, description) +pub fn argument(name: impl Into, description: impl Into) -> ArgumentDefinition { + ArgumentDefinition::new(name, description, ValueType::String) +} + +pub fn argument_of_type( + name: impl Into, + description: impl Into, + value_type: ValueType, +) -> ArgumentDefinition { + ArgumentDefinition::new(name, description, value_type) } /// creates a value-taking option declaration with the required flag set to true. -pub fn switch(name: impl Into, description: impl Into) -> Switch { - Switch::new(name, description) +pub fn switch(name: impl Into, description: impl Into) -> SwitchDefinition { + SwitchDefinition::new(name, description) } /// Fluent command builder that hides strategy implementation details. pub struct CommandBuilder { @@ -265,7 +323,19 @@ impl CommandBuilder { /// Sets command strategy using a function/closure. pub fn handler_fn(mut self, runner: F) -> Self where - F: Fn(Vec, Vec, Vec) -> Result<(), StrategyError> + F: Fn(&ExecutionContext, InvocationArgs) -> Result<(), StrategyError> + + Send + + Sync + + 'static, + { + self.strategy = Some(Arc::new(FunctionStrategy::new(runner))); + self + } + + /// Alias for [`CommandBuilder::handler_fn`] kept for compatibility. + pub fn handler_fn_with_context(mut self, runner: F) -> Self + where + F: Fn(&ExecutionContext, InvocationArgs) -> Result<(), StrategyError> + Send + Sync + 'static, @@ -302,14 +372,14 @@ impl CommandBuilder { } /// Adds option/flag description entries for this command. - pub fn with_options(mut self, options: Vec) -> Self { + pub fn with_options(mut self, options: Vec) -> Self { self.metadata = self.metadata.with_options(options); self } /// Adds /// Argument/flag description entries for this command. - pub fn with_arguments(mut self, arguments: Vec) -> Self { + pub fn with_arguments(mut self, arguments: Vec) -> Self { self.metadata = self.metadata.with_arguments(arguments); self } @@ -326,7 +396,7 @@ impl CommandBuilder { pub fn build(self) -> Command { let strategy: Arc = if self.subcommands.is_empty() { self.strategy.unwrap_or_else(|| { - Arc::new(FunctionStrategy::new(|_, _, _| { + Arc::new(FunctionStrategy::new(|_, _| { Err(StrategyError::internal( "command has no handler; configure a handler or subcommand", )) @@ -356,169 +426,3 @@ impl From for Command { value.build() } } - -mod parser { - use super::{Argument, CommandMetaData, ParsedInvocation, Switch}; - use crate::StrategyError; - - pub(super) struct ArgumentParser; - - impl ArgumentParser { - fn find_declared_argument<'a>( - metadata: &'a CommandMetaData, - flag: &str, - ) -> Option<&'a Argument> { - metadata.arguments.iter().find(|argument| { - argument.name == flag || argument.aliases.iter().any(|alias| alias == flag) - }) - } - - fn find_declared_switch<'a>( - metadata: &'a CommandMetaData, - flag: &str, - ) -> Option<&'a Switch> { - metadata.options.iter().find(|option| { - option.name == flag || option.aliases.iter().any(|alias| alias == flag) - }) - } - - fn upsert_argument(arguments: &mut Vec, argument: Argument) { - if let Some(existing) = arguments - .iter_mut() - .find(|existing| existing.name == argument.name) - { - *existing = argument; - return; - } - - arguments.push(argument); - } - - fn validate_required_arguments( - metadata: &CommandMetaData, - arguments: &[Argument], - ) -> Result<(), StrategyError> { - for required in metadata - .arguments - .iter() - .filter(|argument| argument.required) - { - let value = arguments - .iter() - .find(|argument| argument.name == required.name) - .and_then(|argument| argument.value.as_deref()); - - if value.is_none_or(|value| value.trim().is_empty()) { - return Err(StrategyError::invalid_arguments(format!( - "missing value for required argument '--{}'", - required.name - ))); - } - } - - Ok(()) - } - - pub fn parse( - args: Vec, - metadata: &CommandMetaData, - is_subcommand: F, - ) -> Result - where - F: Fn(&str) -> bool, - { - let mut options = Vec::new(); - let mut arguments = Vec::new(); - let mut params = Vec::new(); - let mut index = 0; - - while index < args.len() { - let token = &args[index]; - - if is_subcommand(token) { - break; - } - - let Some(flag) = token.strip_prefix("--") else { - params.push(token.clone()); - index += 1; - continue; - }; - - if let Some((flag_name, inline_value)) = flag.split_once('=') { - if let Some(argument_decl) = Self::find_declared_argument(metadata, flag_name) { - Self::upsert_argument( - &mut arguments, - argument_decl.clone().set_value(inline_value.to_string()), - ); - index += 1; - continue; - } - - if let Some(option_decl) = Self::find_declared_switch(metadata, flag_name) { - return Err(StrategyError::invalid_arguments(format!( - "switch '--{}' does not take a value", - option_decl.name - ))); - } - - return Err(StrategyError::invalid_arguments(format!( - "unknown flag '--{}'", - flag_name - ))); - } - - if let Some(argument_decl) = Self::find_declared_argument(metadata, flag) { - let Some(next) = args.get(index + 1) else { - return Err(StrategyError::invalid_arguments(format!( - "missing value for argument '--{}'", - argument_decl.name - ))); - }; - - if next.starts_with("--") || is_subcommand(next) { - return Err(StrategyError::invalid_arguments(format!( - "missing value for argument '--{}'", - argument_decl.name - ))); - } - - Self::upsert_argument( - &mut arguments, - argument_decl.clone().set_value(next.clone()), - ); - index += 2; - continue; - } - - if let Some(option_decl) = Self::find_declared_switch(metadata, flag) { - if flag.contains('=') { - return Err(StrategyError::invalid_arguments(format!( - "switch '--{}' does not take a value", - option_decl.name - ))); - } - - options.push(option_decl.clone()); - index += 1; - continue; - } - - return Err(StrategyError::invalid_arguments(format!( - "unknown flag '--{}'", - flag - ))); - } - - params.extend_from_slice(&args[index..]); - - Self::validate_required_arguments(metadata, &arguments)?; - - Ok(ParsedInvocation { - options, - arguments, - params, - }) - } - } -} diff --git a/src/cli/error.rs b/src/cli/error.rs index 3c28c79..ef8c129 100644 --- a/src/cli/error.rs +++ b/src/cli/error.rs @@ -63,3 +63,6 @@ impl fmt::Display for StrategyError { } impl Error for StrategyError {} + +#[cfg(test)] +mod tests; diff --git a/src/cli/error/tests.rs b/src/cli/error/tests.rs new file mode 100644 index 0000000..fc2ee86 --- /dev/null +++ b/src/cli/error/tests.rs @@ -0,0 +1,32 @@ +use super::{StrategyError, StrategyErrorKind}; + +#[test] +fn strategy_error_kind_display_labels_are_stable() { + assert_eq!( + StrategyErrorKind::InvalidArguments.to_string(), + "invalid-arguments" + ); + assert_eq!(StrategyErrorKind::Execution.to_string(), "execution"); + assert_eq!(StrategyErrorKind::Internal.to_string(), "internal"); +} + +#[test] +fn strategy_error_constructors_set_kind_and_message() { + let invalid = StrategyError::invalid_arguments("bad input"); + assert_eq!(invalid.kind, StrategyErrorKind::InvalidArguments); + assert_eq!(invalid.message, "bad input"); + + let execution = StrategyError::execution("failed"); + assert_eq!(execution.kind, StrategyErrorKind::Execution); + assert_eq!(execution.message, "failed"); + + let internal = StrategyError::internal("boom"); + assert_eq!(internal.kind, StrategyErrorKind::Internal); + assert_eq!(internal.message, "boom"); +} + +#[test] +fn strategy_error_display_formats_kind_and_message() { + let err = StrategyError::new(StrategyErrorKind::Execution, "could not run"); + assert_eq!(err.to_string(), "execution: could not run"); +} diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 87cee78..d15f563 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -4,8 +4,10 @@ mod registry; mod strategy; pub use command::{ - Argument, Command, CommandBuilder, CommandMetaData, Switch, argument, command, switch, + Argument, ArgumentDefinition, ArgumentValue, Command, CommandBuilder, CommandMetaData, + SwitchDefinition, ValueType, argument, argument_of_type, command, switch, }; + pub use error::{StrategyError, StrategyErrorKind}; -pub(crate) use registry::CommandRegistry; +pub(crate) use registry::CommandCatalogue; pub use strategy::{CommandStrategy, FunctionStrategy, SubcommandCatalog, SubcommandRouter}; diff --git a/src/cli/registry.rs b/src/cli/registry.rs index ce44de9..4aa13f2 100644 --- a/src/cli/registry.rs +++ b/src/cli/registry.rs @@ -1,24 +1,92 @@ -use std::collections::BTreeMap; - use super::Command; +use std::collections::{BTreeMap, BTreeSet}; /// Internal registry storage for command-to-command mappings. #[derive(Default, Clone)] -pub(crate) struct CommandRegistry { +pub(crate) struct CommandCatalogue { commands: BTreeMap, + aliases: BTreeMap, } -impl CommandRegistry { - pub(crate) fn get(&self, name: &str) -> Option { - self.commands.get(name).cloned() +impl CommandCatalogue { + pub(crate) fn get(&self, name: &str) -> Option<&Command> { + if let Some(command) = self.commands.get(name) { + return Some(command); + } + + self.aliases + .get(name) + .and_then(|canonical| self.commands.get(canonical)) } - pub(crate) fn register(&mut self, command: Command) -> &mut CommandRegistry { - self.commands.insert(command.metadata.name.clone(), command); - self + pub(crate) fn register(&mut self, command: Command) -> Result<&mut CommandCatalogue, String> { + self.validate_alias_collisions(&command)?; + + let command_name = command.metadata.name.clone(); + + if let Some(previous) = self.commands.remove(&command_name) { + for alias in previous.metadata.aliases { + self.aliases.remove(&alias); + } + } + + for alias in &command.metadata.aliases { + self.aliases.insert(alias.clone(), command_name.clone()); + } + + self.commands.insert(command_name, command); + Ok(self) } - pub(crate) fn get_all(&self) -> Vec { - self.commands.values().cloned().collect() + pub(crate) fn get_all(&self) -> Vec<&Command> { + self.commands.values().collect() + } + + fn validate_alias_collisions(&self, command: &Command) -> Result<(), String> { + if let Some(existing_owner) = self.aliases.get(&command.metadata.name) + && existing_owner != &command.metadata.name + { + return Err(format!( + "command name '{}' conflicts with existing alias owned by '{}'", + command.metadata.name, existing_owner + )); + } + + let mut seen_aliases = BTreeSet::new(); + for alias in &command.metadata.aliases { + if alias == &command.metadata.name { + return Err(format!( + "alias '{}' duplicates command name '{}'", + alias, command.metadata.name + )); + } + + if !seen_aliases.insert(alias.clone()) { + return Err(format!( + "alias '{}' is declared more than once for command '{}'", + alias, command.metadata.name + )); + } + + if let Some(existing_command) = self.commands.get(alias) + && existing_command.metadata.name != command.metadata.name + { + return Err(format!( + "alias '{}' conflicts with existing command name '{}'", + alias, existing_command.metadata.name + )); + } + + if let Some(existing_owner) = self.aliases.get(alias) + && existing_owner != &command.metadata.name + { + return Err(format!( + "alias '{}' conflicts with existing alias owned by '{}'", + alias, existing_owner + )); + } + } + + Ok(()) } } diff --git a/src/cli/strategy.rs b/src/cli/strategy.rs index 76be163..a69b23f 100644 --- a/src/cli/strategy.rs +++ b/src/cli/strategy.rs @@ -1,6 +1,7 @@ +use crate::core::{ExecutionContext, InvocationArgs, InvocationElement}; use std::{collections::BTreeMap, sync::Arc}; -use super::{Argument, Command, StrategyError, Switch}; +use super::{Command, StrategyError}; /// Optional help-time capability exposed by strategies that can route to child strategies. pub trait SubcommandCatalog { @@ -14,9 +15,8 @@ pub trait CommandStrategy: Send + Sync { /// Strategy implementations should validate argument viability internally. fn execute( &self, - options: Vec, - arguments: Vec, - params: Vec, + context: &ExecutionContext, + invocation: InvocationArgs, ) -> Result<(), StrategyError>; /// Optional catalog exposure used by help renderers to discover nested command trees. @@ -28,14 +28,14 @@ pub trait CommandStrategy: Send + Sync { /// Adapter that turns a function or closure into a [`CommandStrategy`]. pub struct FunctionStrategy where - F: Fn(Vec, Vec, Vec) -> Result<(), StrategyError> + Send + Sync, + F: Fn(&ExecutionContext, InvocationArgs) -> Result<(), StrategyError> + Send + Sync, { runner: F, } impl FunctionStrategy where - F: Fn(Vec, Vec, Vec) -> Result<(), StrategyError> + Send + Sync, + F: Fn(&ExecutionContext, InvocationArgs) -> Result<(), StrategyError> + Send + Sync, { pub fn new(runner: F) -> Self { Self { runner } @@ -44,15 +44,14 @@ where impl CommandStrategy for FunctionStrategy where - F: Fn(Vec, Vec, Vec) -> Result<(), StrategyError> + Send + Sync, + F: Fn(&ExecutionContext, InvocationArgs) -> Result<(), StrategyError> + Send + Sync, { fn execute( &self, - options: Vec, - arguments: Vec, - params: Vec, + context: &ExecutionContext, + invocation: InvocationArgs, ) -> Result<(), StrategyError> { - (self.runner)(options, arguments, params) + (self.runner)(context, invocation) } } @@ -117,11 +116,10 @@ impl SubcommandCatalog for SubcommandRouter { impl CommandStrategy for SubcommandRouter { fn execute( &self, - _options: Vec, - _arguments: Vec, - params: Vec, + context: &ExecutionContext, + invocation: InvocationArgs, ) -> Result<(), StrategyError> { - let Some(subcommand_name) = params.first() else { + let Some(subcommand_name) = invocation.params.first() else { return Err(StrategyError::invalid_arguments(format!( "missing subcommand. available: {}", self.available_subcommands() @@ -135,7 +133,21 @@ impl CommandStrategy for SubcommandRouter { )) })?; - command.execute(params[1..].to_vec()) + command.execute( + context, + InvocationArgs { + name: command.metadata.name.clone(), + args: Vec::new(), + switches: Vec::new(), + params: invocation.params[1..].to_vec(), + order: invocation.params[1..] + .iter() + .cloned() + .map(InvocationElement::Param) + .collect(), + subcommand: None, + }, + ) } fn subcommand_catalog(&self) -> Option<&dyn SubcommandCatalog> { @@ -157,14 +169,13 @@ impl FallbackSubcommandStrategy { impl CommandStrategy for FallbackSubcommandStrategy { fn execute( &self, - options: Vec, - arguments: Vec, - params: Vec, + context: &ExecutionContext, + invocation: InvocationArgs, ) -> Result<(), StrategyError> { - if params.is_empty() { - return self.strategy.execute(options, arguments, params); + if invocation.params.is_empty() { + return self.strategy.execute(context, invocation); } - self.router.execute(options, arguments, params) + self.router.execute(context, invocation) } fn subcommand_catalog(&self) -> Option<&dyn SubcommandCatalog> { @@ -191,3 +202,6 @@ impl SubcommandCatalog for FallbackSubcommandStrategy { subcommands.into_values().collect() } } + +#[cfg(test)] +mod tests; diff --git a/src/cli/strategy/tests.rs b/src/cli/strategy/tests.rs new file mode 100644 index 0000000..0925dd9 --- /dev/null +++ b/src/cli/strategy/tests.rs @@ -0,0 +1,276 @@ +use std::sync::{Arc, Mutex}; + +use super::{ + CommandStrategy, FallbackSubcommandStrategy, FunctionStrategy, SubcommandCatalog, + SubcommandRouter, +}; +use crate::{ + Command, CoreConfig, InvocationArgs, StrategyError, StrategyErrorKind, + SubcommandRouter as PublicRouter, command, +}; + +fn invocation(params: Vec) -> InvocationArgs { + InvocationArgs { + name: "run".to_string(), + args: Vec::new(), + switches: Vec::new(), + params, + order: Vec::new(), + subcommand: None, + } +} + +fn execution_context() -> crate::ExecutionContext { + CoreConfig::new().execution_context() +} + +#[test] +fn router_errors_when_subcommand_token_is_missing() { + let router = SubcommandRouter::new().register( + command("run", "run command") + .handler_fn(|_, _| Ok(())) + .build(), + ); + + let context = execution_context(); + let result = router.execute(&context, invocation(Vec::new())); + match result { + Err(err) => { + assert_eq!(err.kind, StrategyErrorKind::InvalidArguments); + assert!(err.message.contains("missing subcommand")); + assert!(err.message.contains("run")); + } + _ => panic!("expected missing subcommand error"), + } +} + +#[test] +fn router_errors_when_subcommand_is_unknown() { + let router = SubcommandRouter::new().register( + command("run", "run command") + .handler_fn(|_, _| Ok(())) + .build(), + ); + + let context = execution_context(); + let result = router.execute(&context, invocation(vec!["ghost".to_string()])); + match result { + Err(err) => { + assert_eq!(err.kind, StrategyErrorKind::InvalidArguments); + assert!(err.message.contains("unknown subcommand 'ghost'")); + assert!(err.message.contains("run")); + } + _ => panic!("expected unknown subcommand error"), + } +} + +#[test] +fn router_resolves_alias_and_forwards_tail_params() { + let calls: Arc>>> = Arc::new(Mutex::new(Vec::new())); + let calls_for_handler = Arc::clone(&calls); + + let subcommand = command("run", "run command") + .with_aliases(vec!["r"]) + .handler_fn(move |_, invocation| { + let params = invocation.params; + calls_for_handler + .lock() + .expect("calls lock should not be poisoned") + .push(params); + Ok(()) + }) + .build(); + + let router = SubcommandRouter::new().register(subcommand); + let context = execution_context(); + let result = router.execute( + &context, + invocation(vec![ + "r".to_string(), + "tail-1".to_string(), + "tail-2".to_string(), + ]), + ); + + assert!(result.is_ok()); + let guard = calls.lock().expect("calls lock should not be poisoned"); + assert_eq!( + guard.as_slice(), + &[vec!["tail-1".to_string(), "tail-2".to_string()]] + ); +} + +#[test] +fn fallback_executes_primary_strategy_when_no_params_are_provided() { + let fallback_calls: Arc>>> = Arc::new(Mutex::new(Vec::new())); + let fallback_calls_for_handler = Arc::clone(&fallback_calls); + let child_calls: Arc>>> = Arc::new(Mutex::new(Vec::new())); + let child_calls_for_handler = Arc::clone(&child_calls); + + let fallback_strategy: Arc = + Arc::new(FunctionStrategy::new(move |_, invocation| { + let params = invocation.params; + fallback_calls_for_handler + .lock() + .expect("fallback lock should not be poisoned") + .push(params); + Ok(()) + })); + + let router = PublicRouter::new().register( + command("run", "run command") + .handler_fn(move |_, invocation| { + let params = invocation.params; + child_calls_for_handler + .lock() + .expect("child lock should not be poisoned") + .push(params); + Ok(()) + }) + .build(), + ); + + let fallback = FallbackSubcommandStrategy::new(fallback_strategy, router); + let context = execution_context(); + let result = fallback.execute(&context, invocation(Vec::new())); + + assert!(result.is_ok()); + assert_eq!( + fallback_calls + .lock() + .expect("fallback lock should not be poisoned") + .as_slice(), + &[Vec::::new()] + ); + assert!( + child_calls + .lock() + .expect("child lock should not be poisoned") + .is_empty() + ); +} + +#[test] +fn fallback_routes_to_router_when_params_exist() { + let fallback_calls: Arc>>> = Arc::new(Mutex::new(Vec::new())); + let fallback_calls_for_handler = Arc::clone(&fallback_calls); + let child_calls: Arc>>> = Arc::new(Mutex::new(Vec::new())); + let child_calls_for_handler = Arc::clone(&child_calls); + + let fallback_strategy: Arc = + Arc::new(FunctionStrategy::new(move |_, invocation| { + let params = invocation.params; + fallback_calls_for_handler + .lock() + .expect("fallback lock should not be poisoned") + .push(params); + Ok(()) + })); + + let router = PublicRouter::new().register( + command("run", "run command") + .handler_fn(move |_, invocation| { + let params = invocation.params; + child_calls_for_handler + .lock() + .expect("child lock should not be poisoned") + .push(params); + Ok(()) + }) + .build(), + ); + + let fallback = FallbackSubcommandStrategy::new(fallback_strategy, router); + let context = execution_context(); + let result = fallback.execute( + &context, + invocation(vec!["run".to_string(), "tail".to_string()]), + ); + + assert!(result.is_ok()); + assert!( + fallback_calls + .lock() + .expect("fallback lock should not be poisoned") + .is_empty() + ); + assert_eq!( + child_calls + .lock() + .expect("child lock should not be poisoned") + .as_slice(), + &[vec!["tail".to_string()]] + ); +} + +struct CatalogStrategy { + catalog: CatalogOnly, +} + +impl CommandStrategy for CatalogStrategy { + fn execute( + &self, + _context: &crate::ExecutionContext, + _invocation: InvocationArgs, + ) -> Result<(), StrategyError> { + Ok(()) + } + + fn subcommand_catalog(&self) -> Option<&dyn SubcommandCatalog> { + Some(&self.catalog) + } +} + +struct CatalogOnly { + commands: Vec, +} + +impl SubcommandCatalog for CatalogOnly { + fn subcommands(&self) -> Vec { + self.commands.clone() + } +} + +#[test] +fn fallback_catalog_merges_router_and_fallback_without_duplicate_names() { + let router = PublicRouter::new() + .register( + command("shared", "router shared") + .handler_fn(|_, _| Ok(())) + .build(), + ) + .register( + command("router-only", "router only") + .handler_fn(|_, _| Ok(())) + .build(), + ); + + let fallback_strategy: Arc = Arc::new(CatalogStrategy { + catalog: CatalogOnly { + commands: vec![ + command("shared", "fallback shared") + .handler_fn(|_, _| Ok(())) + .build(), + command("fallback-only", "fallback only") + .handler_fn(|_, _| Ok(())) + .build(), + ], + }, + }); + + let fallback = FallbackSubcommandStrategy::new(fallback_strategy, router); + let mut names = SubcommandCatalog::subcommands(&fallback) + .into_iter() + .map(|cmd| cmd.metadata.name) + .collect::>(); + names.sort(); + + assert_eq!( + names, + vec![ + "fallback-only".to_string(), + "router-only".to_string(), + "shared".to_string(), + ] + ); +} diff --git a/src/core.rs b/src/core.rs index ba07ffd..fa7a5e4 100644 --- a/src/core.rs +++ b/src/core.rs @@ -1,17 +1,363 @@ -use std::{error::Error, fmt, sync::Arc}; +use std::{ + error::Error, + fmt, + sync::{Arc, Mutex, mpsc}, + thread, +}; + +use futures_channel::oneshot; + +use crate::cli::ValueType; +use crate::core::logging::LogLevel; +use crate::{ + Argument, ArgumentDefinition, ArgumentValue, Command, StrategyError, SwitchDefinition, + cli::CommandCatalogue, +}; + +pub mod logging { + + #[repr(u8)] + #[derive(Clone, Copy, Eq, PartialEq, Debug)] + pub enum LogLevel { + Debug, + Info, + Warn, + Error, + } + pub trait LogSink: Send + Sync { + fn log(&self, level: LogLevel, message: &str); + fn info(&self, message: &str) { + self.log(LogLevel::Info, message); + } + fn warn(&self, message: &str) { + self.log(LogLevel::Warn, message); + } + fn error(&self, message: &str) { + self.log(LogLevel::Error, message); + } + fn debug(&self, message: &str) { + self.log(LogLevel::Debug, message); + } + } -use crate::{Command, StrategyError, cli::CommandRegistry}; + pub struct ConsoleLogSink { + verbosity: LogLevel, + } + + impl ConsoleLogSink { + pub fn new(verbosity: LogLevel) -> Self { + Self { verbosity } + } + } + impl LogSink for ConsoleLogSink { + fn log(&self, level: LogLevel, message: &str) { + match level { + // always print errors + LogLevel::Error => eprintln!("{}", message), + // print only if loglevel is higher or equal to loglevel set in log sink + _ => { + if self.verbosity as i8 - level as i8 <= 0 { + println!("{}", message) + } + } + } + } + } +} /// Renders user-facing help output from registered command metadata. pub trait HelpRenderer: Send + Sync { - fn render(&self, caller: &str, registered_commands: &[Command]) -> String; + fn render(&self, caller: &str, registered_commands: &[&Command]) -> String; +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum InvocationElement { + Argument(Argument), + Switch(String), + Param(String), +} + +/// InvocationArgs is a single-consumption execution envelope and is not guaranteed to remain intact after dispatch traversal. +#[derive(Clone, Debug, PartialEq)] +pub struct InvocationArgs { + pub name: String, + pub args: Vec, + pub switches: Vec, + pub params: Vec, + pub order: Vec, + pub subcommand: Option>, +} + +impl InvocationArgs { + pub fn leaf_name(&self) -> &str { + self.subcommand + .as_deref() + .map_or(self.name.as_str(), InvocationArgs::leaf_name) + } +} + +pub trait ArgumentInterpreter: Send + Sync { + fn interpret( + &self, + arg: &[String], + registered_commands: &[&Command], + ) -> Result; +} + +#[derive(Clone)] +pub struct ExecutionContext { + pub logger: Arc, +} + +#[derive(Debug, Default, Clone, Copy)] +pub struct PlainTextArgumentInterpreter; + +impl PlainTextArgumentInterpreter { + fn find_declared_argument<'a>( + command: &'a Command, + flag: &str, + ) -> Option<&'a ArgumentDefinition> { + command.metadata.arguments.iter().find(|argument| { + argument.name == flag || argument.aliases.iter().any(|alias| alias == flag) + }) + } + + fn find_declared_switch<'a>(command: &'a Command, flag: &str) -> Option<&'a SwitchDefinition> { + command + .metadata + .options + .iter() + .find(|option| option.name == flag || option.aliases.iter().any(|alias| alias == flag)) + } + + fn resolve_command<'a>(commands: &'a [&'a Command], token: &str) -> Option<&'a Command> { + commands.iter().find_map(|command| { + ((command.metadata.name == token) + || command.metadata.aliases.iter().any(|alias| alias == token)) + .then_some(*command) + }) + } + + fn upsert_argument(arguments: &mut Vec, argument: Argument) { + if let Some(index) = arguments + .iter() + .position(|existing| existing.name == argument.name) + { + arguments.remove(index); + } + + arguments.push(argument); + } + + fn validate_required_arguments( + command: &Command, + arguments: &[Argument], + ) -> Result<(), CMDKitError> { + for required in command.metadata.arguments.iter().filter(|arg| arg.required) { + let value = arguments + .iter() + .find(|argument| argument.name == required.name); + + if value.is_none() { + return Err(Self::invalid_arguments( + command, + format!("missing value for required argument '--{}'", required.name), + )); + } + } + + Ok(()) + } + + fn invalid_arguments(command: &Command, message: impl Into) -> CMDKitError { + CMDKitError::StrategyExecution { + command: command.metadata.name.clone(), + source: StrategyError::invalid_arguments(message), + } + } + + fn parse_command( + &self, + command: &Command, + args: &[String], + ) -> Result { + Self::parse_command_impl(command, args) + } + + fn parse_command_impl( + command: &Command, + args: &[String], + ) -> Result { + let mut switches = Vec::new(); + let mut arguments = Vec::new(); + let mut params = Vec::new(); + let mut order = Vec::new(); + let mut index = 0; + + while index < args.len() { + let token = &args[index]; + + if params.is_empty() + && let Some(subcommand) = command.resolve_subcommand(token) + { + Self::validate_required_arguments(command, &arguments)?; + + return Ok(InvocationArgs { + name: command.metadata.name.clone(), + args: arguments, + switches, + params, + order, + subcommand: Some(Box::new(Self::parse_command_impl( + &subcommand, + &args[index + 1..], + )?)), + }); + } + + let Some(flag) = token.strip_prefix("--") else { + params.push(token.clone()); + order.push(InvocationElement::Param(token.clone())); + index += 1; + continue; + }; + + if let Some((flag_name, inline_value)) = flag.split_once('=') { + if let Some(declared_argument) = Self::find_declared_argument(command, flag_name) { + if inline_value.is_empty() { + // Skip creating argument for empty values; let validation catch it + index += 1; + continue; + } + let argument = Self::convert_to_argument_value( + declared_argument, + &inline_value.to_string(), + ); + let order_arg = argument.clone(); + Self::upsert_argument(&mut arguments, argument); + order.push(InvocationElement::Argument(order_arg)); + index += 1; + continue; + } + + if let Some(option_decl) = Self::find_declared_switch(command, flag_name) { + return Err(Self::invalid_arguments( + command, + format!("switch '--{}' does not take a value", option_decl.name), + )); + } + + return Err(Self::invalid_arguments( + command, + format!("unknown flag '--{}'", flag_name), + )); + } + + if let Some(declared_argument) = Self::find_declared_argument(command, flag) { + let Some(argument_value) = args.get(index + 1) else { + let msg = if declared_argument.required { + format!( + "missing value for required argument '--{}'", + declared_argument.name + ) + } else { + format!("missing value for argument '--{}'", declared_argument.name) + }; + return Err(Self::invalid_arguments(command, msg)); + }; + + if argument_value.is_empty() + || argument_value.starts_with("--") + || command.resolve_subcommand(argument_value).is_some() + { + let msg = if declared_argument.required { + format!( + "missing value for required argument '--{}'", + declared_argument.name + ) + } else { + format!("missing value for argument '--{}'", declared_argument.name) + }; + return Err(Self::invalid_arguments(command, msg)); + } + + let argument = Self::convert_to_argument_value(declared_argument, argument_value); + let order_arg = argument.clone(); + Self::upsert_argument(&mut arguments, argument); + order.push(InvocationElement::Argument(order_arg)); + index += 2; + continue; + } + + if let Some(option_decl) = Self::find_declared_switch(command, flag) { + let switch = &option_decl.name; + switches.push(switch.clone()); + order.push(InvocationElement::Switch(switch.clone())); + index += 1; + continue; + } + + return Err(Self::invalid_arguments( + command, + format!("unknown flag '--{}'", flag), + )); + } + + Self::validate_required_arguments(command, &arguments)?; + + Ok(InvocationArgs { + name: command.metadata.name.clone(), + args: arguments, + switches, + params, + order, + subcommand: None, + }) + } + + fn convert_to_argument_value( + declared_argument: &ArgumentDefinition, + argument_value: &String, + ) -> Argument { + let value = match declared_argument.value_type { + ValueType::Float => ArgumentValue::Float(argument_value.parse().unwrap()), + ValueType::Int => ArgumentValue::Int(argument_value.parse().unwrap()), + ValueType::Bool => ArgumentValue::Bool(argument_value.parse().unwrap()), + _ => ArgumentValue::String(argument_value.to_string()), + }; + declared_argument.set_value(value) + } +} + +impl ArgumentInterpreter for PlainTextArgumentInterpreter { + fn interpret( + &self, + arg: &[String], + registered_commands: &[&Command], + ) -> Result { + let Some(command_name) = arg.first() else { + return Err(CMDKitError::MissingCommand { + help: String::new(), + }); + }; + + let command = + Self::resolve_command(registered_commands, command_name).ok_or_else(|| { + CMDKitError::UnknownCommand { + command: command_name.clone(), + help: String::new(), + } + })?; + + self.parse_command(command, &arg[1..]) + } } /// Default plain-text help renderer. pub struct PlainTextHelpRenderer; impl HelpRenderer for PlainTextHelpRenderer { - fn render(&self, caller: &str, registered_commands: &[Command]) -> String { + fn render(&self, caller: &str, registered_commands: &[&Command]) -> String { fn render_recursive(command: &Command, depth: usize, path: String, out: &mut Vec) { let indent = " ".repeat(depth); out.push(format!( @@ -112,6 +458,8 @@ impl HelpRenderer for PlainTextHelpRenderer { #[derive(Clone)] pub struct CoreConfig { pub help_renderer: Arc, + pub argument_interpreter: Arc, + pub logger: Arc, } impl CoreConfig { @@ -119,9 +467,25 @@ impl CoreConfig { pub fn new() -> Self { Self { help_renderer: Arc::new(PlainTextHelpRenderer), + argument_interpreter: Arc::new(PlainTextArgumentInterpreter), + logger: Arc::new(logging::ConsoleLogSink::new(LogLevel::Info)), + } + } + + pub fn execution_context(&self) -> ExecutionContext { + ExecutionContext { + logger: Arc::clone(&self.logger), } } + pub fn with_logger(mut self, logger: L) -> Self + where + L: logging::LogSink + 'static, + { + self.logger = Arc::new(logger); + self + } + /// Replaces the help renderer in a builder-friendly way. pub fn with_help_renderer(mut self, renderer: R) -> Self where @@ -130,6 +494,15 @@ impl CoreConfig { self.help_renderer = Arc::new(renderer); self } + + /// Replaces the argument interpreter in a builder-friendly way. + pub fn with_argument_interpreter(mut self, interpreter: I) -> Self + where + I: ArgumentInterpreter + 'static, + { + self.argument_interpreter = Arc::new(interpreter); + self + } } impl Default for CoreConfig { @@ -140,7 +513,11 @@ impl Default for CoreConfig { /// Error returned by CMDkit during command routing and strategy execution. #[derive(Debug)] -pub enum CliCoreError { +pub enum CMDKitError { + /// Command registration failed due to invalid or conflicting metadata. + Registration { message: String }, + /// Master executor cannot accept or complete jobs. + ExecutorUnavailable { message: String }, /// No command name was provided in argv. MissingCommand { help: String }, /// The command name does not exist in the registry. @@ -154,9 +531,15 @@ pub enum CliCoreError { }, } -impl fmt::Display for CliCoreError { +impl fmt::Display for CMDKitError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { + Self::Registration { message } => { + write!(f, "Command registration failed: {message}") + } + Self::ExecutorUnavailable { message } => { + write!(f, "Executor unavailable: {message}") + } Self::MissingCommand { help } => { write!(f, "No command provided.\n\n{help}") } @@ -170,7 +553,7 @@ impl fmt::Display for CliCoreError { } } -impl Error for CliCoreError { +impl Error for CMDKitError { fn source(&self) -> Option<&(dyn Error + 'static)> { match self { Self::StrategyExecution { source, .. } => Some(source), @@ -181,30 +564,34 @@ impl Error for CliCoreError { /// Instance-owned CLI runtime. /// -/// Each [`CliCore`] owns a lazily initialized command registry and can be reused +/// Each [`CMDKit`] owns a lazily initialized command registry and can be reused /// across multiple invocations without relying on process-global mutable state. -pub struct CliCore { - registry: CommandRegistry, +pub struct CMDKit { + registry: CommandCatalogue, config: CoreConfig, } -impl CliCore { - /// Creates a [`CliCore`] instance from a [`CoreConfig`]. - pub fn builder() -> CliCoreBuilder { - CliCoreBuilder::new() +impl CMDKit { + /// Creates a [`CMDKit`] instance from a [`CoreConfig`]. + pub fn builder() -> CMDKitBuilder { + CMDKitBuilder::new() } /// Retrieves a registered command by name. - pub fn get(&self, name: &str) -> Option { + pub fn get(&self, name: &str) -> Option<&Command> { self.registry.get(name) } /// Returns all currently registered commands. - pub fn get_all(&self) -> Vec { + pub fn get_all(&self) -> Vec<&Command> { self.registry.get_all() } /// Runs the CLI with pre-built commands and prints user-facing errors. + /// + /// This is a convenience wrapper that does not surface registration failures + /// to the caller. Prefer [`CMDKit::try_run_with_commands`] when callers need + /// to handle command-registration collisions programmatically. pub fn run_with_commands(commands: &[Command]) { if let Err(e) = Self::try_run_with_commands(commands) { eprintln!("{e}"); @@ -212,9 +599,13 @@ impl CliCore { } /// Runs the CLI with pre-built commands and recoverable errors. - pub fn try_run_with_commands(commands: &[Command]) -> Result<(), CliCoreError> { + /// + /// This is the preferred entrypoint for embedding and library use because it + /// returns structured registration errors (for example alias/name collisions) + /// instead of panicking. + pub fn try_run_with_commands(commands: &[Command]) -> Result<(), CMDKitError> { Self::builder() - .with_commands(commands) + .try_with_commands(commands)? .build() .try_run_from_env() } @@ -228,7 +619,7 @@ impl CliCore { /// /// This is useful for tests and embedding scenarios where argument sources /// are not read from process environment. - pub fn try_run_from_args(&self, args: &[String]) -> Result<(), CliCoreError> { + pub fn try_run_from_args(&self, args: &[String]) -> Result<(), CMDKitError> { let binary = args .iter() .next() @@ -240,76 +631,273 @@ impl CliCore { return Ok(()); } - if args.len() < 2 { - return Err(CliCoreError::MissingCommand { - help: self.render_help(&binary), - }); - } - - let command_name = args[1].clone(); - let command_args = args.get(2..).unwrap_or(&[]).to_vec(); + let registered_commands = self.get_all(); + let invocation = self + .config + .argument_interpreter + .interpret(args.get(1..).unwrap_or(&[]), ®istered_commands[..]) + .map_err(|error| self.attach_help(error, &binary))?; let command = self - .get(&command_name) - .ok_or_else(|| CliCoreError::UnknownCommand { - command: command_name.clone(), + .resolve_registered_command(®istered_commands[..], &invocation.name) + .ok_or_else(|| CMDKitError::UnknownCommand { + command: invocation.name.clone(), help: self.render_help(&binary), })?; + let command_name = invocation.leaf_name().to_string(); + command - .execute(command_args) - .map_err(|source| CliCoreError::StrategyExecution { + .execute(&self.config.execution_context(), invocation) + .map_err(|source| CMDKitError::StrategyExecution { command: command_name, source, }) } fn render_help(&self, caller: &str) -> String { - self.config.help_renderer.render(caller, &self.get_all()) + let registered_commands = self.get_all(); + self.config + .help_renderer + .render(caller, ®istered_commands[..]) } - fn try_run_from_env(&self) -> Result<(), CliCoreError> { + fn try_run_from_env(&self) -> Result<(), CMDKitError> { let argv = std::env::args().collect::>(); self.try_run_from_args(&argv) } + + fn attach_help(&self, error: CMDKitError, caller: &str) -> CMDKitError { + match error { + CMDKitError::MissingCommand { .. } => CMDKitError::MissingCommand { + help: self.render_help(caller), + }, + CMDKitError::UnknownCommand { command, .. } => CMDKitError::UnknownCommand { + command, + help: self.render_help(caller), + }, + other => other, + } + } + + fn resolve_registered_command<'a>( + &self, + commands: &'a [&'a Command], + name: &str, + ) -> Option<&'a Command> { + commands.iter().find_map(|command| { + ((command.metadata.name == name) + || command.metadata.aliases.iter().any(|alias| alias == name)) + .then_some(*command) + }) + } } -pub struct CliCoreBuilder { +pub struct CMDKitBuilder { config: CoreConfig, - registry: CommandRegistry, + registry: CommandCatalogue, } -impl CliCoreBuilder { +impl CMDKitBuilder { pub fn with_config(mut self, config: CoreConfig) -> Self { self.config = config; self } /// Registers a command into this runtime instance. - pub fn register(mut self, command: Command) -> Self { - self.registry.register(command); - self + /// + /// Prefer [`CMDKitBuilder::try_register`] when command metadata can come from + /// external or dynamic sources. + /// + /// # Panics + /// Panics when registration fails (for example, alias/name collisions). + pub fn register(self, command: Command) -> Self { + self.try_register(command) + .expect("command registration should succeed") } - fn new() -> CliCoreBuilder { + /// Registers a command and returns a structured error on failure. + /// + /// This is the preferred registration API because command registration is + /// fallible: aliases and command names are validated for collisions. + pub fn try_register(mut self, command: Command) -> Result { + self.registry + .register(command) + .map_err(|message| CMDKitError::Registration { message })?; + Ok(self) + } + + fn new() -> CMDKitBuilder { Self { config: Default::default(), registry: Default::default(), } } - pub fn with_commands(mut self, commands: &[Command]) -> Self { + + /// Registers multiple commands into this runtime instance. + /// + /// Prefer [`CMDKitBuilder::try_with_commands`] in library and embedding + /// scenarios so registration failures can be handled by the caller. + /// + /// # Panics + /// Panics when any command registration fails (for example, alias/name + /// collisions). + pub fn with_commands(self, commands: &[Command]) -> Self { + self.try_with_commands(commands) + .expect("bulk command registration should succeed") + } + + /// Registers multiple commands and returns a structured error on failure. + /// + /// This is the preferred bulk-registration API because registration is + /// fallible and should generally be handled by the caller. + pub fn try_with_commands(mut self, commands: &[Command]) -> Result { for cmd in commands { - self.registry.register(cmd.clone()); + self.registry + .register(cmd.clone()) + .map_err(|message| CMDKitError::Registration { message })?; } + Ok(self) + } + + /// Replaces the argument interpreter for all commands built by this builder. + pub fn with_argument_interpreter(mut self, interpreter: I) -> Self + where + I: ArgumentInterpreter + 'static, + { + self.config.argument_interpreter = Arc::new(interpreter); + self + } + + pub fn with_logger(mut self, log_sink: L) -> Self + where + L: logging::LogSink + 'static, + { + self.config.logger = Arc::new(log_sink); self } - pub fn build(&self) -> CliCore { - CliCore { + /// Finalizes this builder into a reusable [`CMDKit`] runtime. + pub fn build(&self) -> CMDKit { + CMDKit { registry: self.registry.clone(), config: self.config.clone(), } } + + /// Creates a [`CMDKitMaster`] that submits invocations to worker threads. + /// + /// The returned master exposes async completion handles for submitted jobs. + /// `worker_count` values of `0` are normalized to `1` worker. + pub fn as_master_executor(&self, config: CoreConfig, worker_count: usize) -> CMDKitMaster { + CMDKitMaster::new(self.registry.clone(), config, worker_count) + } +} + +type WorkerResult = Result<(), CMDKitError>; + +/// A future handle that resolves when a submitted invocation completes. +/// +/// Awaiting this handle yields `Result, Canceled>` where: +/// - outer `Err(Canceled)` means the executor dropped the completion channel. +/// - outer `Ok(inner)` carries the command execution result. +pub type ExecutionHandle = oneshot::Receiver; + +struct QueuedInvocation { + args: Vec, + completion_tx: oneshot::Sender, +} + +/// Multi-worker command dispatcher that returns awaitable completion handles. +/// +/// `CMDKitMaster` accepts invocations immediately and executes them on background +/// worker threads using the registry snapshot from the originating builder. +pub struct CMDKitMaster { + submit_tx: mpsc::Sender, + config: CoreConfig, +} + +impl CMDKitMaster { + fn new(registry: CommandCatalogue, config: CoreConfig, worker_count: usize) -> Self { + let (submit_tx, submit_rx) = mpsc::channel::(); + let shared_rx = Arc::new(Mutex::new(submit_rx)); + + for _ in 0..worker_count.max(1) { + let rx = Arc::clone(&shared_rx); + let worker_registry = registry.clone(); + let worker_config = config.clone(); + + thread::spawn(move || { + let cmdkit = CMDKit { + registry: worker_registry, + config: worker_config, + }; + + loop { + let next_job = { + let guard = rx + .lock() + .expect("executor queue mutex should not be poisoned"); + guard.recv() + }; + + let Ok(job) = next_job else { + break; + }; + + let result = cmdkit.try_run_from_args(&job.args); + let _ = job.completion_tx.send(result); + } + }); + } + + Self { submit_tx, config } + } + + fn resolved_handle(result: WorkerResult) -> ExecutionHandle { + let (tx, rx) = oneshot::channel(); + let _ = tx.send(result); + rx + } + + fn dispatch(&self, args: &[String]) -> Result { + let (completion_tx, completion_rx) = oneshot::channel(); + self.submit_tx + .send(QueuedInvocation { + args: args.to_vec(), + completion_tx, + }) + .map_err(|_| CMDKitError::ExecutorUnavailable { + message: "worker queue is closed".to_string(), + })?; + + Ok(completion_rx) + } + + /// Submits an invocation for worker execution and returns its completion handle. + /// + /// This method is non-blocking with respect to command execution. Callers can + /// await the returned [`ExecutionHandle`] to observe completion. + pub fn try_run_from_args(&self, args: &[String]) -> Result { + let binary = args + .iter() + .next() + .cloned() + .unwrap_or_else(|| "cli".to_string()); + + if args.get(1).is_some_and(|arg| arg == "help") { + println!("{}", self.config.help_renderer.render(&binary, &[])); + return Ok(Self::resolved_handle(Ok(()))); + } + + self.dispatch(args) + } + + /// Submits process argv for worker execution and returns its completion handle. + pub fn try_run_from_env(&self) -> Result { + let argv = std::env::args().collect::>(); + self.try_run_from_args(&argv) + } } #[cfg(test)] diff --git a/src/core/tests.rs b/src/core/tests.rs index e9b1319..a00b8c4 100644 --- a/src/core/tests.rs +++ b/src/core/tests.rs @@ -1,4 +1,43 @@ -use crate::core::CoreConfig; +use std::sync::{Arc, Mutex}; + +use futures_executor::block_on; + +use crate::{ + ArgumentValue, CMDKit, CMDKitError, Command, CoreConfig, InvocationArgs, StrategyError, + argument, command, +}; + +struct MarkerHelpRenderer; + +impl crate::core::HelpRenderer for MarkerHelpRenderer { + fn render(&self, caller: &str, _registered_commands: &[&Command]) -> String { + format!("marker-help:{caller}") + } +} + +struct FixedInterpreter { + command_name: String, +} + +impl crate::core::ArgumentInterpreter for FixedInterpreter { + fn interpret( + &self, + _arg: &[String], + _registered_commands: &[&Command], + ) -> Result { + Ok(InvocationArgs { + name: self.command_name.clone(), + args: vec![ + argument("path", "path arg") + .set_value(ArgumentValue::String("from-fixed".to_string())), + ], + switches: Vec::new(), + params: Vec::new(), + order: Vec::new(), + subcommand: None, + }) + } +} #[test] fn core_config_defaults_to_plain_text_help_renderer() { @@ -6,3 +45,207 @@ fn core_config_defaults_to_plain_text_help_renderer() { let text = config.help_renderer.render("app", &[]); assert!(text.contains("Usage:")); } + +#[test] +fn core_config_can_replace_help_renderer() { + let config = CoreConfig::new().with_help_renderer(MarkerHelpRenderer); + let text = config.help_renderer.render("app", &[]); + assert_eq!(text, "marker-help:app"); +} + +#[test] +fn invocation_leaf_name_returns_deepest_nested_name() { + let invocation = InvocationArgs { + name: "root".to_string(), + args: Vec::new(), + switches: Vec::new(), + params: Vec::new(), + order: Vec::new(), + subcommand: Some(Box::new(InvocationArgs { + name: "child".to_string(), + args: Vec::new(), + switches: Vec::new(), + params: Vec::new(), + order: Vec::new(), + subcommand: Some(Box::new(InvocationArgs { + name: "leaf".to_string(), + args: Vec::new(), + switches: Vec::new(), + params: Vec::new(), + order: Vec::new(), + subcommand: None, + })), + })), + }; + + assert_eq!(invocation.leaf_name(), "leaf"); +} + +#[test] +fn cmdkit_error_display_and_source_are_structured() { + let missing = CMDKitError::MissingCommand { + help: "help text".to_string(), + }; + assert!(missing.to_string().contains("No command provided")); + assert!(std::error::Error::source(&missing).is_none()); + + let unknown = CMDKitError::UnknownCommand { + command: "ghost".to_string(), + help: "help text".to_string(), + }; + assert!(unknown.to_string().contains("Unknown command: ghost")); + assert!(std::error::Error::source(&unknown).is_none()); + + let strategy = StrategyError::execution("failed to execute"); + let strategy_execution = CMDKitError::StrategyExecution { + command: "run".to_string(), + source: strategy, + }; + assert!( + strategy_execution + .to_string() + .contains("Strategy execution failed for 'run': execution: failed to execute") + ); + assert!(std::error::Error::source(&strategy_execution).is_some()); +} + +#[test] +fn builder_argument_interpreter_can_drive_with_commands_registration() { + let calls: Arc>> = Arc::new(Mutex::new(Vec::new())); + let calls_for_handler = Arc::clone(&calls); + + let cmd = command("echo", "echo command") + .handler_fn(move |_, invocation| { + let arguments = invocation.args; + let value: String = match &arguments + .iter() + .find(|arg| arg.name == "path") + .expect("Couldn't find Argument with name : path") + .value + { + ArgumentValue::String(value) => value.to_string(), + _ => panic!(), + }; + calls_for_handler + .lock() + .expect("calls lock should not be poisoned") + .push(value); + Ok(()) + }) + .with_arguments(vec![argument("path", "path arg")]) + .build(); + + let core = CMDKit::builder() + .with_argument_interpreter(FixedInterpreter { + command_name: "echo".to_string(), + }) + .with_commands(&[cmd]) + .build(); + + assert!(core.try_run_from_args(&["app".to_string()]).is_ok()); + + let guard = calls.lock().expect("calls lock should not be poisoned"); + assert_eq!(guard.as_slice(), ["from-fixed"]); +} + +#[test] +fn builder_with_config_uses_custom_renderer_for_missing_command_help() { + let core = CMDKit::builder() + .with_config(CoreConfig::new().with_help_renderer(MarkerHelpRenderer)) + .build(); + + let result = core.try_run_from_args(&["custom".to_string()]); + + match result { + Err(CMDKitError::MissingCommand { help }) => { + assert_eq!(help, "marker-help:custom"); + } + _ => panic!("expected missing command error"), + } +} + +#[test] +fn cmdkit_master_executes_command_and_returns_success_handle() { + let calls: Arc>> = Arc::new(Mutex::new(Vec::new())); + let calls_for_handler = Arc::clone(&calls); + + let cmd = command("echo", "echo command").handler_fn(move |_, invocation| { + let params = invocation.params; + calls_for_handler + .lock() + .expect("calls lock should not be poisoned") + .push(params.join(" ")); + Ok(()) + }); + + let builder = CMDKit::builder().with_commands(&[cmd.build()]); + let master = builder.as_master_executor(CoreConfig::new(), 1); + + let handle = master + .try_run_from_args(&["app".to_string(), "echo".to_string(), "hello".to_string()]) + .expect("submission should succeed"); + + let result = block_on(handle).expect("completion channel should stay open"); + assert!(result.is_ok()); + + let guard = calls.lock().expect("calls lock should not be poisoned"); + assert_eq!(guard.as_slice(), ["hello"]); +} + +#[test] +fn cmdkit_master_propagates_execution_error_through_handle() { + let builder = CMDKit::builder(); + let master = builder.as_master_executor(CoreConfig::new(), 1); + + let handle = master + .try_run_from_args(&["app".to_string(), "missing".to_string()]) + .expect("submission should succeed"); + + let result = block_on(handle).expect("completion channel should stay open"); + match result { + Err(CMDKitError::UnknownCommand { command, .. }) => { + assert_eq!(command, "missing"); + } + _ => panic!("expected unknown command error"), + } +} + +#[test] +fn cmdkit_master_help_path_returns_resolved_success_handle() { + let builder = CMDKit::builder(); + let master = builder.as_master_executor(CoreConfig::new(), 1); + + let handle = master + .try_run_from_args(&["app".to_string(), "help".to_string()]) + .expect("help submission should succeed"); + + let result = block_on(handle).expect("completion channel should stay open"); + assert!(result.is_ok()); +} + +#[test] +fn cmdkit_master_normalizes_zero_worker_count() { + let calls: Arc>> = Arc::new(Mutex::new(Vec::new())); + let calls_for_handler = Arc::clone(&calls); + + let cmd = command("ping", "ping command").handler_fn(move |_, _| { + calls_for_handler + .lock() + .expect("calls lock should not be poisoned") + .push("ran".to_string()); + Ok(()) + }); + + let builder = CMDKit::builder().with_commands(&[cmd.build()]); + let master = builder.as_master_executor(CoreConfig::new(), 0); + + let handle = master + .try_run_from_args(&["app".to_string(), "ping".to_string()]) + .expect("submission should succeed"); + + let result = block_on(handle).expect("completion channel should stay open"); + assert!(result.is_ok()); + + let guard = calls.lock().expect("calls lock should not be poisoned"); + assert_eq!(guard.len(), 1); +} diff --git a/src/lib.rs b/src/lib.rs index 4758591..ed8396a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,18 +5,55 @@ pub mod cli; pub mod core; pub use cli::{ - Argument, Command, CommandBuilder, CommandMetaData, CommandStrategy, FunctionStrategy, - StrategyError, StrategyErrorKind, SubcommandCatalog, SubcommandRouter, Switch, argument, - command, switch, + Argument, ArgumentDefinition, ArgumentValue, Command, CommandBuilder, CommandMetaData, + CommandStrategy, FunctionStrategy, StrategyError, StrategyErrorKind, SubcommandCatalog, + SubcommandRouter, SwitchDefinition, argument, command, switch, +}; +pub use core::logging::{ConsoleLogSink, LogLevel, LogSink}; +pub use core::{ + ArgumentInterpreter, CMDKit, CMDKitBuilder, CMDKitError, CMDKitMaster, CoreConfig, + ExecutionContext, ExecutionHandle, HelpRenderer, InvocationArgs, InvocationElement, + PlainTextArgumentInterpreter, PlainTextHelpRenderer, }; -pub use core::{CliCore, CliCoreError, CoreConfig, HelpRenderer, PlainTextHelpRenderer}; -/// Runs a fresh default [`CliCore`] instance with pre-built commands. +/// Runs a fresh default [`CMDKit`] instance with pre-built commands. +/// +/// This is a convenience wrapper that prints errors. Prefer +/// [`try_run_with_commands`] when callers should handle registration failures +/// (such as alias/name collisions) programmatically. pub fn run_with_commands(commands: &[Command]) { - core::CliCore::run_with_commands(commands) + core::CMDKit::run_with_commands(commands) +} + +/// Runs a fresh default [`CMDKit`] instance with pre-built commands. +/// +/// This is the preferred wrapper for library use because it returns +/// [`CMDKitError`] instead of hiding failure paths. +pub fn try_run_with_commands(commands: &[Command]) -> Result<(), CMDKitError> { + core::CMDKit::try_run_with_commands(commands) } -/// Runs a fresh default [`CliCore`] instance with pre-built commands. -pub fn try_run_with_commands(commands: &[Command]) -> Result<(), CliCoreError> { - core::CliCore::try_run_with_commands(commands) +#[cfg(test)] +mod tests { + use super::{CMDKitError, run_with_commands}; + use crate::CMDKit; + + #[test] + fn wrapper_try_run_with_commands_propagates_missing_command_error() { + let result = CMDKit::builder() + .build() + .try_run_from_args(&["app".to_string()]); + + match result { + Err(CMDKitError::MissingCommand { help }) => { + assert!(help.contains("Usage:")); + } + _ => panic!("expected missing command error"), + } + } + + #[test] + fn wrapper_run_with_commands_handles_errors_without_panicking() { + run_with_commands(&[]); + } } diff --git a/tests/cli_suite.rs b/tests/cli_suite.rs index 599421f..53e4264 100644 --- a/tests/cli_suite.rs +++ b/tests/cli_suite.rs @@ -1,6 +1,6 @@ use std::sync::atomic::{AtomicUsize, Ordering}; -use cmdkit::{Argument, CliCore, Command, CommandStrategy, StrategyError, Switch}; +use cmdkit::{CMDKit, Command, CommandStrategy, StrategyError, core::InvocationArgs}; struct TestStrategy; struct TestStrategyV2; @@ -15,9 +15,8 @@ fn unique_name(prefix: &str) -> String { impl CommandStrategy for TestStrategy { fn execute( &self, - _options: Vec, - _arguments: Vec, - _params: Vec, + _context: &cmdkit::ExecutionContext, + _invocation: InvocationArgs, ) -> Result<(), StrategyError> { println!("Test strategy executed"); Ok(()) @@ -27,9 +26,8 @@ impl CommandStrategy for TestStrategy { impl CommandStrategy for TestStrategyV2 { fn execute( &self, - _options: Vec, - _arguments: Vec, - _params: Vec, + _context: &cmdkit::ExecutionContext, + _invocation: InvocationArgs, ) -> Result<(), StrategyError> { println!("Test strategy v2 executed"); Ok(()) @@ -41,7 +39,7 @@ fn test_register_and_get() { let name = unique_name("register_get"); let functionality = Command::new(name.clone(), "A test functionality", TestStrategy); - let core = CliCore::builder().register(functionality.clone()).build(); + let core = CMDKit::builder().register(functionality.clone()).build(); let retrieved = core.get(&name).expect("Functionality should be registered"); assert_eq!(retrieved.metadata.name, functionality.metadata.name); assert_eq!( @@ -55,14 +53,14 @@ fn test_get_all() { let name = unique_name("get_all"); let functionality = Command::new(name.clone(), "A test functionality", TestStrategy); - let core = CliCore::builder().register(functionality.clone()).build(); + let core = CMDKit::builder().register(functionality.clone()).build(); let all = core.get_all(); assert!(all.iter().any(|f| f.metadata.name == name)); } #[test] fn test_non_existent() { - let core = CliCore::builder().build(); + let core = CMDKit::builder().build(); let result = core.get("nonexistent"); assert!( result.is_none(), @@ -73,7 +71,7 @@ fn test_non_existent() { #[test] fn test_register_duplicate_name_overwrites() { let name = unique_name("duplicate"); - let core = CliCore::builder() + let core = CMDKit::builder() .register(Command::new(name.clone(), "original", TestStrategy)) .register(Command::new(name.clone(), "updated", TestStrategyV2)) .build(); @@ -84,10 +82,10 @@ fn test_register_duplicate_name_overwrites() { #[test] fn test_instances_are_isolated() { - let core_b = CliCore::builder().build(); + let core_b = CMDKit::builder().build(); let name = unique_name("isolated"); - let core_a = CliCore::builder() + let core_a = CMDKit::builder() .register(Command::new(name.clone(), "in a", TestStrategy)) .build(); diff --git a/tests/clicore_suite.rs b/tests/clicore_suite.rs index d290be2..76ce5bd 100644 --- a/tests/clicore_suite.rs +++ b/tests/clicore_suite.rs @@ -1,10 +1,11 @@ use std::sync::{Arc, Mutex}; use cmdkit::{ - Argument, CliCore, CliCoreError, Command, CommandStrategy, StrategyError, StrategyErrorKind, - SubcommandRouter, Switch, argument, command, switch, + Argument, ArgumentInterpreter, ArgumentValue, CMDKit, CMDKitError, Command, CommandStrategy, + InvocationArgs, InvocationElement, PlainTextArgumentInterpreter, StrategyError, + StrategyErrorKind, SubcommandRouter, argument, cli, command, switch, }; -type CallLog = Arc, Vec, Vec)>>>; +type CallLog = Arc, Vec, Vec)>>>; struct RecorderStrategy { calls: CallLog, @@ -14,12 +15,11 @@ struct RecorderStrategy { impl CommandStrategy for RecorderStrategy { fn execute( &self, - options: Vec, - arguments: Vec, - params: Vec, + _context: &cmdkit::ExecutionContext, + invocation: InvocationArgs, ) -> Result<(), StrategyError> { let mut guard = self.calls.lock().expect("call log lock poisoned"); - guard.push((options, arguments, params)); + guard.push((invocation.switches, invocation.args, invocation.params)); if let Some(err) = &self.error { return Err(err.clone()); @@ -38,22 +38,27 @@ fn build_recorder_functionality( Command::new(name, description, RecorderStrategy { calls, error }) } -fn has_switch(switches: &[Switch], name: &str) -> bool { - switches.iter().any(|switch| switch.name == name) +fn has_switch(switches: &[String], name: &str) -> bool { + switches.iter().any(|switch| switch == name) } fn argument_value<'a>(arguments: &'a [Argument], name: &str) -> Option<&'a str> { - arguments + let value = &arguments .iter() .find(|argument| argument.name == name) - .and_then(|argument| argument.value.as_deref()) + .expect("argument should be found in the strategy call log") // For test purposes, we expect the argument to be present + .value; + match value { + cmdkit::ArgumentValue::String(value) => Some(value.as_str()), + _ => None, + } } #[test] fn register_and_get_by_name_works() { let calls = Arc::new(Mutex::new(Vec::new())); - let core = CliCore::builder() + let core = CMDKit::builder() .register(build_recorder_functionality( "echo", "echo arguments", @@ -73,7 +78,7 @@ fn register_and_get_by_name_works() { fn duplicate_registration_overwrites_previous_entry() { let calls = Arc::new(Mutex::new(Vec::new())); - let core = CliCore::builder() + let core = CMDKit::builder() .register(build_recorder_functionality( "dup", "first", @@ -92,11 +97,158 @@ fn duplicate_registration_overwrites_previous_entry() { assert_eq!(got.metadata.description, "second"); } +#[test] +fn top_level_command_alias_invocation_succeeds() { + let calls = Arc::new(Mutex::new(Vec::new())); + + let core = CMDKit::builder() + .register( + command("build", "build project") + .with_aliases(vec!["b", "compile"]) + .handler(RecorderStrategy { + calls: Arc::clone(&calls), + error: None, + }) + .build(), + ) + .build(); + + let args = vec!["app".to_string(), "b".to_string()]; + assert!(core.try_run_from_args(&args).is_ok()); + + let guard = calls.lock().expect("call log lock poisoned"); + assert_eq!(guard.len(), 1); +} + +#[test] +fn nested_subcommand_alias_invocation_succeeds() { + let calls = Arc::new(Mutex::new(Vec::new())); + let calls_for_strategy = Arc::clone(&calls); + + let core = CMDKit::builder() + .register( + command("tool", "tool root") + .subcommand( + command("run", "run command") + .with_aliases(vec!["r"]) + .handler_fn(move |_, invocation| { + let options = invocation.switches; + let arguments = invocation.args; + let params = invocation.params; + let mut guard = + calls_for_strategy.lock().expect("call log lock poisoned"); + guard.push((options, arguments, params)); + Ok(()) + }) + .build(), + ) + .build(), + ) + .build(); + + let args = vec!["app".to_string(), "tool".to_string(), "r".to_string()]; + assert!(core.try_run_from_args(&args).is_ok()); + + let guard = calls.lock().expect("call log lock poisoned"); + assert_eq!(guard.len(), 1); +} + +#[test] +fn registration_rejects_alias_conflicting_with_existing_command_name() { + let result = CMDKit::builder() + .try_register(command("build", "build").handler_fn(|_, _| Ok(())).build()) + .and_then(|builder| { + builder.try_register( + command("deploy", "deploy") + .with_aliases(vec!["build"]) + .handler_fn(|_, _| Ok(())) + .build(), + ) + }); + + match result { + Err(CMDKitError::Registration { message }) => { + assert!(message.contains("alias 'build' conflicts with existing command name 'build'")); + } + _ => panic!("expected registration collision error"), + } +} + +#[test] +fn registration_rejects_alias_conflicting_with_existing_alias() { + let result = CMDKit::builder() + .try_register( + command("build", "build") + .with_aliases(vec!["b"]) + .handler_fn(|_, _| Ok(())) + .build(), + ) + .and_then(|builder| { + builder.try_register( + command("bundle", "bundle") + .with_aliases(vec!["b"]) + .handler_fn(|_, _| Ok(())) + .build(), + ) + }); + + match result { + Err(CMDKitError::Registration { message }) => { + assert!(message.contains("alias 'b' conflicts with existing alias owned by 'build'")); + } + _ => panic!("expected registration collision error"), + } +} + +#[test] +fn registration_rejects_command_name_conflicting_with_existing_alias() { + let result = CMDKit::builder() + .try_register( + command("build", "build") + .with_aliases(vec!["b"]) + .handler_fn(|_, _| Ok(())) + .build(), + ) + .and_then(|builder| { + builder.try_register(command("b", "b cmd").handler_fn(|_, _| Ok(())).build()) + }); + + match result { + Err(CMDKitError::Registration { message }) => { + assert!( + message.contains("command name 'b' conflicts with existing alias owned by 'build'") + ); + } + _ => panic!("expected registration collision error"), + } +} + +#[test] +fn bulk_registration_rejects_collisions() { + let cmd_a = command("build", "build") + .with_aliases(vec!["b"]) + .handler_fn(|_, _| Ok(())) + .build(); + let cmd_b = command("bundle", "bundle") + .with_aliases(vec!["b"]) + .handler_fn(|_, _| Ok(())) + .build(); + + let result = CMDKit::builder().try_with_commands(&[cmd_a, cmd_b]); + + match result { + Err(CMDKitError::Registration { message }) => { + assert!(message.contains("alias 'b' conflicts with existing alias owned by 'build'")); + } + _ => panic!("expected registration collision error"), + } +} + #[test] fn run_from_args_routes_trailing_arguments_to_strategy() { let calls = Arc::new(Mutex::new(Vec::new())); - let core = CliCore::builder() + let core = CMDKit::builder() .register( command("echo", "echo arguments") .handler(RecorderStrategy { @@ -131,7 +283,7 @@ fn run_from_args_routes_trailing_arguments_to_strategy() { fn strategy_receives_subtask_tokens_for_chain_of_responsibility() { let calls = Arc::new(Mutex::new(Vec::new())); - let core = CliCore::builder() + let core = CMDKit::builder() .register( command("test", "test root") .handler(RecorderStrategy { @@ -167,10 +319,13 @@ fn functionality_from_fn_supports_function_based_strategy_registration() { let calls = Arc::new(Mutex::new(Vec::new())); let calls_for_strategy = Arc::clone(&calls); - let core = CliCore::builder() + let core = CMDKit::builder() .register( command("fncmd", "defined from a function") - .handler_fn(move |options, arguments, params| { + .handler_fn(move |_, invocation| { + let options = invocation.switches; + let arguments = invocation.args; + let params = invocation.params; let mut guard = calls_for_strategy.lock().expect("call log lock poisoned"); guard.push((options, arguments, params)); Ok(()) @@ -197,13 +352,13 @@ fn functionality_from_fn_supports_function_based_strategy_registration() { #[test] fn run_from_args_returns_missing_command_error() { - let core = CliCore::builder(); + let core = CMDKit::builder(); let args = vec!["app".to_string()]; let result = core.build().try_run_from_args(&args); match result { - Err(CliCoreError::MissingCommand { help }) => { + Err(CMDKitError::MissingCommand { help }) => { assert!(help.contains("Usage:")); } _ => panic!("expected missing command error"), @@ -212,13 +367,13 @@ fn run_from_args_returns_missing_command_error() { #[test] fn help_text_uses_explicit_binary_name_for_missing_command() { - let core = CliCore::builder(); + let core = CMDKit::builder(); let args = vec!["custom-cli".to_string()]; let result = core.build().try_run_from_args(&args); match result { - Err(CliCoreError::MissingCommand { help }) => { + Err(CMDKitError::MissingCommand { help }) => { assert!(help.contains("Usage: custom-cli [args...]")); } _ => panic!("expected missing command error"), @@ -227,13 +382,13 @@ fn help_text_uses_explicit_binary_name_for_missing_command() { #[test] fn help_text_uses_explicit_binary_name_for_unknown_command() { - let core = CliCore::builder(); + let core = CMDKit::builder(); let args = vec!["custom-cli".to_string(), "not-a-command".to_string()]; let result = core.build().try_run_from_args(&args); match result { - Err(CliCoreError::UnknownCommand { help, .. }) => { + Err(CMDKitError::UnknownCommand { help, .. }) => { assert!(help.contains("Usage: custom-cli [args...]")); } _ => panic!("expected unknown command error"), @@ -242,13 +397,13 @@ fn help_text_uses_explicit_binary_name_for_unknown_command() { #[test] fn run_from_args_returns_unknown_command_error() { - let core = CliCore::builder().build(); + let core = CMDKit::builder().build(); let args = vec!["app".to_string(), "unknown".to_string()]; let result = core.try_run_from_args(&args); match result { - Err(CliCoreError::UnknownCommand { command, help }) => { + Err(CMDKitError::UnknownCommand { command, help }) => { assert_eq!(command, "unknown"); assert!(help.contains("supported commands:")); } @@ -260,7 +415,7 @@ fn run_from_args_returns_unknown_command_error() { fn strategy_errors_bubble_with_kind_and_message() { let calls = Arc::new(Mutex::new(Vec::new())); - let core = CliCore::builder() + let core = CMDKit::builder() .register(build_recorder_functionality( "validate", "fails validation", @@ -276,7 +431,7 @@ fn strategy_errors_bubble_with_kind_and_message() { let result = core.try_run_from_args(&args); match result { - Err(CliCoreError::StrategyExecution { command, source }) => { + Err(CMDKitError::StrategyExecution { command, source }) => { assert_eq!(command, "validate"); assert_eq!(source.kind, StrategyErrorKind::InvalidArguments); assert_eq!(source.message, "missing value"); @@ -286,10 +441,10 @@ fn strategy_errors_bubble_with_kind_and_message() { } #[test] -fn parser_rejects_unknown_flags() { +fn interpreter_rejects_unknown_flags() { let calls = Arc::new(Mutex::new(Vec::new())); - let core = CliCore::builder() + let core = CMDKit::builder() .register( command("strict", "strict command") .handler(RecorderStrategy { @@ -310,7 +465,7 @@ fn parser_rejects_unknown_flags() { let result = core.try_run_from_args(&args); match result { - Err(CliCoreError::StrategyExecution { command, source }) => { + Err(CMDKitError::StrategyExecution { command, source }) => { assert_eq!(command, "strict"); assert_eq!(source.kind, StrategyErrorKind::InvalidArguments); assert!(source.message.contains("unknown flag '--unknown'")); @@ -320,10 +475,10 @@ fn parser_rejects_unknown_flags() { } #[test] -fn parser_enforces_required_argument_presence_and_non_empty_values() { +fn interpreter_enforces_required_argument_presence_and_non_empty_values() { let calls = Arc::new(Mutex::new(Vec::new())); - let core = CliCore::builder() + let core = CMDKit::builder() .register( command("strict", "strict command") .handler(RecorderStrategy { @@ -338,7 +493,7 @@ fn parser_enforces_required_argument_presence_and_non_empty_values() { let missing_value = vec!["app".to_string(), "strict".to_string()]; let missing_result = core.try_run_from_args(&missing_value); match missing_result { - Err(CliCoreError::StrategyExecution { source, .. }) => { + Err(CMDKitError::StrategyExecution { source, .. }) => { assert_eq!(source.kind, StrategyErrorKind::InvalidArguments); assert!( source @@ -356,7 +511,7 @@ fn parser_enforces_required_argument_presence_and_non_empty_values() { ]; let inline_empty_result = core.try_run_from_args(&inline_empty); match inline_empty_result { - Err(CliCoreError::StrategyExecution { source, .. }) => { + Err(CMDKitError::StrategyExecution { source, .. }) => { assert_eq!(source.kind, StrategyErrorKind::InvalidArguments); assert!( source @@ -375,7 +530,7 @@ fn parser_enforces_required_argument_presence_and_non_empty_values() { ]; let next_empty_result = core.try_run_from_args(&next_empty); match next_empty_result { - Err(CliCoreError::StrategyExecution { source, .. }) => { + Err(CMDKitError::StrategyExecution { source, .. }) => { assert_eq!(source.kind, StrategyErrorKind::InvalidArguments); assert!( source @@ -388,14 +543,17 @@ fn parser_enforces_required_argument_presence_and_non_empty_values() { } #[test] -fn parser_accepts_argument_aliases_and_uses_last_value_wins() { +fn interpreter_accepts_argument_aliases_and_uses_last_value_wins() { let calls = Arc::new(Mutex::new(Vec::new())); let calls_for_strategy = Arc::clone(&calls); - let core = CliCore::builder() + let core = CMDKit::builder() .register( command("alias", "alias command") - .handler_fn(move |options, arguments, params| { + .handler_fn(move |_, invocation| { + let options = invocation.switches; + let arguments = invocation.args; + let params = invocation.params; let mut guard = calls_for_strategy.lock().expect("call log lock poisoned"); guard.push((options, arguments, params)); Ok(()) @@ -422,15 +580,19 @@ fn parser_accepts_argument_aliases_and_uses_last_value_wins() { assert_eq!(guard.len(), 1); assert_eq!(guard[0].1.len(), 1); assert_eq!(guard[0].1[0].name, "path"); - assert_eq!(guard[0].1[0].value.as_deref(), Some("second")); + + match &guard[0].1[0].value { + ArgumentValue::String(value) => assert_eq!(value, "second"), + _ => panic!("expected value string"), + } } #[test] fn independent_instances_do_not_share_registry_entries() { - let core_b = CliCore::builder().build(); + let core_b = CMDKit::builder().build(); let calls = Arc::new(Mutex::new(Vec::new())); - let core_a = CliCore::builder() + let core_a = CMDKit::builder() .register(build_recorder_functionality( "isolated", "only in core_a", @@ -443,13 +605,326 @@ fn independent_instances_do_not_share_registry_entries() { assert!(core_b.get("isolated").is_none()); } +#[test] +fn configured_argument_interpreter_can_drive_invocation() { + struct FixedInterpreter; + + use cli::ArgumentValue; + impl ArgumentInterpreter for FixedInterpreter { + fn interpret( + &self, + _arg: &[String], + _registered_commands: &[&Command], + ) -> Result { + Ok(cmdkit::InvocationArgs { + name: "echo".to_string(), + args: vec![ + argument("path", "path value") + .set_value(ArgumentValue::String("from-interpreter".to_string())), + ], + switches: vec!["loud".to_string(), "loud switch".to_string()], + params: vec!["tail".to_string()], + order: vec![ + InvocationElement::Switch("loud".to_string()), + InvocationElement::Switch("loud switch".to_string()), + InvocationElement::Argument( + argument("path", "path value") + .set_value(ArgumentValue::String("from-interpreter".to_string())), + ), + InvocationElement::Param("tail".to_string()), + ], + subcommand: None, + }) + } + } + + let calls = Arc::new(Mutex::new(Vec::new())); + let core = CMDKit::builder() + .with_argument_interpreter(FixedInterpreter) + .register( + command("echo", "echo arguments") + .handler(RecorderStrategy { + calls: Arc::clone(&calls), + error: None, + }) + .with_options(vec![switch("loud", "loud switch")]) + .with_arguments(vec![argument("path", "path value")]) + .build(), + ) + .build(); + + assert!(core.try_run_from_args(&["app".to_string()]).is_ok()); + + let guard = calls.lock().expect("call log lock poisoned"); + assert_eq!(guard.len(), 1); + assert!(has_switch(&guard[0].0, "loud")); + assert_eq!( + argument_value(&guard[0].1, "path"), + Some("from-interpreter") + ); + assert_eq!(guard[0].2, vec!["tail".to_string()]); +} + +#[test] +fn configured_argument_interpreter_rejects_unresolvable_subcommand_path() { + struct InvalidSubcommandInterpreter; + + impl ArgumentInterpreter for InvalidSubcommandInterpreter { + fn interpret( + &self, + _arg: &[String], + _registered_commands: &[&Command], + ) -> Result { + Ok(cmdkit::InvocationArgs { + name: "echo".to_string(), + args: Vec::new(), + switches: Vec::new(), + params: Vec::new(), + order: Vec::new(), + subcommand: Some(Box::new(cmdkit::InvocationArgs { + name: "ghost".to_string(), + args: Vec::new(), + switches: Vec::new(), + params: Vec::new(), + order: Vec::new(), + subcommand: None, + })), + }) + } + } + + let calls = Arc::new(Mutex::new(Vec::new())); + let core = CMDKit::builder() + .with_argument_interpreter(InvalidSubcommandInterpreter) + .register( + command("echo", "echo arguments") + .handler(RecorderStrategy { + calls: Arc::clone(&calls), + error: None, + }) + .build(), + ) + .build(); + + let result = core.try_run_from_args(&["app".to_string()]); + match result { + Err(CMDKitError::StrategyExecution { source, .. }) => { + assert_eq!(source.kind, StrategyErrorKind::InvalidArguments); + assert!(source.message.contains("unknown subcommand 'ghost'")); + } + _ => panic!("expected invalid subcommand path error"), + } + + let guard = calls.lock().expect("call log lock poisoned"); + assert!(guard.is_empty()); +} + +#[test] +fn plain_text_interpreter_returns_missing_command_without_registered_input() { + let interpreter = PlainTextArgumentInterpreter; + + let result = interpreter.interpret(&[], &[]); + + match result { + Err(CMDKitError::MissingCommand { help }) => { + assert!(help.is_empty()); + } + _ => panic!("expected missing command interpreter error"), + } +} + +#[test] +fn plain_text_interpreter_returns_unknown_command_for_unregistered_name() { + let interpreter = PlainTextArgumentInterpreter; + + let result = interpreter.interpret(&["missing".to_string()], &[]); + + match result { + Err(CMDKitError::UnknownCommand { command, help }) => { + assert_eq!(command, "missing"); + assert!(help.is_empty()); + } + _ => panic!("expected unknown command interpreter error"), + } +} + +#[test] +fn plain_text_interpreter_resolves_aliases_to_canonical_typed_metadata() { + let interpreter = PlainTextArgumentInterpreter; + let root = command("tool", "tool root") + .with_aliases(vec!["t"]) + .with_options(vec![ + switch("verbose", "verbose output").with_aliases(vec!["v".to_string()]), + ]) + .with_arguments(vec![argument("path", "path value").with_aliases(vec!["p"])]) + .handler_fn(|_, _| Ok(())) + .build(); + + let invocation = interpreter + .interpret( + &[ + "t".to_string(), + "--v".to_string(), + "--p".to_string(), + "src/lib.rs".to_string(), + ], + &[&root], + ) + .expect("interpreter should resolve aliases using declared metadata"); + + assert_eq!(invocation.name, "tool"); + assert_eq!(invocation.switches.len(), 1); + assert_eq!(invocation.switches[0], "verbose"); + + assert_eq!(invocation.args.len(), 1); + assert_eq!(invocation.args[0].name, "path"); + + match &invocation.args[0].value { + ArgumentValue::String(value) => assert_eq!(value, "src/lib.rs"), + _ => panic!("expected argument value to be a string"), + } +} + +#[test] +fn plain_text_interpreter_moves_repeated_argument_to_latest_position() { + let interpreter = PlainTextArgumentInterpreter; + let root = command("tool", "tool root") + .with_options(vec![switch("verbose", "verbose output")]) + .with_arguments(vec![ + argument("path", "path value"), + argument("mode", "mode value"), + ]) + .handler_fn(|_, _| Ok(())) + .build(); + + let invocation = interpreter + .interpret( + &[ + "tool".to_string(), + "--path".to_string(), + "first".to_string(), + "--mode".to_string(), + "fast".to_string(), + "--path".to_string(), + "second".to_string(), + "--verbose".to_string(), + ], + &[&root], + ) + .expect("interpreter should keep the latest argument occurrence"); + + assert_eq!(invocation.args.len(), 2); + assert_eq!(invocation.args[0].name, "mode"); + match &invocation.args[0].value { + ArgumentValue::String(value) => assert_eq!(value, "fast"), + _ => panic!("expected argument value to be a string"), + } + + assert_eq!(invocation.args[1].name, "path"); + match &invocation.args[1].value { + ArgumentValue::String(value) => assert_eq!(value, "second"), + _ => panic!("expected argument value to be a string"), + } + + assert_eq!(invocation.order.len(), 4); + + assert!(matches!( + &invocation.order[0], + InvocationElement::Argument(argument_decl) + if argument_decl.name == "path" && get_value(&argument_decl.value) == "first" + )); + assert!(matches!( + &invocation.order[1], + InvocationElement::Argument(argument_decl) + if argument_decl.name == "mode" && get_value(&argument_decl.value) == "fast" + )); + assert!(matches!( + &invocation.order[2], + InvocationElement::Argument(argument_decl) + if argument_decl.name == "path" && get_value(&argument_decl.value) == "second" + )); + assert!(matches!( + &invocation.order[3], + InvocationElement::Switch(switch_decl) if switch_decl == "verbose" + )); +} + +fn get_value(arg: &ArgumentValue) -> String { + match arg { + ArgumentValue::String(s) => s.to_string(), + _ => panic!("expected argument value to be an array"), + } +} +#[test] +fn plain_text_interpreter_preserves_typed_argument_order() { + let interpreter = PlainTextArgumentInterpreter; + let root = command("tool", "tool root") + .with_options(vec![switch("verbose", "verbose output")]) + .with_arguments(vec![argument("path", "path value")]) + .subcommand( + command("child", "child command") + .with_options(vec![switch("force", "force switch")]) + .with_arguments(vec![argument("mode", "mode value")]) + .handler_fn(|_, _| Ok(())) + .build(), + ) + .handler_fn(|_, _| Ok(())) + .build(); + + let invocation = interpreter + .interpret( + &[ + "tool".to_string(), + "--verbose".to_string(), + "--path".to_string(), + "src".to_string(), + "child".to_string(), + "--force".to_string(), + "--mode=fast".to_string(), + "tail".to_string(), + ], + &[&root], + ) + .expect("interpreter should parse argv-style input"); + + assert_eq!(invocation.name, "tool"); + assert_eq!(invocation.order.len(), 2); + assert!(matches!( + &invocation.order[0], + InvocationElement::Switch(switch_decl) if switch_decl == "verbose" + )); + assert!(matches!( + &invocation.order[1], + InvocationElement::Argument(argument_decl) + if argument_decl.name == "path" && get_value(&argument_decl.value) == "src" + )); + + let child = invocation + .subcommand + .expect("subcommand invocation should exist"); + assert_eq!(child.name, "child"); + assert_eq!(child.params, vec!["tail".to_string()]); + assert!(matches!( + &child.order[0], + InvocationElement::Switch(switch_decl) if switch_decl == "force" + )); + assert!(matches!( + &child.order[1], + InvocationElement::Argument(argument_decl) + if argument_decl.name == "mode" && get_value(&argument_decl.value) == "fast" + )); +} + #[test] fn subcommand_router_dispatches_recursively_to_deep_children() { let deep_calls = Arc::new(Mutex::new(Vec::new())); let deep_calls_for_leaf = Arc::clone(&deep_calls); let deep_leaf = command("leaf", "deep leaf") - .handler_fn(move |options, arguments, params| { + .handler_fn(move |_, invocation| { + let options = invocation.switches; + let arguments = invocation.args; + let params = invocation.params; deep_calls_for_leaf .lock() .expect("call log lock poisoned") @@ -475,7 +950,7 @@ fn subcommand_router_dispatches_recursively_to_deep_children() { )), ); - let core = CliCore::builder().register(root).build(); + let core = CMDKit::builder().register(root).build(); let args = vec![ "app".to_string(), @@ -502,7 +977,10 @@ fn subcommand_boundary_defers_flag_parsing_to_child_command() { let child_calls_for_strategy = Arc::clone(&child_calls); let run = command("run", "run command") - .handler_fn(move |options, arguments, params| { + .handler_fn(move |_, invocation| { + let options = invocation.switches; + let arguments = invocation.args; + let params = invocation.params; child_calls_for_strategy .lock() .expect("call log lock poisoned") @@ -513,7 +991,7 @@ fn subcommand_boundary_defers_flag_parsing_to_child_command() { .build(); let root = Command::new("tool", "tool root", SubcommandRouter::new().register(run)); - let core = CliCore::builder().register(root).build(); + let core = CMDKit::builder().register(root).build(); let args = vec![ "app".to_string(), @@ -535,10 +1013,10 @@ fn subcommand_boundary_defers_flag_parsing_to_child_command() { #[test] fn positional_tokens_before_subcommand_boundary_remain_current_level_params() { let run = command("run", "run command") - .handler_fn(|_, _, _| Ok(())) + .handler_fn(|_, _| Ok(())) .build(); let root = Command::new("tool", "tool root", SubcommandRouter::new().register(run)); - let core = CliCore::builder().register(root).build(); + let core = CMDKit::builder().register(root).build(); let args = vec![ "app".to_string(), @@ -549,7 +1027,7 @@ fn positional_tokens_before_subcommand_boundary_remain_current_level_params() { let result = core.try_run_from_args(&args); match result { - Err(CliCoreError::StrategyExecution { source, .. }) => { + Err(CMDKitError::StrategyExecution { source, .. }) => { assert_eq!(source.kind, StrategyErrorKind::InvalidArguments); assert!(source.message.contains("unknown subcommand 'pre'")); } @@ -565,21 +1043,18 @@ fn help_renderer_includes_recursive_subcommands_from_router_catalog() { SubcommandRouter::new().register(Command::new( "child", "child command", - SubcommandRouter::new().register(Command::from_fn( - "leaf", - "leaf command", - |_, _, _| Ok(()), - )), + SubcommandRouter::new() + .register(Command::from_fn("leaf", "leaf command", |_, _| Ok(()))), )), ); - let core = CliCore::builder().register(root).build(); + let core = CMDKit::builder().register(root).build(); let args = vec!["app".to_string()]; let result = core.try_run_from_args(&args); match result { - Err(CliCoreError::MissingCommand { help }) => { + Err(CMDKitError::MissingCommand { help }) => { assert!(help.contains("tool: tool root")); assert!(help.contains("tool child: child command")); assert!(help.contains("tool child leaf: leaf command")); @@ -590,26 +1065,27 @@ fn help_renderer_includes_recursive_subcommands_from_router_catalog() { #[test] fn help_renderer_includes_nested_catalogs_hidden_by_fallback_wrappers() { - let root = command("tool", "tool root") - .handler(SubcommandRouter::new().register(Command::new( - "inner", - "inner branch", - SubcommandRouter::new().register(Command::from_fn( - "leaf", - "leaf command", - |_, _, _| Ok(()), - )), - ))) - .subcommand(command("outer", "outer command").handler_fn(|_, _, _| Ok(()))) - .build(); - - let core = CliCore::builder().register(root).build(); + let root = + command("tool", "tool root") + .handler(SubcommandRouter::new().register(Command::new( + "inner", + "inner branch", + SubcommandRouter::new().register(Command::from_fn( + "leaf", + "leaf command", + |_, _| Ok(()), + )), + ))) + .subcommand(command("outer", "outer command").handler_fn(|_, _| Ok(()))) + .build(); + + let core = CMDKit::builder().register(root).build(); let args = vec!["app".to_string()]; let result = core.try_run_from_args(&args); match result { - Err(CliCoreError::MissingCommand { help }) => { + Err(CMDKitError::MissingCommand { help }) => { assert!(help.contains("tool outer: outer command")); assert!(help.contains("tool inner: inner branch")); assert!(help.contains("tool inner leaf: leaf command")); @@ -621,7 +1097,7 @@ fn help_renderer_includes_nested_catalogs_hidden_by_fallback_wrappers() { #[test] fn help_renderer_includes_optional_metadata_fields() { let cmd = command("build", "Build a project") - .handler_fn(|_, _, _| Ok(())) + .handler_fn(|_, _| Ok(())) .with_aliases(vec!["b", "compile"]) .with_usage("build --path [--release]") .with_long_description("Builds the project artifacts for distribution") @@ -639,13 +1115,13 @@ fn help_renderer_includes_optional_metadata_fields() { ]) .build(); - let core = CliCore::builder().register(cmd).build(); + let core = CMDKit::builder().register(cmd).build(); let args = vec!["app".to_string()]; let result = core.try_run_from_args(&args); match result { - Err(CliCoreError::MissingCommand { help }) => { + Err(CMDKitError::MissingCommand { help }) => { assert!(help.contains("- build: Build a project")); assert!(help.contains("usage: build --path [--release]")); assert!(help.contains("details: Builds the project artifacts for distribution")); diff --git a/tests/command_suit.rs b/tests/command_suit.rs index e84f8aa..aa68403 100644 --- a/tests/command_suit.rs +++ b/tests/command_suit.rs @@ -1,24 +1,19 @@ use std::sync::{Arc, Mutex}; -use cmdkit::{Argument, CliCore, Switch, argument, command, switch}; +use cmdkit::{Argument, ArgumentValue, CMDKit, argument, command, switch}; -fn format_switches(switches: &[Switch]) -> String { - switches - .iter() - .map(|switch| switch.name.clone()) - .collect::>() - .join(",") +fn format_switches(switches: &[String]) -> String { + switches.to_vec().join(",") } fn format_arguments(arguments: &[Argument]) -> String { arguments .iter() - .map(|argument| { - format!( - "{}={}", - argument.name, - argument.value.clone().unwrap_or_default() - ) + .map(|argument| match argument.value { + ArgumentValue::String(ref value) => format!("{}={value}", argument.name), + ArgumentValue::Int(value) => format!("{}={value}", argument.name), + ArgumentValue::Float(value) => format!("{}={value}", argument.name), + _ => argument.name.to_string(), }) .collect::>() .join(",") @@ -28,10 +23,13 @@ fn format_arguments(arguments: &[Argument]) -> String { fn strategy_chain_handles_subtask_tokens() { let captured = Arc::new(Mutex::new(Vec::new())); let captured_for_strategy = Arc::clone(&captured); - let core = CliCore::builder() + let core = CMDKit::builder() .register( command("parent", "Parent command") - .handler_fn(move |options, arguments, params| { + .handler_fn(move |_, invocation| { + let options = invocation.switches; + let arguments = invocation.args; + let params = invocation.params; let mut guard = captured_for_strategy .lock() .expect("capture lock should not be poisoned"); @@ -71,10 +69,13 @@ fn command_builder_registers_leaf_command_without_exposing_strategy_types() { let captured = Arc::new(Mutex::new(Vec::new())); let captured_for_handler = Arc::clone(&captured); - let core = CliCore::builder() + let core = CMDKit::builder() .register( command("echo", "Echo command") - .handler_fn(move |options, arguments, params| { + .handler_fn(move |_, invocation| { + let options = invocation.switches; + let arguments = invocation.args; + let params = invocation.params; captured_for_handler .lock() .expect("capture lock should not be poisoned") @@ -111,13 +112,16 @@ fn command_builder_registers_recursive_subcommands_without_router_exposure() { let captured = Arc::new(Mutex::new(Vec::new())); let captured_for_handler = Arc::clone(&captured); - let core = CliCore::builder() + let core = CMDKit::builder() .register( command("tool", "tool root") .subcommand( command("run", "run tasks").subcommand( command("one", "run one task") - .handler_fn(move |options, arguments, params| { + .handler_fn(move |_, invocation| { + let options = invocation.switches; + let arguments = invocation.args; + let params = invocation.params; captured_for_handler .lock() .expect("capture lock should not be poisoned")