From 621453c5b3ad40d011d37b59c812fe65d8d84f81 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Mon, 6 Nov 2023 12:24:25 -0500 Subject: [PATCH 1/2] fix: print help if custom CLI is empty & replace unwrap where prossible --- .../commands/contract/bindings/typescript.rs | 10 ++++++++-- cmd/soroban-cli/src/commands/contract/deploy.rs | 8 +++----- cmd/soroban-cli/src/commands/contract/invoke.rs | 17 ++++++++++++----- cmd/soroban-cli/src/commands/plugin.rs | 4 +++- cmd/soroban-cli/src/utils.rs | 4 +++- 5 files changed, 29 insertions(+), 14 deletions(-) diff --git a/cmd/soroban-cli/src/commands/contract/bindings/typescript.rs b/cmd/soroban-cli/src/commands/contract/bindings/typescript.rs index c0a780153..1c39cf2ae 100644 --- a/cmd/soroban-cli/src/commands/contract/bindings/typescript.rs +++ b/cmd/soroban-cli/src/commands/contract/bindings/typescript.rs @@ -61,13 +61,17 @@ pub enum Error { Fetch(#[from] fetch::Error), #[error(transparent)] Spec(#[from] contract_spec::Error), + #[error(transparent)] + Wasm(#[from] wasm::Error), + #[error("Failed to get file name from path: {0:?}")] + FailedToGetFileName(PathBuf), } impl Cmd { pub async fn run(&self) -> Result<(), Error> { let spec = if let Some(wasm) = &self.wasm { let wasm: wasm::Args = wasm.into(); - wasm.parse().unwrap().spec + wasm.parse()?.spec } else { let fetch = contract::fetch::Cmd { contract_id: self.contract_id.clone(), @@ -100,7 +104,9 @@ impl Cmd { .ok() .unwrap_or_else(Network::futurenet); let absolute_path = self.output_dir.canonicalize()?; - let file_name = absolute_path.file_name().unwrap(); + let file_name = absolute_path + .file_name() + .ok_or_else(|| Error::FailedToGetFileName(absolute_path.clone()))?; let contract_name = &file_name .to_str() .ok_or_else(|| Error::NotUtf8(file_name.to_os_string()))?; diff --git a/cmd/soroban-cli/src/commands/contract/deploy.rs b/cmd/soroban-cli/src/commands/contract/deploy.rs index 5ef18a833..5467b992a 100644 --- a/cmd/soroban-cli/src/commands/contract/deploy.rs +++ b/cmd/soroban-cli/src/commands/contract/deploy.rs @@ -85,6 +85,8 @@ pub enum Error { Config(#[from] config::Error), #[error(transparent)] StrKey(#[from] stellar_strkey::DecodeError), + #[error(transparent)] + Infallible(#[from] std::convert::Infallible), } impl Cmd { @@ -204,11 +206,7 @@ fn get_contract_id( contract_id_preimage: ContractIdPreimage, network_passphrase: &str, ) -> Result { - let network_id = Hash( - Sha256::digest(network_passphrase.as_bytes()) - .try_into() - .unwrap(), - ); + let network_id = Hash(Sha256::digest(network_passphrase.as_bytes()).try_into()?); let preimage = HashIdPreimage::ContractId(HashIdPreimageContractId { network_id, contract_id_preimage, diff --git a/cmd/soroban-cli/src/commands/contract/invoke.rs b/cmd/soroban-cli/src/commands/contract/invoke.rs index 59c9bf440..4543ed05c 100644 --- a/cmd/soroban-cli/src/commands/contract/invoke.rs +++ b/cmd/soroban-cli/src/commands/contract/invoke.rs @@ -161,14 +161,19 @@ impl Cmd { let mut cmd = clap::Command::new(self.contract_id.clone()) .no_binary_name(true) .term_width(300) + // .help .max_term_width(300); for ScSpecFunctionV0 { name, .. } in spec.find_functions()? { cmd = cmd.subcommand(build_custom_cmd(&name.to_string_lossy(), &spec)?); } cmd.build(); + let long_help = cmd.render_long_help(); let mut matches_ = cmd.get_matches_from(&self.slop); - let (function, matches_) = &matches_.remove_subcommand().unwrap(); + let Some((function, matches_)) = &matches_.remove_subcommand() else { + println!("{long_help}"); + std::process::exit(1); + }; let func = spec.find_function(function)?; // create parsed_args in same order as the inputs to func @@ -177,7 +182,7 @@ impl Cmd { .inputs .iter() .map(|i| { - let name = i.name.to_string().unwrap(); + let name = i.name.to_string()?; if let Some(mut val) = matches_.get_raw(&name) { let mut s = val.next().unwrap().to_string_lossy().to_string(); if matches!(i.type_, ScSpecTypeDef::Address) { @@ -205,7 +210,10 @@ impl Cmd { &std::fs::read(arg_path) .map_err(|_| Error::MissingFileArg(arg_path.clone()))?, ) - .unwrap()) + .map_err(|()| Error::CannotParseArg { + arg: name.clone(), + error: soroban_spec_tools::Error::Unknown, + })?) } else { let file_contents = std::fs::read_to_string(arg_path) .map_err(|_| Error::MissingFileArg(arg_path.clone()))?; @@ -414,7 +422,6 @@ fn build_custom_cmd(name: &str, spec: &Spec) -> Result { if kebab_name != name { cmd = cmd.alias(kebab_name); } - let func = spec.find_function(name).unwrap(); let doc: &'static str = Box::leak(func.doc.to_string_lossy().into_boxed_str()); let long_doc: &'static str = Box::leak(arg_file_help(doc).into_boxed_str()); @@ -428,7 +435,7 @@ fn build_custom_cmd(name: &str, spec: &Spec) -> Result { .alias(name.to_kebab_case()) .num_args(1) .value_parser(clap::builder::NonEmptyStringValueParser::new()) - .long_help(spec.doc(name, type_).unwrap()); + .long_help(spec.doc(name, type_)?); file_arg = file_arg .long(&file_arg_name) diff --git a/cmd/soroban-cli/src/commands/plugin.rs b/cmd/soroban-cli/src/commands/plugin.rs index e847dd043..27c191f08 100644 --- a/cmd/soroban-cli/src/commands/plugin.rs +++ b/cmd/soroban-cli/src/commands/plugin.rs @@ -19,6 +19,8 @@ pub enum Error { ExecutableNotFound(String, String), #[error(transparent)] Which(#[from] which::Error), + #[error(transparent)] + Regex(#[from] regex::Error), } const SUBCOMMAND_TOLERANCE: f64 = 0.75; @@ -82,7 +84,7 @@ pub fn list() -> Result, Error> { } else { r"^soroban-.*" }; - let re = regex::Regex::new(re_str).unwrap(); + let re = regex::Regex::new(re_str)?; Ok(which::which_re(re)? .filter_map(|b| { let s = b.file_name()?.to_str()?; diff --git a/cmd/soroban-cli/src/utils.rs b/cmd/soroban-cli/src/utils.rs index fcc3964c9..27faee733 100644 --- a/cmd/soroban-cli/src/utils.rs +++ b/cmd/soroban-cli/src/utils.rs @@ -131,6 +131,8 @@ pub mod parsing { CannotParseAccountId { account_id: String }, #[error("cannot parse asset: {asset}")] CannotParseAsset { asset: String }, + #[error(transparent)] + Regex(#[from] regex::Error), } pub fn parse_asset(str: &str) -> Result { @@ -145,7 +147,7 @@ pub mod parsing { } let code = split[0]; let issuer = split[1]; - let re = Regex::new("^[[:alnum:]]{1,12}$").unwrap(); + let re = Regex::new("^[[:alnum:]]{1,12}$")?; if !re.is_match(code) { return Err(Error::InvalidAssetCode { asset: str.to_string(), From 80717facb588f39cd4f68344b71d5b00e1d06cad Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Mon, 6 Nov 2023 13:08:16 -0500 Subject: [PATCH 2/2] chore: remove comment --- cmd/soroban-cli/src/commands/contract/invoke.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/soroban-cli/src/commands/contract/invoke.rs b/cmd/soroban-cli/src/commands/contract/invoke.rs index 4543ed05c..2d4c77c61 100644 --- a/cmd/soroban-cli/src/commands/contract/invoke.rs +++ b/cmd/soroban-cli/src/commands/contract/invoke.rs @@ -161,7 +161,6 @@ impl Cmd { let mut cmd = clap::Command::new(self.contract_id.clone()) .no_binary_name(true) .term_width(300) - // .help .max_term_width(300); for ScSpecFunctionV0 { name, .. } in spec.find_functions()? {