From aa9f565fe88d938a12bb648efb5d01245f7538ac Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Tue, 17 Sep 2024 11:59:51 +0200 Subject: [PATCH 1/2] remove some unwraps --- crates/shell/src/commands/uname.rs | 57 ++++++++++++++++++------------ crates/shell/src/commands/which.rs | 31 +++++++++------- 2 files changed, 53 insertions(+), 35 deletions(-) diff --git a/crates/shell/src/commands/uname.rs b/crates/shell/src/commands/uname.rs index 0d956ba..2e08f2c 100644 --- a/crates/shell/src/commands/uname.rs +++ b/crates/shell/src/commands/uname.rs @@ -26,29 +26,42 @@ fn display(uname: &UNameOutput) -> String { impl ShellCommand for UnameCommand { fn execute(&self, mut context: ShellCommandContext) -> LocalBoxFuture<'static, ExecuteResult> { - let matches = uu_uname::uu_app() - .no_binary_name(true) - .try_get_matches_from(context.args) - .unwrap(); + Box::pin(async move { + match execute_uname(&mut context) { + Ok(_) => ExecuteResult::from_exit_code(0), + Err(e) => { + context.stderr.write_line(&e).ok(); + ExecuteResult::from_exit_code(1) + } + } + }) + } +} - let options = uu_uname::Options { - all: matches.get_flag(options::ALL), - kernel_name: matches.get_flag(options::KERNEL_NAME), - nodename: matches.get_flag(options::NODENAME), - kernel_release: matches.get_flag(options::KERNEL_RELEASE), - kernel_version: matches.get_flag(options::KERNEL_VERSION), - machine: matches.get_flag(options::MACHINE), - processor: matches.get_flag(options::PROCESSOR), - hardware_platform: matches.get_flag(options::HARDWARE_PLATFORM), - os: matches.get_flag(options::OS), - }; +fn execute_uname(context: &mut ShellCommandContext) -> Result<(), String> { + let matches = uu_uname::uu_app() + .override_usage("uname [OPTION]...") + .no_binary_name(true) + .try_get_matches_from(&context.args) + .map_err(|e| e.to_string())?; - let uname = UNameOutput::new(&options).unwrap(); - context - .stdout - .write_line(display(&uname).trim_end()) - .unwrap(); + let options = uu_uname::Options { + all: matches.get_flag(options::ALL), + kernel_name: matches.get_flag(options::KERNEL_NAME), + nodename: matches.get_flag(options::NODENAME), + kernel_release: matches.get_flag(options::KERNEL_RELEASE), + kernel_version: matches.get_flag(options::KERNEL_VERSION), + machine: matches.get_flag(options::MACHINE), + processor: matches.get_flag(options::PROCESSOR), + hardware_platform: matches.get_flag(options::HARDWARE_PLATFORM), + os: matches.get_flag(options::OS), + }; - Box::pin(futures::future::ready(ExecuteResult::from_exit_code(0))) - } + let uname = UNameOutput::new(&options).unwrap(); + context + .stdout + .write_line(display(&uname).trim_end()) + .map_err(|e| e.to_string())?; + + Ok(()) } diff --git a/crates/shell/src/commands/which.rs b/crates/shell/src/commands/which.rs index ac796ad..6ddb114 100644 --- a/crates/shell/src/commands/which.rs +++ b/crates/shell/src/commands/which.rs @@ -4,29 +4,33 @@ use futures::future::LocalBoxFuture; pub struct WhichCommand; impl ShellCommand for WhichCommand { - fn execute(&self, context: ShellCommandContext) -> LocalBoxFuture<'static, ExecuteResult> { - Box::pin(futures::future::ready(execute_which(context))) + fn execute(&self, mut context: ShellCommandContext) -> LocalBoxFuture<'static, ExecuteResult> { + Box::pin(futures::future::ready(match execute_which(&mut context) { + Ok(_) => ExecuteResult::from_exit_code(0), + Err(exit_code) => ExecuteResult::from_exit_code(exit_code), + })) } } -fn execute_which(mut context: ShellCommandContext) -> ExecuteResult { +fn execute_which(context: &mut ShellCommandContext) -> Result<(), i32> { if context.args.len() != 1 { - context.stderr.write_line("Expected one argument.").unwrap(); - return ExecuteResult::from_exit_code(1); + context.stderr.write_line("Expected one argument").ok(); + return Err(1); } + let arg = &context.args[0]; if let Some(alias) = context.state.alias_map().get(arg) { context .stdout .write_line(&format!("alias: \"{}\"", alias.join(" "))) - .unwrap(); - return ExecuteResult::from_exit_code(0); + .ok(); + return Ok(()); } if context.state.resolve_custom_command(arg).is_some() { - context.stdout.write_line("").unwrap(); - return ExecuteResult::from_exit_code(0); + context.stdout.write_line("").ok(); + return Ok(()); } if let Some(path) = context.state.env_vars().get("PATH") { @@ -35,14 +39,15 @@ fn execute_which(mut context: ShellCommandContext) -> ExecuteResult { .and_then(|mut i| i.next().ok_or(which::Error::CannotFindBinaryPath)); if let Ok(p) = which_result { - context.stdout.write_line(&p.to_string_lossy()).unwrap(); - return ExecuteResult::from_exit_code(0); + context.stdout.write_line(&p.to_string_lossy()).ok(); + return Ok(()); } } context .stderr .write_line(&format!("{} not found", arg)) - .unwrap(); - ExecuteResult::from_exit_code(1) + .ok(); + + Err(1) } From 8f4c6980f33f0c15d8147ca055fbfa5425911013 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Tue, 17 Sep 2024 14:06:16 +0200 Subject: [PATCH 2/2] increase coverage --- crates/tests/src/lib.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/crates/tests/src/lib.rs b/crates/tests/src/lib.rs index 9f2d48a..ec20a31 100644 --- a/crates/tests/src/lib.rs +++ b/crates/tests/src/lib.rs @@ -797,6 +797,20 @@ async fn which() { .assert_stdout("\n") .run() .await; + + TestBuilder::new() + .command("which bla foo") + .assert_exit_code(1) + .assert_stderr("Expected one argument\n") + .run() + .await; + + TestBuilder::new() + .command("alias ll=\"ls -al\" && which ll") + .assert_exit_code(0) + .assert_stdout("alias: \"ls -al\"\n") + .run() + .await; } #[cfg(test)]