Yet more ununwraps

crates/nu-parser/src/hir/baseline_parse/tests.rs

@@ -103,11 +103,11 @@ impl TestRegistry {
 }
 
 impl SignatureRegistry for TestRegistry {
-    fn has(&self, name: &str) -> bool {
-        self.signatures.contains_key(name)
+    fn has(&self, name: &str) -> Result<bool, ShellError> {
+        Ok(self.signatures.contains_key(name))
     }
-    fn get(&self, name: &str) -> Option<Signature> {
-        self.signatures.get(name).cloned()
+    fn get(&self, name: &str) -> Result<Option<Signature>, ShellError> {
+        Ok(self.signatures.get(name).cloned())
     }
 }
 

crates/nu-parser/src/hir/syntax_shape.rs

@@ -127,8 +127,8 @@ impl ExpandExpression for SyntaxShape {
 }
 
 pub trait SignatureRegistry {
-    fn has(&self, name: &str) -> bool;
-    fn get(&self, name: &str) -> Option<Signature>;
+    fn has(&self, name: &str) -> Result<bool, ShellError>;
+    fn get(&self, name: &str) -> Result<Option<Signature>, ShellError>;
 }
 
 #[derive(Getters, new)]
@@ -673,16 +673,26 @@ impl FallibleColorSyntax for CommandHeadShape {
                 UnspannedAtomicToken::Word { text } => {
                     let name = text.slice(context.source);
 
-                    if context.registry.has(name) {
+                    if context.registry.has(name)? {
                         // If the registry has the command, color it as an internal command
                         token_nodes.color_shape(FlatShape::InternalCommand.spanned(text));
-                        let signature = context.registry.get(name).ok_or_else(|| {
-                            ShellError::labeled_error(
-                                "Internal error: could not load signature from registry",
-                                "could not load from registry",
-                                text,
-                            )
-                        })?;
+                        let signature = context
+                            .registry
+                            .get(name)
+                            .map_err(|_| {
+                                ShellError::labeled_error(
+                                    "Internal error: could not load signature from registry",
+                                    "could not load from registry",
+                                    text,
+                                )
+                            })?
+                            .ok_or_else(|| {
+                                ShellError::labeled_error(
+                                    "Internal error: could not load signature from registry",
+                                    "could not load from registry",
+                                    text,
+                                )
+                            })?;
                         Ok(CommandHeadKind::Internal(signature))
                     } else {
                         // Otherwise, color it as an external command
@@ -721,10 +731,14 @@ impl ExpandSyntax for CommandHeadShape {
                     },
                     UnspannedToken::Bare => {
                         let name = token_span.slice(context.source);
-                        if context.registry.has(name) {
-                            let signature = context.registry.get(name).ok_or_else(|| {
-                                ParseError::internal_error(name.spanned(token_span))
-                            })?;
+                        if context.registry.has(name)? {
+                            let signature = context
+                                .registry
+                                .get(name)
+                                .map_err(|_| ParseError::internal_error(name.spanned(token_span)))?
+                                .ok_or_else(|| {
+                                    ParseError::internal_error(name.spanned(token_span))
+                                })?;
                             CommandSignature::Internal(signature.spanned(token_span))
                         } else {
                             CommandSignature::External(token_span)

src/commands/classified/internal.rs

@@ -26,7 +26,7 @@ pub(crate) async fn run_internal_command(
 
     let result = {
         context.run_command(
-            internal_command,
+            internal_command?,
             command.name_tag.clone(),
             command.args.clone(),
             &source,

src/commands/split_by.rs

@@ -156,6 +156,7 @@ mod tests {
     use crate::commands::group_by::group;
     use crate::commands::split_by::split;
     use indexmap::IndexMap;
+    use nu_errors::ShellError;
     use nu_protocol::{UntaggedValue, Value};
     use nu_source::*;
 
@@ -171,9 +172,9 @@ mod tests {
         UntaggedValue::table(list).into_untagged_value()
     }
 
-    fn nu_releases_grouped_by_date() -> Value {
+    fn nu_releases_grouped_by_date() -> Result<Value, ShellError> {
         let key = String::from("date").tagged_unknown();
-        group(&key, nu_releases_commiters(), Tag::unknown()).unwrap()
+        group(&key, nu_releases_commiters(), Tag::unknown())
     }
 
     fn nu_releases_commiters() -> Vec<Value> {
@@ -209,11 +210,11 @@ mod tests {
     }
 
     #[test]
-    fn splits_inner_tables_by_key() {
+    fn splits_inner_tables_by_key() -> Result<(), ShellError> {
         let for_key = String::from("country").tagged_unknown();
 
         assert_eq!(
-            split(&for_key, &nu_releases_grouped_by_date(), Tag::unknown()).unwrap(),
+            split(&for_key, &nu_releases_grouped_by_date()?, Tag::unknown())?,
             UntaggedValue::row(indexmap! {
                 "EC".into() => row(indexmap! {
                     "August 23-2019".into() => table(&[
@@ -250,6 +251,8 @@ mod tests {
                 })
             }).into_untagged_value()
         );
+
+        Ok(())
     }
 
     #[test]

src/commands/to_json.rs

@@ -39,12 +39,28 @@ pub fn value_to_json_value(v: &Value) -> Result<serde_json::Value, ShellError> {
         UntaggedValue::Primitive(Primitive::Date(d)) => serde_json::Value::String(d.to_string()),
         UntaggedValue::Primitive(Primitive::EndOfStream) => serde_json::Value::Null,
         UntaggedValue::Primitive(Primitive::BeginningOfStream) => serde_json::Value::Null,
-        UntaggedValue::Primitive(Primitive::Decimal(f)) => serde_json::Value::Number(
-            serde_json::Number::from_f64(
-                f.to_f64().expect("TODO: What about really big decimals?"),
-            )
-            .unwrap(),
-        ),
+        UntaggedValue::Primitive(Primitive::Decimal(f)) => {
+            if let Some(f) = f.to_f64() {
+                if let Some(num) = serde_json::Number::from_f64(
+                    f.to_f64().expect("TODO: What about really big decimals?"),
+                ) {
+                    serde_json::Value::Number(num)
+                } else {
+                    return Err(ShellError::labeled_error(
+                        "Could not convert value to decimal number",
+                        "could not convert to decimal",
+                        &v.tag,
+                    ));
+                }
+            } else {
+                return Err(ShellError::labeled_error(
+                    "Could not convert value to decimal number",
+                    "could not convert to decimal",
+                    &v.tag,
+                ));
+            }
+        }
+
         UntaggedValue::Primitive(Primitive::Int(i)) => {
             serde_json::Value::Number(serde_json::Number::from(CoerceInto::<i64>::coerce_into(
                 i.tagged(&v.tag),

src/context.rs

@@ -17,13 +17,25 @@ pub struct CommandRegistry {
 }
 
 impl SignatureRegistry for CommandRegistry {
-    fn has(&self, name: &str) -> bool {
-        let registry = self.registry.lock().unwrap();
-        registry.contains_key(name)
+    fn has(&self, name: &str) -> Result<bool, ShellError> {
+        if let Ok(registry) = self.registry.lock() {
+            Ok(registry.contains_key(name))
+        } else {
+            Err(ShellError::untagged_runtime_error(format!(
+                "Could not load from registry: {}",
+                name
+            )))
+        }
     }
-    fn get(&self, name: &str) -> Option<Signature> {
-        let registry = self.registry.lock().unwrap();
-        registry.get(name).map(|command| command.signature())
+    fn get(&self, name: &str) -> Result<Option<Signature>, ShellError> {
+        if let Ok(registry) = self.registry.lock() {
+            Ok(registry.get(name).map(|command| command.signature()))
+        } else {
+            Err(ShellError::untagged_runtime_error(format!(
+                "Could not get from registry: {}",
+                name
+            )))
+        }
     }
 }
 
@@ -48,8 +60,10 @@ impl CommandRegistry {
         registry.get(name).cloned()
     }
 
-    pub(crate) fn expect_command(&self, name: &str) -> Arc<Command> {
-        self.get_command(name).unwrap()
+    pub(crate) fn expect_command(&self, name: &str) -> Result<Arc<Command>, ShellError> {
+        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 {
@@ -171,7 +185,7 @@ impl Context {
         self.registry.get_command(name)
     }
 
-    pub(crate) fn expect_command(&self, name: &str) -> Arc<Command> {
+    pub(crate) fn expect_command(&self, name: &str) -> Result<Arc<Command>, ShellError> {
         self.registry.expect_command(name)
     }