chore: remove some unwraps (#136)

* remove some unwraps * increase coverage
prefix-dev · Sep 17, 2024 · 144a8de · 144a8de
1 parent 16b3204
commit 144a8de
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 35 deletions.
diff --git 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
@@ -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("<builtin function>").unwrap();
-        return ExecuteResult::from_exit_code(0);
+        context.stdout.write_line("<builtin function>").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)
 }
diff --git a/crates/tests/src/lib.rs b/crates/tests/src/lib.rs
@@ -797,6 +797,20 @@ async fn which() {
         .assert_stdout("<builtin function>\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)]