From 4031546e4d130227687d406b7abca961f0d34385 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C3=BAl=20Cabrera?= Date: Thu, 5 Sep 2024 08:59:24 -0400 Subject: [PATCH] CLI options improvements (#747) * CLI improvements This commit is a first step toward https://github.com/bytecodealliance/javy/issues/742. Note that the migration is not totally done. Aside from putting in together the initial infrastructure to use `ValueParserFactory` the main intention of this change is to make it easier to derive argument name and documentation from option groups, said differently, make the process of adding new arguments more manageable and less repetitive. * Improved help * Mark js and codegen options as optional and use default if not present * Clippy fixes * Fully implement `ValueParserFactory` This commit removes the usage of `FromStr` and completely migrates to `ValueParserFactory` The main complication here is the deeply nested `Vec` needed in order for `clap` to allow multiple occurrences of the same option, e.g., `-C key=val -C key=val` * More efficient `to_kebab_case` --- crates/cli/src/codegen/builder.rs | 2 +- crates/cli/src/commands.rs | 215 ++++++++++++++---------------- crates/cli/src/main.rs | 9 +- crates/cli/src/option.rs | 175 ++++++++++++++++++++++++ 4 files changed, 283 insertions(+), 118 deletions(-) create mode 100644 crates/cli/src/option.rs diff --git a/crates/cli/src/codegen/builder.rs b/crates/cli/src/codegen/builder.rs index b000e125..bc2c33fd 100644 --- a/crates/cli/src/codegen/builder.rs +++ b/crates/cli/src/codegen/builder.rs @@ -4,7 +4,7 @@ use javy_config::Config; use std::path::PathBuf; /// Options for using WIT in the code generation process. -#[derive(Default)] +#[derive(Default, Clone, Debug)] pub(crate) struct WitOptions { /// The path of the .wit file to use. pub path: Option, diff --git a/crates/cli/src/commands.rs b/crates/cli/src/commands.rs index eea37479..dbbbc17d 100644 --- a/crates/cli/src/commands.rs +++ b/crates/cli/src/commands.rs @@ -1,9 +1,16 @@ -use anyhow::{anyhow, bail}; -use clap::{Parser, Subcommand}; +use crate::option_group; +use anyhow::{anyhow, Result}; +use clap::{ + builder::{StringValueParser, TypedValueParser, ValueParserFactory}, + Parser, Subcommand, +}; use javy_config::Config; -use std::{path::PathBuf, str::FromStr}; +use std::path::PathBuf; use crate::codegen::WitOptions; +use crate::option::{ + fmt_help, GroupDescriptor, GroupOption, GroupOptionBuilder, GroupOptionParser, OptionValue, +}; #[derive(Debug, Parser)] #[command( @@ -80,28 +87,15 @@ pub struct BuildCommandOpts { /// Desired path of the WebAssembly output file. pub output: PathBuf, - #[arg( - short = 'C', - long = "codegen", - long_help = "Available codegen options: --C dynamic[=y|n] -- Creates a smaller module that requires a dynamically linked QuickJS provider Wasm module to execute (see `emit-provider` command). --C wit=path -- Optional path to WIT file describing exported functions. Only supports function exports with no arguments and no return values. --C wit-world=val -- Optional WIT world name for WIT file. Must be specified if WIT is file path is specified. --C source-compression[=y|n] -- Enable source code compression, which generates smaller WebAssembly files at the cost of increased compile time. Defaults to enabled. - " - )] - /// Codegen options. - pub codegen: Vec, - - #[arg( - short = 'J', - long = "js", - long_help = "Available JS runtime options: --J redirect-stdout-to-stderr[=y|n] -- Redirects console.log to stderr. - " - )] - /// JS runtime options. - pub js_runtime: Vec, + #[arg(short = 'C', long = "codegen")] + /// Code generation options. + /// Use `-C help` for more details. + pub codegen: Vec>, + + #[arg(short = 'J', long = "javascript")] + /// JavaScript runtime options. + /// Use `-J help` for more details. + pub js: Vec>, } #[derive(Debug, Parser)] @@ -111,6 +105,53 @@ pub struct EmitProviderCommandOpts { pub out: Option, } +impl ValueParserFactory for GroupOption +where + T: GroupDescriptor, +{ + type Parser = GroupOptionParser; + + fn value_parser() -> Self::Parser { + GroupOptionParser(std::marker::PhantomData) + } +} + +impl TypedValueParser for GroupOptionParser +where + T: GroupDescriptor + GroupOptionBuilder, +{ + type Value = GroupOption; + + fn parse_ref( + &self, + cmd: &clap::Command, + arg: Option<&clap::Arg>, + value: &std::ffi::OsStr, + ) -> Result { + let val = StringValueParser::new().parse_ref(cmd, arg, value)?; + let arg = arg.expect("argument to be defined"); + let short = arg.get_short().expect("short version to be defined"); + let long = arg.get_long().expect("long version to be defined"); + + if val == "help" { + fmt_help(long, &short.to_string(), &T::options()); + std::process::exit(0); + } + + let mut opts = vec![]; + + for val in val.split(',') { + opts.push(T::parse(val).map_err(|e| { + clap::Error::raw(clap::error::ErrorKind::InvalidValue, format!("{}", e)) + })?) + } + + Ok(GroupOption(opts)) + } +} + +/// Code generation option group. +#[derive(Clone, Debug)] pub struct CodegenOptionGroup { /// Creates a smaller module that requires a dynamically linked QuickJS provider Wasm /// module to execute (see `emit-provider` command). @@ -131,125 +172,73 @@ impl Default for CodegenOptionGroup { } } -#[derive(Clone, Debug)] -pub enum CodegenOption { - /// Creates a smaller module that requires a dynamically linked QuickJS provider Wasm - /// module to execute (see `emit-provider` command). - Dynamic(bool), - /// Optional path to WIT file describing exported functions. - /// Only supports function exports with no arguments and no return values. - Wit(PathBuf), - /// Optional path to WIT file describing exported functions. - /// Only supports function exports with no arguments and no return values. - WitWorld(String), - /// Enable source code compression, which generates smaller WebAssembly files at the cost of increased compile time. Defaults to enabled. - SourceCompression(bool), -} - -impl FromStr for CodegenOption { - type Err = anyhow::Error; - - fn from_str(s: &str) -> std::result::Result { - let mut parts = s.splitn(2, '='); - let key = parts.next().ok_or_else(|| anyhow!("Invalid codegen key"))?; - let value = parts.next(); - let option = match key { - "dynamic" => Self::Dynamic(match value { - None => true, - Some("y") => true, - Some("n") => false, - _ => bail!("Invalid value for dynamic"), - }), - "wit" => Self::Wit(PathBuf::from( - value.ok_or_else(|| anyhow!("Must provide value for wit"))?, - )), - "wit-world" => Self::WitWorld( - value - .ok_or_else(|| anyhow!("Must provide value for wit-world"))? - .to_string(), - ), - "source-compression" => Self::SourceCompression(match value { - None => true, - Some("y") => true, - Some("n") => false, - _ => bail!("Invalid value for source-compression"), - }), - _ => bail!("Invalid codegen key"), - }; - Ok(option) +option_group! { + #[derive(Clone, Debug)] + pub enum CodegenOption { + /// Creates a smaller module that requires a dynamically linked QuickJS provider Wasm + /// module to execute (see `emit-provider` command). + Dynamic(bool), + /// Optional path to WIT file describing exported functions. + /// Only supports function exports with no arguments and no return values. + Wit(PathBuf), + /// Optional path to WIT file describing exported functions. + /// Only supports function exports with no arguments and no return values. + WitWorld(String), + /// Enable source code compression, which generates smaller WebAssembly files at the cost of increased compile time. Defaults to enabled. + SourceCompression(bool), } } -impl TryFrom> for CodegenOptionGroup { +impl TryFrom>> for CodegenOptionGroup { type Error = anyhow::Error; - fn try_from(value: Vec) -> Result { + fn try_from(value: Vec>) -> Result { let mut options = Self::default(); let mut wit = None; let mut wit_world = None; - for option in value { + for option in value.iter().flat_map(|i| i.0.iter()) { match option { - CodegenOption::Dynamic(enabled) => options.dynamic = enabled, + CodegenOption::Dynamic(enabled) => options.dynamic = *enabled, CodegenOption::Wit(path) => wit = Some(path), CodegenOption::WitWorld(world) => wit_world = Some(world), - CodegenOption::SourceCompression(enabled) => options.source_compression = enabled, + CodegenOption::SourceCompression(enabled) => options.source_compression = *enabled, } } - options.wit = WitOptions::from_tuple((wit, wit_world))?; + options.wit = WitOptions::from_tuple((wit.cloned(), wit_world.cloned()))?; Ok(options) } } -#[derive(Debug, PartialEq)] -pub struct JsRuntimeOptionGroup { +#[derive(Clone, Debug, PartialEq)] +pub struct JsOptionGroup { /// Whether to redirect console.log to stderr. pub redirect_stdout_to_stderr: bool, } -impl Default for JsRuntimeOptionGroup { +impl Default for JsOptionGroup { fn default() -> Self { Config::default().into() } } -#[derive(Clone, Debug)] -pub enum JsRuntimeOption { - /// Whether to redirect console.log to stderr. - RedirectStdoutToStderr(bool), -} - -impl FromStr for JsRuntimeOption { - type Err = anyhow::Error; - - fn from_str(s: &str) -> std::result::Result { - let mut parts = s.splitn(2, '='); - let key = parts - .next() - .ok_or_else(|| anyhow!("Invalid JS runtime key"))?; - let value = parts.next(); - let option = match key { - "redirect-stdout-to-stderr" => Self::RedirectStdoutToStderr(match value { - None => true, - Some("y") => true, - Some("n") => false, - _ => bail!("Invalid value for redirect-stdout-to-stderr"), - }), - _ => bail!("Invalid JS runtime key"), - }; - Ok(option) +option_group! { + #[derive(Clone, Debug)] + pub enum JsOption { + /// Whether to redirect the output of console.log to standard error. + RedirectStdoutToStderr(bool), } } -impl From> for JsRuntimeOptionGroup { - fn from(value: Vec) -> Self { +impl From>> for JsOptionGroup { + fn from(value: Vec>) -> Self { let mut group = Self::default(); - for option in value { + for option in value.iter().flat_map(|e| e.0.iter()) { match option { - JsRuntimeOption::RedirectStdoutToStderr(enabled) => { - group.redirect_stdout_to_stderr = enabled; + JsOption::RedirectStdoutToStderr(enabled) => { + group.redirect_stdout_to_stderr = *enabled; } } } @@ -258,8 +247,8 @@ impl From> for JsRuntimeOptionGroup { } } -impl From for Config { - fn from(value: JsRuntimeOptionGroup) -> Self { +impl From for Config { + fn from(value: JsOptionGroup) -> Self { let mut config = Self::default(); config.set( Config::REDIRECT_STDOUT_TO_STDERR, @@ -269,7 +258,7 @@ impl From for Config { } } -impl From for JsRuntimeOptionGroup { +impl From for JsOptionGroup { fn from(value: Config) -> Self { Self { redirect_stdout_to_stderr: value.contains(Config::REDIRECT_STDOUT_TO_STDERR), diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 814a4e74..a09772b7 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -2,6 +2,7 @@ mod bytecode; mod codegen; mod commands; mod js; +mod option; mod wit; use crate::codegen::WitOptions; @@ -9,7 +10,7 @@ use crate::commands::{Cli, Command, EmitProviderCommandOpts}; use anyhow::Result; use clap::Parser; use codegen::{CodeGenBuilder, DynamicGenerator, StaticGenerator}; -use commands::{CodegenOptionGroup, JsRuntimeOptionGroup}; +use commands::{CodegenOptionGroup, JsOptionGroup}; use javy_config::Config; use js::JS; use std::fs; @@ -65,11 +66,11 @@ fn main() -> Result<()> { .source_compression(codegen.source_compression) .provider_version("2"); - let js_runtime_options: JsRuntimeOptionGroup = opts.js_runtime.clone().into(); + let js_opts: JsOptionGroup = opts.js.clone().into(); let mut gen = if codegen.dynamic { - builder.build::(js_runtime_options.into())? + builder.build::(js_opts.into())? } else { - builder.build::(js_runtime_options.into())? + builder.build::(js_opts.into())? }; let wasm = gen.generate(&js)?; diff --git a/crates/cli/src/option.rs b/crates/cli/src/option.rs new file mode 100644 index 00000000..80f1d20b --- /dev/null +++ b/crates/cli/src/option.rs @@ -0,0 +1,175 @@ +use anyhow::{bail, Result}; +use std::path::PathBuf; + +/// An option group used for parsing strings to their group option representation. +#[derive(Clone, Debug)] +pub struct GroupOption(pub Vec); + +#[derive(Clone)] +pub struct GroupOptionParser(pub std::marker::PhantomData); + +/// Generic option attributes. +#[derive(Debug)] +pub struct OptionMeta { + pub name: String, + pub doc: String, + pub help: String, +} + +pub fn fmt_help(cmd: &str, short: &str, meta: &[OptionMeta]) { + println!("Available options for {}", cmd); + for opt in meta { + println!(); + print!("-{:<3}", short); + print!("{:>3}", opt.name); + println!("{}", opt.help); + for line in opt.doc.split('\n') { + print!("{}", line); + } + println!(); + } +} + +/// Commonalities between all option groups. +// Until we make more extensive use of this trait. +#[allow(dead_code)] +pub trait GroupDescriptor { + fn options() -> Vec; +} + +pub trait GroupOptionBuilder: Clone + Sized + Send + Sync + 'static { + fn parse(val: &str) -> anyhow::Result + where + Self: Sized; +} + +pub fn to_kebab_case(val: &str) -> String { + let mut kebab_case = val + .chars() + .flat_map(|c| { + if c.is_uppercase() { + vec!['-', c.to_ascii_lowercase()] + } else { + vec![c] + } + }) + .collect::(); + + if kebab_case.starts_with('-') { + kebab_case.remove(0); + } + + kebab_case +} + +#[macro_export] +macro_rules! option_group { + ( + $(#[$attr:meta])* + pub enum $opts:ident { + $( + $(#[doc = $doc:tt])* + $opt:ident($type:ty), + )+ + } + + ) => { + + $(#[$attr])* + pub enum $opts { + $( + $opt($type), + )+ + } + + impl $crate::option::GroupDescriptor for $opts { + fn options() -> Vec<$crate::option::OptionMeta> { + let mut options = vec![]; + $( + let name = $crate::option::to_kebab_case(stringify!($opt)); + options.push($crate::option::OptionMeta { + name: name.to_string(), + doc: concat!($($doc, "\n",)*).into(), + help: <$type>::help().to_string(), + }); + )+ + + options + } + } + + impl $crate::option::GroupOptionBuilder for $opts { + fn parse(val: &str) -> anyhow::Result { + let mut parts = val.splitn(2, '='); + let key = parts.next().ok_or_else(|| anyhow!("Expected key. None found"))?; + let val = parts.next(); + + $( + if key == $crate::option::to_kebab_case(stringify!($opt)) { + return Ok($opts::$opt($crate::option::OptionValue::parse(val)?)); + } + )+ + + Err(anyhow!("Invalid argument")) + } + } + }; +} + +/// Represents all values that can be parsed. +pub trait OptionValue { + fn help() -> &'static str; + fn parse(val: Option<&str>) -> Result + where + Self: Sized; +} + +impl OptionValue for bool { + fn help() -> &'static str { + "[=y|n]" + } + + fn parse(val: Option<&str>) -> Result + where + Self: Sized, + { + match val { + None => Ok(true), + Some("yes") | Some("y") => Ok(true), + Some("no") | Some("n") => Ok(false), + Some(_) => bail!("Unknown boolean flag. Valid options: y[es], n[o], (empty)"), + } + } +} + +impl OptionValue for String { + fn help() -> &'static str { + "=val" + } + + fn parse(val: Option<&str>) -> Result + where + Self: Sized, + { + match val { + Some(v) => Ok(v.into()), + None => bail!("Expected string argument"), + } + } +} + +impl OptionValue for PathBuf { + fn help() -> &'static str { + "=path" + } + + fn parse(val: Option<&str>) -> Result + where + Self: Sized, + { + match val { + Some(v) => Ok(PathBuf::from(v)), + None => bail!("Expected path argument"), + } + } +}