More ununwraps (#1152)

* More ununwraps

* More ununwraps

* Update completer.rs

* Update completer.rs

src/cli.rs

@@ -61,16 +61,16 @@ fn load_plugin(path: &std::path::Path, context: &mut Context) -> Result<(), Shel
                         let name = params.name.clone();
                         let fname = fname.to_string();
 
-                        if context.get_command(&name).is_some() {
+                        if context.get_command(&name)?.is_some() {
                             trace!("plugin {:?} already loaded.", &name);
                         } else if params.is_filter {
-                            context.add_commands(vec![whole_stream_command(PluginCommand::new(
+                            context.add_commands(vec![whole_stream_command(
-                                name, fname, params,
+                                PluginCommand::new(name, fname, params),
-                            ))]);
+                            )])?;
                         } else {
                             context.add_commands(vec![whole_stream_command(PluginSink::new(
                                 name, fname, params,
-                            ))]);
+                            ))])?;
                         }
                         Ok(())
                     }
@@ -328,7 +328,7 @@ pub async fn cli() -> Result<(), Box<dyn Error>> {
             whole_stream_command(FromXML),
             whole_stream_command(FromYAML),
             whole_stream_command(FromYML),
-        ]);
+        ])?;
 
         cfg_if::cfg_if! {
             if #[cfg(data_processing_primitives)] {
@@ -337,7 +337,7 @@ pub async fn cli() -> Result<(), Box<dyn Error>> {
                 whole_stream_command(EvaluateBy),
                 whole_stream_command(TSortBy),
                 whole_stream_command(MapMaxBy),
-                ]);
+                ])?;
             }
         }
 
@@ -345,7 +345,7 @@ pub async fn cli() -> Result<(), Box<dyn Error>> {
         {
             context.add_commands(vec![whole_stream_command(
                 crate::commands::clip::clipboard::Clip,
-            )]);
+            )])?;
         }
     }
 

src/commands/autoview.rs

@@ -70,7 +70,7 @@ pub fn autoview(
                                 }
                             }
                         };
-                        if let Some(table) = table {
+                        if let Some(table) = table? {
                             let mut new_output_stream: OutputStream = stream.to_output_stream();
                             let mut finished = false;
                             let mut current_idx = 0;
@@ -136,7 +136,7 @@ pub fn autoview(
                                     value: UntaggedValue::Primitive(Primitive::String(ref s)),
                                     tag: Tag { anchor, span },
                                 } if anchor.is_some() => {
-                                    if let Some(text) = text {
+                                    if let Some(text) = text? {
                                         let mut stream = VecDeque::new();
                                         stream.push_back(UntaggedValue::string(s).into_value(Tag { anchor, span }));
                                         let result = text.run(raw.with_input(stream.into()), &context.commands);
@@ -155,7 +155,7 @@ pub fn autoview(
                                     value: UntaggedValue::Primitive(Primitive::Line(ref s)),
                                     tag: Tag { anchor, span },
                                 } if anchor.is_some() => {
-                                    if let Some(text) = text {
+                                    if let Some(text) = text? {
                                         let mut stream = VecDeque::new();
                                         stream.push_back(UntaggedValue::string(s).into_value(Tag { anchor, span }));
                                         let result = text.run(raw.with_input(stream.into()), &context.commands);
@@ -190,7 +190,7 @@ pub fn autoview(
                                 }
 
                                 Value { value: UntaggedValue::Primitive(Primitive::Binary(ref b)), .. } => {
-                                    if let Some(binary) = binary {
+                                    if let Some(binary) = binary? {
                                         let mut stream = VecDeque::new();
                                         stream.push_back(x);
                                         let result = binary.run(raw.with_input(stream.into()), &context.commands);
@@ -205,7 +205,7 @@ pub fn autoview(
                                     yield Err(e);
                                 }
                                 Value { value: ref item, .. } => {
-                                    if let Some(table) = table {
+                                    if let Some(table) = table? {
                                         let mut stream = VecDeque::new();
                                         stream.push_back(x);
                                         let result = table.run(raw.with_input(stream.into()), &context.commands);

src/commands/classified/internal.rs

@@ -57,7 +57,7 @@ pub(crate) async fn run_internal_command(
                         let contents_tag = tagged_contents.tag.clone();
                         let command_name = format!("from-{}", extension);
                         let command = command.clone();
-                        if let Some(converter) = context.registry.get_command(&command_name) {
+                        if let Some(converter) = context.registry.get_command(&command_name)? {
                             let new_args = RawCommandArgs {
                                 host: context.host.clone(),
                                 ctrl_c: context.ctrl_c.clone(),
@@ -103,7 +103,7 @@ pub(crate) async fn run_internal_command(
                                     HelpShell::for_command(
                                         UntaggedValue::string(cmd).into_value(tag),
                                         &context.registry(),
-                                    ).unwrap(),
+                                    )?,
                                 ));
                             }
                             _ => {

src/commands/command.rs

@@ -227,7 +227,7 @@ pub struct RunnableContext {
 }
 
 impl RunnableContext {
-    pub fn get_command(&self, name: &str) -> Option<Arc<Command>> {
+    pub fn get_command(&self, name: &str) -> Result<Option<Arc<Command>>, ShellError> {
         self.commands.get_command(name)
     }
 }

src/commands/enter.rs

@@ -51,7 +51,7 @@ impl PerItemCommand for Enter {
 
                     let (_, command) = (spec[0], spec[1]);
 
-                    if registry.has(command) {
+                    if registry.has(command)? {
                         Ok(vec![Ok(ReturnSuccess::Action(CommandAction::EnterHelpShell(
                             UntaggedValue::string(command).into_value(Tag::unknown()),
                         )))]
@@ -88,7 +88,7 @@ impl PerItemCommand for Enter {
                                 if let Some(extension) = file_extension {
                                     let command_name = format!("from-{}", extension);
                                     if let Some(converter) =
-                                        registry.get_command(&command_name)
+                                        registry.get_command(&command_name)?
                                     {
                                         let new_args = RawCommandArgs {
                                             host: raw_args.host,

src/commands/from_ssv.rs

@@ -391,7 +391,7 @@ mod tests {
     }
 
     #[test]
-    fn it_trims_remaining_separator_space() {
+    fn it_trims_remaining_separator_space() -> Result<(), ShellError> {
         let input = r#"
             colA   colB     colC
             val1   val2     val3
@@ -399,14 +399,17 @@ mod tests {
 
         let trimmed = |s: &str| s.trim() == s;
 
-        let result = string_to_table(input, false, true, 2).unwrap();
+        let result = string_to_table(input, false, true, 2)
+            .ok_or_else(|| ShellError::unexpected("table couldn't be parsed"))?;
         assert!(result
             .iter()
-            .all(|row| row.iter().all(|(a, b)| trimmed(a) && trimmed(b))))
+            .all(|row| row.iter().all(|(a, b)| trimmed(a) && trimmed(b))));
+
+        Ok(())
     }
 
     #[test]
-    fn it_keeps_empty_columns() {
+    fn it_keeps_empty_columns() -> Result<(), ShellError> {
         let input = r#"
             colA   col B     col C
                    val2      val3
@@ -414,7 +417,8 @@ mod tests {
             val7             val8
         "#;
 
-        let result = string_to_table(input, false, true, 2).unwrap();
+        let result = string_to_table(input, false, true, 2)
+            .ok_or_else(|| ShellError::unexpected("table couldn't be parsed"))?;
         assert_eq!(
             result,
             vec![
@@ -434,35 +438,39 @@ mod tests {
                     owned("col C", "val8")
                 ],
             ]
-        )
+        );
+        Ok(())
     }
 
     #[test]
-    fn it_uses_the_full_final_column() {
+    fn it_uses_the_full_final_column() -> Result<(), ShellError> {
         let input = r#"
             colA   col B
             val1   val2   trailing value that should be included
         "#;
 
-        let result = string_to_table(input, false, true, 2).unwrap();
+        let result = string_to_table(input, false, true, 2)
+            .ok_or_else(|| ShellError::unexpected("table couldn't be parsed"))?;
         assert_eq!(
             result,
             vec![vec![
                 owned("colA", "val1"),
                 owned("col B", "val2   trailing value that should be included"),
             ],]
-        )
+        );
+        Ok(())
     }
 
     #[test]
-    fn it_handles_empty_values_when_headerless_and_aligned_columns() {
+    fn it_handles_empty_values_when_headerless_and_aligned_columns() -> Result<(), ShellError> {
         let input = r#"
             a multi-word value  b           d
             1                        3-3    4
                                                        last
         "#;
 
-        let result = string_to_table(input, true, true, 2).unwrap();
+        let result = string_to_table(input, true, true, 2)
+            .ok_or_else(|| ShellError::unexpected("table couldn't be parsed"))?;
         assert_eq!(
             result,
             vec![
@@ -488,22 +496,28 @@ mod tests {
                     owned("Column5", "last")
                 ],
             ]
-        )
+        );
+        Ok(())
     }
 
     #[test]
-    fn input_is_parsed_correctly_if_either_option_works() {
+    fn input_is_parsed_correctly_if_either_option_works() -> Result<(), ShellError> {
         let input = r#"
                 docker-registry   docker-registry=default                   docker-registry=default   172.30.78.158   5000/TCP
                 kubernetes        component=apiserver,provider=kubernetes   <none>                    172.30.0.2      443/TCP
                 kubernetes-ro     component=apiserver,provider=kubernetes   <none>                    172.30.0.1      80/TCP
             "#;
 
-        let aligned_columns_headerless = string_to_table(input, true, true, 2).unwrap();
+        let aligned_columns_headerless = string_to_table(input, true, true, 2)
-        let separator_headerless = string_to_table(input, true, false, 2).unwrap();
+            .ok_or_else(|| ShellError::unexpected("table couldn't be parsed"))?;
-        let aligned_columns_with_headers = string_to_table(input, false, true, 2).unwrap();
+        let separator_headerless = string_to_table(input, true, false, 2)
-        let separator_with_headers = string_to_table(input, false, false, 2).unwrap();
+            .ok_or_else(|| ShellError::unexpected("table couldn't be parsed"))?;
+        let aligned_columns_with_headers = string_to_table(input, false, true, 2)
+            .ok_or_else(|| ShellError::unexpected("table couldn't be parsed"))?;
+        let separator_with_headers = string_to_table(input, false, false, 2)
+            .ok_or_else(|| ShellError::unexpected("table couldn't be parsed"))?;
         assert_eq!(aligned_columns_headerless, separator_headerless);
         assert_eq!(aligned_columns_with_headers, separator_with_headers);
+        Ok(())
     }
 }

src/commands/help.rs

@@ -40,12 +40,12 @@ impl PerItemCommand for Help {
             }) => {
                 let mut help = VecDeque::new();
                 if document == "commands" {
-                    let mut sorted_names = registry.names();
+                    let mut sorted_names = registry.names()?;
                     sorted_names.sort();
                     for cmd in sorted_names {
                         let mut short_desc = TaggedDictBuilder::new(tag.clone());
                         let value = command_dict(
-                            registry.get_command(&cmd).ok_or_else(|| {
+                            registry.get_command(&cmd)?.ok_or_else(|| {
                                 ShellError::labeled_error(
                                     format!("Could not load {}", cmd),
                                     "could not load command",
@@ -71,7 +71,7 @@ impl PerItemCommand for Help {
 
                         help.push_back(ReturnSuccess::value(short_desc.into_value()));
                     }
-                } else if let Some(command) = registry.get_command(document) {
+                } else if let Some(command) = registry.get_command(document)? {
                     let mut long_desc = String::new();
 
                     long_desc.push_str(&command.usage());

src/commands/plugin.rs

@@ -106,14 +106,26 @@ pub fn filter_plugin(
                 let mut reader = BufReader::new(stdout);
 
                 let request = JsonRpc::new("begin_filter", call_info.clone());
-                let request_raw = serde_json::to_string(&request).unwrap();
+                let request_raw = serde_json::to_string(&request);
-                match stdin.write(format!("{}\n", request_raw).as_bytes()) {
+
+                match request_raw {
+                    Err(_) => {
+                        let mut result = VecDeque::new();
+                        result.push_back(Err(ShellError::labeled_error(
+                            "Could not load json from plugin",
+                            "could not load json from plugin",
+                            &call_info.name_tag,
+                        )));
+                        return result;
+                    }
+                    Ok(request_raw) => match stdin.write(format!("{}\n", request_raw).as_bytes()) {
                         Ok(_) => {}
                         Err(err) => {
                             let mut result = VecDeque::new();
                             result.push_back(Err(ShellError::unexpected(format!("{}", err))));
                             return result;
                         }
+                    },
                 }
 
                 let mut input = String::new();
@@ -179,8 +191,20 @@ pub fn filter_plugin(
                                 Ok(params) => {
                                     let request: JsonRpc<std::vec::Vec<Value>> =
                                         JsonRpc::new("quit", vec![]);
-                                    let request_raw = serde_json::to_string(&request).unwrap();
+                                    let request_raw = serde_json::to_string(&request);
+                                    match request_raw {
+                                        Ok(request_raw) => {
                                             let _ = stdin.write(format!("{}\n", request_raw).as_bytes()); // TODO: Handle error
+                                        }
+                                        Err(e) => {
+                                            let mut result = VecDeque::new();
+                                            result.push_back(Err(ShellError::untagged_runtime_error(format!(
+                                                "Error while processing begin_filter response: {:?} {}",
+                                                e, input
+                                            ))));
+                                            return result;
+                                        }
+                                    }
 
                                     params
                                 }
@@ -221,8 +245,20 @@ pub fn filter_plugin(
                 let mut reader = BufReader::new(stdout);
 
                 let request = JsonRpc::new("filter", v);
-                let request_raw = serde_json::to_string(&request).unwrap();
+                let request_raw = serde_json::to_string(&request);
+                match request_raw {
+                    Ok(request_raw) => {
                         let _ = stdin.write(format!("{}\n", request_raw).as_bytes()); // TODO: Handle error
+                    }
+                    Err(e) => {
+                        let mut result = VecDeque::new();
+                        result.push_back(Err(ShellError::untagged_runtime_error(format!(
+                            "Error while processing filter response: {:?}",
+                            e
+                        ))));
+                        return result;
+                    }
+                }
 
                 let mut input = String::new();
                 match reader.read_line(&mut input) {
@@ -304,22 +340,32 @@ pub fn sink_plugin(
         let input: Vec<Value> = args.input.values.collect().await;
 
         let request = JsonRpc::new("sink", (call_info.clone(), input));
-        let request_raw = serde_json::to_string(&request).unwrap();
+        let request_raw = serde_json::to_string(&request);
-        let mut tmpfile = tempfile::NamedTempFile::new().unwrap();
+        if let Ok(request_raw) = request_raw {
+            if let Ok(mut tmpfile) = tempfile::NamedTempFile::new() {
                 let _ = writeln!(tmpfile, "{}", request_raw);
                 let _ = tmpfile.flush();
 
                 let mut child = std::process::Command::new(path)
                     .arg(tmpfile.path())
-            .spawn()
+                    .spawn();
-            .expect("Failed to spawn child process");
 
+                if let Ok(mut child) = child {
                     let _ = child.wait();
 
                     // Needed for async_stream to type check
                     if false {
                         yield ReturnSuccess::value(UntaggedValue::nothing().into_untagged_value());
                     }
+                } else {
+                    yield Err(ShellError::untagged_runtime_error("Could not create process for sink command"));
+                }
+            } else {
+                yield Err(ShellError::untagged_runtime_error("Could not open file to send sink command message"));
+            }
+        } else {
+            yield Err(ShellError::untagged_runtime_error("Could not create message to sink command"));
+        }
     };
     Ok(OutputStream::new(stream))
 }

src/commands/save.rs

@@ -179,7 +179,7 @@ fn save(
             break if !save_raw {
                 if let Some(extension) = full_path.extension() {
                     let command_name = format!("to-{}", extension.to_string_lossy());
-                    if let Some(converter) = registry.get_command(&command_name) {
+                    if let Some(converter) = registry.get_command(&command_name)? {
                         let new_args = RawCommandArgs {
                             host,
                             ctrl_c,

src/commands/which_.rs

@@ -95,7 +95,7 @@ fn which(
                 }
             }
 
-            let builtin = commands.has(&item);
+            let builtin = commands.has(&item)?;
             if builtin {
                 yield ReturnSuccess::value(entry_builtin!(item, application.tag.clone()));
             }
@@ -128,7 +128,7 @@ fn which(
                 if let Ok(path) = ichwh::which(&item).await {
                     yield ReturnSuccess::value(entry_path!(item, path.into(), application.tag.clone()));
                 }
-            } else if commands.has(&item) {
+            } else if commands.has(&item)? {
                 yield ReturnSuccess::value(entry_builtin!(item, application.tag.clone()));
             } else if let Ok(path) = ichwh::which(&item).await {
                 yield ReturnSuccess::value(entry_path!(item, path.into(), application.tag.clone()));

src/context.rs

@@ -54,32 +54,46 @@ impl CommandRegistry {
         }
     }
 
-    pub(crate) fn get_command(&self, name: &str) -> Option<Arc<Command>> {
+    pub(crate) fn get_command(&self, name: &str) -> Result<Option<Arc<Command>>, ShellError> {
-        let registry = self.registry.lock().unwrap();
+        let registry = self.registry.lock().map_err(|_| {
+            ShellError::untagged_runtime_error("Internal error: get_command could not get mutex")
+        })?;
 
-        registry.get(name).cloned()
+        Ok(registry.get(name).cloned())
     }
 
     pub(crate) fn expect_command(&self, name: &str) -> Result<Arc<Command>, ShellError> {
-        self.get_command(name).ok_or_else(|| {
+        self.get_command(name)?.ok_or_else(|| {
             ShellError::untagged_runtime_error(format!("Could not load command: {}", name))
         })
     }
 
-    pub(crate) fn has(&self, name: &str) -> bool {
+    pub(crate) fn has(&self, name: &str) -> Result<bool, ShellError> {
-        let registry = self.registry.lock().unwrap();
+        let registry = self.registry.lock().map_err(|_| {
+            ShellError::untagged_runtime_error("Internal error: has could not get mutex")
+        })?;
 
-        registry.contains_key(name)
+        Ok(registry.contains_key(name))
     }
 
-    pub(crate) fn insert(&mut self, name: impl Into<String>, command: Arc<Command>) {
+    pub(crate) fn insert(
-        let mut registry = self.registry.lock().unwrap();
+        &mut self,
+        name: impl Into<String>,
+        command: Arc<Command>,
+    ) -> Result<(), ShellError> {
+        let mut registry = self.registry.lock().map_err(|_| {
+            ShellError::untagged_runtime_error("Internal error: insert could not get mutex")
+        })?;
+
         registry.insert(name.into(), command);
+        Ok(())
     }
 
-    pub(crate) fn names(&self) -> Vec<String> {
+    pub(crate) fn names(&self) -> Result<Vec<String>, ShellError> {
-        let registry = self.registry.lock().unwrap();
+        let registry = self.registry.lock().map_err(|_| {
-        registry.keys().cloned().collect()
+            ShellError::untagged_runtime_error("Internal error: names could not get mutex")
+        })?;
+        Ok(registry.keys().cloned().collect())
     }
 }
 
@@ -175,13 +189,15 @@ impl Context {
         block(&mut *errors)
     }
 
-    pub fn add_commands(&mut self, commands: Vec<Arc<Command>>) {
+    pub fn add_commands(&mut self, commands: Vec<Arc<Command>>) -> Result<(), ShellError> {
         for command in commands {
-            self.registry.insert(command.name().to_string(), command);
+            self.registry.insert(command.name().to_string(), command)?;
-        }
         }
 
-    pub(crate) fn get_command(&self, name: &str) -> Option<Arc<Command>> {
+        Ok(())
+    }
+
+    pub(crate) fn get_command(&self, name: &str) -> Result<Option<Arc<Command>>, ShellError> {
         self.registry.get_command(name)
     }
 

src/data/base.rs

@@ -43,7 +43,7 @@ impl EvaluateTrait for Block {
             return Ok(UntaggedValue::nothing().into_value(&self.tag));
         }
 
-        let mut last = None;
+        let mut last = Ok(UntaggedValue::nothing().into_value(&self.tag));
 
         trace!(
             "EXPRS = {:?}",
@@ -54,15 +54,10 @@ impl EvaluateTrait for Block {
         );
 
         for expr in self.expressions.iter() {
-            last = Some(evaluate_baseline_expr(
+            last = evaluate_baseline_expr(&expr, &CommandRegistry::empty(), &scope, &self.source)
-                &expr,
-                &CommandRegistry::empty(),
-                &scope,
-                &self.source,
-            )?)
         }
 
-        Ok(last.unwrap())
+        last
     }
 
     fn clone_box(&self) -> Evaluate {
@@ -382,7 +377,7 @@ mod tests {
         let actual = sample
             .into_untagged_value()
             .replace_data_at_column_path(&field_path?.item, replacement)
-            .unwrap();
+            .ok_or_else(|| ShellError::untagged_runtime_error("Could not replace column"))?;
 
         assert_eq!(actual, row(indexmap! {"amigos".into() => string("jonas")}));
 
@@ -411,9 +406,15 @@ mod tests {
         let tag = replacement.tag.clone();
 
         let actual = sample
-            .into_value(tag.clone())
+            .into_value(&tag)
             .replace_data_at_column_path(&field_path?.item, replacement.clone())
-            .unwrap();
+            .ok_or_else(|| {
+                ShellError::labeled_error(
+                    "Could not replace column",
+                    "could not replace column",
+                    &tag,
+                )
+            })?;
 
         assert_eq!(
             actual,
@@ -467,7 +468,13 @@ mod tests {
         let actual = sample
             .into_value(tag.clone())
             .replace_data_at_column_path(&field_path?.item, replacement.clone())
-            .unwrap();
+            .ok_or_else(|| {
+                ShellError::labeled_error(
+                    "Could not replace column",
+                    "could not replace column",
+                    &tag,
+                )
+            })?;
 
         assert_eq!(
             actual,

src/lib.rs

@@ -1,4 +1,4 @@
-#![recursion_limit = "1024"]
+#![recursion_limit = "2048"]
 
 #[cfg(test)]
 #[macro_use]

src/shell/completer.rs

@@ -17,7 +17,7 @@ impl NuCompleter {
         pos: usize,
         context: &rustyline::Context,
     ) -> rustyline::Result<(usize, Vec<rustyline::completion::Pair>)> {
-        let commands: Vec<String> = self.commands.names();
+        let commands: Vec<String> = self.commands.names().unwrap_or_else(|_| vec![]);
 
         let line_chars: Vec<_> = line[..pos].chars().collect();
 

src/shell/filesystem_shell.rs

@@ -1047,7 +1047,13 @@ impl Shell for FilesystemShell {
                     }
 
                     if trash.item {
-                        SendToTrash::remove(path).unwrap();
+                        SendToTrash::remove(path).map_err(|_| {
+                            ShellError::labeled_error(
+                                "Could not move file to trash",
+                                "could not move to trash",
+                                target.tag(),
+                            )
+                        })?;
                     } else if path.is_dir() {
                         std::fs::remove_dir_all(&path)?;
                     } else if path.is_file() {

src/shell/help_shell.rs

@@ -21,13 +21,13 @@ pub struct HelpShell {
 }
 
 impl HelpShell {
-    pub fn index(registry: &CommandRegistry) -> Result<HelpShell, std::io::Error> {
+    pub fn index(registry: &CommandRegistry) -> Result<HelpShell, ShellError> {
         let mut cmds = TaggedDictBuilder::new(Tag::unknown());
         let mut specs = Vec::new();
 
-        for cmd in registry.names() {
+        for cmd in registry.names()? {
             let mut spec = TaggedDictBuilder::new(Tag::unknown());
-            let value = command_dict(registry.get_command(&cmd).unwrap(), Tag::unknown());
+            let value = command_dict(registry.get_command(&cmd)?.unwrap(), Tag::unknown());
 
             spec.insert_untagged("name", cmd);
             spec.insert_untagged(
@@ -51,10 +51,7 @@ impl HelpShell {
         })
     }
 
-    pub fn for_command(
+    pub fn for_command(cmd: Value, registry: &CommandRegistry) -> Result<HelpShell, ShellError> {
-        cmd: Value,
-        registry: &CommandRegistry,
-    ) -> Result<HelpShell, std::io::Error> {
         let mut sh = HelpShell::index(&registry)?;
 
         if let Value {

tests/commands/cd.rs

@@ -96,7 +96,7 @@ fn filesystem_change_to_home_directory() {
             "#
         );
 
-        assert_eq!(PathBuf::from(actual), dirs::home_dir().unwrap());
+        assert_eq!(Some(PathBuf::from(actual)), dirs::home_dir());
     })
 }