Clean up some unwraps (#1147)

crates/nu-build/src/lib.rs

@@ -13,10 +13,10 @@ lazy_static! {
 
 // got from https://github.com/mitsuhiko/insta/blob/b113499249584cb650150d2d01ed96ee66db6b30/src/runtime.rs#L67-L88
 
-fn get_cargo_workspace(manifest_dir: &str) -> Option<&Path> {
+fn get_cargo_workspace(manifest_dir: &str) -> Result<Option<&Path>, Box<dyn std::error::Error>> {
-    let mut workspaces = WORKSPACES.lock().unwrap();
+    let mut workspaces = WORKSPACES.lock()?;
     if let Some(rv) = workspaces.get(manifest_dir) {
-        Some(rv)
+        Ok(Some(rv))
     } else {
         #[derive(Deserialize)]
         struct Manifest {
@@ -26,12 +26,11 @@ fn get_cargo_workspace(manifest_dir: &str) -> Option<&Path> {
             .arg("metadata")
             .arg("--format-version=1")
             .current_dir(manifest_dir)
-            .output()
+            .output()?;
-            .unwrap();
+        let manifest: Manifest = serde_json::from_slice(&output.stdout)?;
-        let manifest: Manifest = serde_json::from_slice(&output.stdout).unwrap();
         let path = Box::leak(Box::new(PathBuf::from(manifest.workspace_root)));
         workspaces.insert(manifest_dir.to_string(), path.as_path());
-        workspaces.get(manifest_dir).cloned()
+        Ok(workspaces.get(manifest_dir).cloned())
     }
 }
 
@@ -43,7 +42,7 @@ struct Feature {
 }
 
 pub fn build() -> Result<(), Box<dyn std::error::Error>> {
-    let input = env::var("CARGO_MANIFEST_DIR").unwrap();
+    let input = env::var("CARGO_MANIFEST_DIR")?;
 
     let all_on = env::var("NUSHELL_ENABLE_ALL_FLAGS").is_ok();
     let flags: HashSet<String> = env::var("NUSHELL_ENABLE_FLAGS")
@@ -56,7 +55,7 @@ pub fn build() -> Result<(), Box<dyn std::error::Error>> {
         );
     }
 
-    let workspace = match get_cargo_workspace(&input) {
+    let workspace = match get_cargo_workspace(&input)? {
         // If the crate is being downloaded from crates.io, it won't have a workspace root, and that's ok
         None => return Ok(()),
         Some(workspace) => workspace,

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

@@ -3,14 +3,14 @@ use crate::parse::token_tree_builder::TokenTreeBuilder as b;
 use crate::Span;
 
 #[test]
-fn supplies_tokens() {
+fn supplies_tokens() -> Result<(), Box<dyn std::error::Error>> {
     let tokens = b::token_list(vec![b::var("it"), b::op("."), b::bare("cpu")]);
     let (tokens, _) = b::build(tokens);
 
     let tokens = tokens.expect_list();
     let mut iterator = TokensIterator::all(tokens, Span::unknown());
 
-    iterator.next().unwrap().expect_var();
+    iterator.next()?.expect_var();
-    iterator.next().unwrap().expect_dot();
+    iterator.next()?.expect_dot();
-    iterator.next().unwrap().expect_bare();
+    iterator.next()?.expect_bare();
 }

crates/nu_plugin_binaryview/src/main.rs

@@ -195,40 +195,32 @@ struct RawImageBuffer {
     buffer: Vec<u8>,
 }
 
-fn load_from_png_buffer(buffer: &[u8]) -> Option<RawImageBuffer> {
+fn load_from_png_buffer(buffer: &[u8]) -> Result<RawImageBuffer, Box<dyn std::error::Error>> {
     use image::ImageDecoder;
 
-    let decoder = image::png::PNGDecoder::new(buffer);
+    let decoder = image::png::PNGDecoder::new(buffer)?;
-    if decoder.is_err() {
-        return None;
-    }
-    let decoder = decoder.unwrap();
 
     let dimensions = decoder.dimensions();
     let colortype = decoder.colortype();
-    let buffer = decoder.read_image().unwrap();
+    let buffer = decoder.read_image()?;
 
-    Some(RawImageBuffer {
+    Ok(RawImageBuffer {
         dimensions,
         colortype,
         buffer,
     })
 }
 
-fn load_from_jpg_buffer(buffer: &[u8]) -> Option<RawImageBuffer> {
+fn load_from_jpg_buffer(buffer: &[u8]) -> Result<RawImageBuffer, Box<dyn std::error::Error>> {
     use image::ImageDecoder;
 
-    let decoder = image::jpeg::JPEGDecoder::new(buffer);
+    let decoder = image::jpeg::JPEGDecoder::new(buffer)?;
-    if decoder.is_err() {
-        return None;
-    }
-    let decoder = decoder.unwrap();
 
     let dimensions = decoder.dimensions();
     let colortype = decoder.colortype();
-    let buffer = decoder.read_image().unwrap();
+    let buffer = decoder.read_image()?;
 
-    Some(RawImageBuffer {
+    Ok(RawImageBuffer {
         dimensions,
         colortype,
         buffer,
@@ -242,16 +234,16 @@ pub fn view_contents(
 ) -> Result<(), Box<dyn std::error::Error>> {
     let mut raw_image_buffer = load_from_png_buffer(buffer);
 
-    if raw_image_buffer.is_none() {
+    if raw_image_buffer.is_err() {
         raw_image_buffer = load_from_jpg_buffer(buffer);
     }
 
-    if raw_image_buffer.is_none() {
+    if raw_image_buffer.is_err() {
         //Not yet supported
         outln!("{:?}", buffer.hex_dump());
         return Ok(());
     }
-    let raw_image_buffer = raw_image_buffer.unwrap();
+    let raw_image_buffer = raw_image_buffer?;
 
     let mut render_context: RenderContext = RenderContext::blank(lores_mode);
     let _ = render_context.update();
@@ -264,7 +256,7 @@ pub fn view_contents(
                 raw_image_buffer.dimensions.1 as u32,
                 raw_image_buffer.buffer,
             )
-            .unwrap();
+            .ok_or("Cannot convert image data")?;
 
             let resized_img = image::imageops::resize(
                 &img,
@@ -287,7 +279,7 @@ pub fn view_contents(
                 raw_image_buffer.dimensions.1 as u32,
                 raw_image_buffer.buffer,
             )
-            .unwrap();
+            .ok_or("Cannot convert image data")?;
 
             let resized_img = image::imageops::resize(
                 &img,
@@ -374,8 +366,8 @@ pub fn view_contents_interactive(
             let image_buffer = nes.image_buffer();
 
             let slice = unsafe { std::slice::from_raw_parts(image_buffer, 256 * 240 * 4) };
-            let img =
+            let img = image::ImageBuffer::<image::Rgba<u8>, &[u8]>::from_raw(256, 240, slice)
-                image::ImageBuffer::<image::Rgba<u8>, &[u8]>::from_raw(256, 240, slice).unwrap();
+                .ok_or("Cannot convert image data")?;
             let resized_img = image::imageops::resize(
                 &img,
                 render_context.width as u32,

crates/nu_plugin_fetch/src/main.rs

@@ -63,7 +63,13 @@ impl Plugin for Fetch {
 
     fn filter(&mut self, value: Value) -> Result<Vec<ReturnValue>, ShellError> {
         Ok(vec![block_on(fetch_helper(
-            &self.path.clone().unwrap(),
+            &self.path.clone().ok_or_else(|| {
+                ShellError::labeled_error(
+                    "internal error: path not set",
+                    "path not set",
+                    &value.tag,
+                )
+            })?,
             self.has_raw,
             value,
         ))])
@@ -93,7 +99,7 @@ async fn fetch_helper(path: &Value, has_raw: bool, row: Value) -> ReturnValue {
     if let Err(e) = result {
         return Err(e);
     }
-    let (file_extension, contents, contents_tag) = result.unwrap();
+    let (file_extension, contents, contents_tag) = result?;
 
     let file_extension = if has_raw {
         None
@@ -131,7 +137,13 @@ pub async fn fetch(
     match response {
         Ok(mut r) => match r.headers().get("content-type") {
             Some(content_type) => {
-                let content_type = Mime::from_str(content_type).unwrap();
+                let content_type = Mime::from_str(content_type).map_err(|_| {
+                    ShellError::labeled_error(
+                        format!("MIME type unknown: {}", content_type),
+                        "given unknown MIME type",
+                        span,
+                    )
+                })?;
                 match (content_type.type_(), content_type.subtype()) {
                     (mime::APPLICATION, mime::XML) => Ok((
                         Some("xml".to_string()),
@@ -225,7 +237,13 @@ pub async fn fetch(
                     )),
                     (mime::TEXT, mime::PLAIN) => {
                         let path_extension = url::Url::parse(location)
-                            .unwrap()
+                            .map_err(|_| {
+                                ShellError::labeled_error(
+                                    format!("Cannot parse URL: {}", location),
+                                    "cannot parse",
+                                    span,
+                                )
+                            })?
                             .path_segments()
                             .and_then(|segments| segments.last())
                             .and_then(|name| if name.is_empty() { None } else { Some(name) })

crates/nu_plugin_inc/src/inc.rs

@@ -152,24 +152,27 @@ mod tests {
         use nu_plugin::test_helpers::value::string;
 
         #[test]
-        fn major() {
+        fn major() -> Result<(), Box<dyn std::error::Error>> {
             let mut inc = Inc::new();
             inc.for_semver(SemVerAction::Major);
-            assert_eq!(inc.apply("0.1.3").unwrap(), string("1.0.0").value);
+            assert_eq!(inc.apply("0.1.3")?, string("1.0.0").value);
+            Ok(())
         }
 
         #[test]
-        fn minor() {
+        fn minor() -> Result<(), Box<dyn std::error::Error>> {
             let mut inc = Inc::new();
             inc.for_semver(SemVerAction::Minor);
-            assert_eq!(inc.apply("0.1.3").unwrap(), string("0.2.0").value);
+            assert_eq!(inc.apply("0.1.3")?, string("0.2.0").value);
+            Ok(())
         }
 
         #[test]
-        fn patch() {
+        fn patch() -> Result<(), Box<dyn std::error::Error>> {
             let mut inc = Inc::new();
             inc.for_semver(SemVerAction::Patch);
-            assert_eq!(inc.apply("0.1.3").unwrap(), string("0.1.4").value);
+            assert_eq!(inc.apply("0.1.3")?, string("0.1.4").value);
+            Ok(())
         }
     }
 }

crates/nu_plugin_match/src/main.rs

@@ -13,11 +13,11 @@ struct Match {
 
 impl Match {
     #[allow(clippy::trivial_regex)]
-    fn new() -> Self {
+    fn new() -> Result<Self, Box<dyn std::error::Error>> {
-        Match {
+        Ok(Match {
             column: String::new(),
-            regex: Regex::new("").unwrap(),
+            regex: Regex::new("")?,
-        }
+        })
     }
 }
 
@@ -49,14 +49,20 @@ impl Plugin for Match {
             match &args[1] {
                 Value {
                     value: UntaggedValue::Primitive(Primitive::String(s)),
-                    ..
+                    tag,
                 } => {
-                    self.regex = Regex::new(s).unwrap();
+                    self.regex = Regex::new(s).map_err(|_| {
+                        ShellError::labeled_error(
+                            "Internal error while creating regex",
+                            "internal error created by pattern",
+                            tag,
+                        )
+                    })?;
                 }
                 Value { tag, .. } => {
                     return Err(ShellError::labeled_error(
                         "Unrecognized type in params",
-                        "value",
+                        "unexpected value",
                         tag,
                     ));
                 }
@@ -102,6 +108,7 @@ impl Plugin for Match {
     }
 }
 
-fn main() {
+fn main() -> Result<(), Box<dyn std::error::Error>> {
-    serve_plugin(&mut Match::new());
+    serve_plugin(&mut Match::new()?);
+    Ok(())
 }

crates/nu_plugin_post/src/main.rs

@@ -26,6 +26,7 @@ struct Post {
     user: Option<String>,
     password: Option<String>,
     headers: Vec<HeaderKind>,
+    tag: Tag,
 }
 
 impl Post {
@@ -37,6 +38,7 @@ impl Post {
             user: None,
             password: None,
             headers: vec![],
+            tag: Tag::unknown(),
         }
     }
 
@@ -61,15 +63,20 @@ impl Post {
             file => Some(file.clone()),
         };
 
-        self.user = call_info.args.get("user").map(|x| x.as_string().unwrap());
+        self.user = match call_info.args.get("user") {
+            Some(user) => Some(user.as_string()?),
+            None => None,
+        };
 
-        self.password = call_info
+        self.password = match call_info.args.get("password") {
-            .args
+            Some(password) => Some(password.as_string()?),
-            .get("password")
+            None => None,
-            .map(|x| x.as_string().unwrap());
+        };
 
         self.headers = get_headers(&call_info)?;
 
+        self.tag = call_info.name_tag;
+
         ReturnSuccess::value(UntaggedValue::nothing().into_untagged_value())
     }
 }
@@ -107,9 +114,13 @@ impl Plugin for Post {
 
     fn filter(&mut self, row: Value) -> Result<Vec<ReturnValue>, ShellError> {
         Ok(vec![block_on(post_helper(
-            &self.path.clone().unwrap(),
+            &self.path.clone().ok_or_else(|| {
+                ShellError::labeled_error("expected a 'path'", "expected a 'path'", &self.tag)
+            })?,
             self.has_raw,
-            &self.body.clone().unwrap(),
+            &self.body.clone().ok_or_else(|| {
+                ShellError::labeled_error("expected a 'body'", "expected a 'body'", &self.tag)
+            })?,
             self.user.clone(),
             self.password.clone(),
             &self.headers.clone(),
@@ -154,9 +165,7 @@ async fn post_helper(
     };
 
     let (file_extension, contents, contents_tag) =
-        post(&path_str, &body, user, password, &headers, path_tag.clone())
+        post(&path_str, &body, user, password, &headers, path_tag.clone()).await?;
-            .await
-            .unwrap();
 
     let file_extension = if has_raw {
         None
@@ -252,7 +261,13 @@ pub async fn post(
         match response {
             Ok(mut r) => match r.headers().get("content-type") {
                 Some(content_type) => {
-                    let content_type = Mime::from_str(content_type).unwrap();
+                    let content_type = Mime::from_str(content_type).map_err(|_| {
+                        ShellError::labeled_error(
+                            format!("Unknown MIME type: {}", content_type),
+                            "unknown MIME type",
+                            &tag,
+                        )
+                    })?;
                     match (content_type.type_(), content_type.subtype()) {
                         (mime::APPLICATION, mime::XML) => Ok((
                             Some("xml".to_string()),
@@ -332,7 +347,13 @@ pub async fn post(
                         )),
                         (mime::TEXT, mime::PLAIN) => {
                             let path_extension = url::Url::parse(location)
-                                .unwrap()
+                                .map_err(|_| {
+                                    ShellError::labeled_error(
+                                        format!("could not parse URL: {}", location),
+                                        "could not parse URL",
+                                        &tag,
+                                    )
+                                })?
                                 .path_segments()
                                 .and_then(|segments| segments.last())
                                 .and_then(|name| if name.is_empty() { None } else { Some(name) })
@@ -412,7 +433,13 @@ pub fn value_to_json_value(v: &Value) -> Result<serde_json::Value, ShellError> {
             serde_json::Number::from_f64(
                 f.to_f64().expect("TODO: What about really big decimals?"),
             )
-            .unwrap(),
+            .ok_or_else(|| {
+                ShellError::labeled_error(
+                    "Can not convert big decimal to f64",
+                    "cannot convert big decimal to f64",
+                    &v.tag,
+                )
+            })?,
         ),
         UntaggedValue::Primitive(Primitive::Int(i)) => {
             serde_json::Value::Number(serde_json::Number::from(CoerceInto::<i64>::coerce_into(
@@ -448,13 +475,22 @@ pub fn value_to_json_value(v: &Value) -> Result<serde_json::Value, ShellError> {
         UntaggedValue::Block(_) | UntaggedValue::Primitive(Primitive::Range(_)) => {
             serde_json::Value::Null
         }
-        UntaggedValue::Primitive(Primitive::Binary(b)) => serde_json::Value::Array(
+        UntaggedValue::Primitive(Primitive::Binary(b)) => {
-            b.iter()
+            let mut output = vec![];
-                .map(|x| {
+
-                    serde_json::Value::Number(serde_json::Number::from_f64(*x as f64).unwrap())
+            for item in b.iter() {
-                })
+                output.push(serde_json::Value::Number(
-                .collect(),
+                    serde_json::Number::from_f64(*item as f64).ok_or_else(|| {
-        ),
+                        ShellError::labeled_error(
+                            "Cannot create number from from f64",
+                            "cannot created number from f64",
+                            &v.tag,
+                        )
+                    })?,
+                ));
+            }
+            serde_json::Value::Array(output)
+        }
         UntaggedValue::Row(o) => {
             let mut m = serde_json::Map::new();
             for (k, v) in o.entries.iter() {

crates/nu_plugin_str/src/nu_plugin_str/mod.rs

@@ -52,7 +52,7 @@ impl Plugin for Str {
                         value: UntaggedValue::Primitive(Primitive::String(s)),
                         ..
                     } => {
-                        self.for_substring(s.to_string());
+                        self.for_substring(s.to_string())?;
                     }
                     _ => {
                         return Err(ShellError::labeled_error(
@@ -77,12 +77,30 @@ impl Plugin for Str {
         if args.has("find-replace") {
             if let Some(Value {
                 value: UntaggedValue::Table(arguments),
-                ..
+                tag,
             }) = args.get("find-replace")
             {
                 self.for_replace(ReplaceAction::FindAndReplace(
-                    arguments.get(0).unwrap().as_string()?,
+                    arguments
-                    arguments.get(1).unwrap().as_string()?,
+                        .get(0)
+                        .ok_or_else(|| {
+                            ShellError::labeled_error(
+                                "expected file and replace strings eg) [find replace]",
+                                "missing find-replace values",
+                                tag,
+                            )
+                        })?
+                        .as_string()?,
+                    arguments
+                        .get(1)
+                        .ok_or_else(|| {
+                            ShellError::labeled_error(
+                                "expected file and replace strings eg) [find replace]",
+                                "missing find-replace values",
+                                tag,
+                            )
+                        })?
+                        .as_string()?,
                 ));
             }
         }

crates/nu_plugin_str/src/strutils.rs

@@ -112,15 +112,21 @@ impl Str {
         }
     }
 
-    pub fn for_substring(&mut self, s: String) {
+    pub fn for_substring(&mut self, s: String) -> Result<(), ShellError> {
         let v: Vec<&str> = s.split(',').collect();
         let start: usize = match v[0] {
             "" => 0,
-            _ => v[0].trim().parse().unwrap(),
+            _ => v[0]
+                .trim()
+                .parse()
+                .map_err(|_| ShellError::untagged_runtime_error("Could not perform substring"))?,
         };
         let end: usize = match v[1] {
             "" => usize::max_value(),
-            _ => v[1].trim().parse().unwrap(),
+            _ => v[1]
+                .trim()
+                .parse()
+                .map_err(|_| ShellError::untagged_runtime_error("Could not perform substring"))?,
         };
         if start > end {
             self.log_error("End must be greater than or equal to Start");
@@ -129,6 +135,8 @@ impl Str {
         } else {
             self.log_error("can only apply one");
         }
+
+        Ok(())
     }
 
     pub fn for_replace(&mut self, mode: ReplaceAction) {
@@ -206,35 +214,39 @@ pub mod tests {
     use nu_plugin::test_helpers::value::{int, string};
 
     #[test]
-    fn downcases() {
+    fn downcases() -> Result<(), Box<dyn std::error::Error>> {
         let mut strutils = Str::new();
         strutils.for_downcase();
-        assert_eq!(strutils.apply("ANDRES").unwrap(), string("andres").value);
+        assert_eq!(strutils.apply("ANDRES")?, string("andres").value);
+        Ok(())
     }
 
     #[test]
-    fn upcases() {
+    fn upcases() -> Result<(), Box<dyn std::error::Error>> {
         let mut strutils = Str::new();
         strutils.for_upcase();
-        assert_eq!(strutils.apply("andres").unwrap(), string("ANDRES").value);
+        assert_eq!(strutils.apply("andres")?, string("ANDRES").value);
+        Ok(())
     }
 
     #[test]
-    fn converts_to_int() {
+    fn converts_to_int() -> Result<(), Box<dyn std::error::Error>> {
         let mut strutils = Str::new();
         strutils.for_to_int();
-        assert_eq!(strutils.apply("9999").unwrap(), int(9999 as i64).value);
+        assert_eq!(strutils.apply("9999")?, int(9999 as i64).value);
+        Ok(())
     }
 
     #[test]
-    fn replaces() {
+    fn replaces() -> Result<(), Box<dyn std::error::Error>> {
         let mut strutils = Str::new();
         strutils.for_replace(ReplaceAction::Direct("robalino".to_string()));
-        assert_eq!(strutils.apply("andres").unwrap(), string("robalino").value);
+        assert_eq!(strutils.apply("andres")?, string("robalino").value);
+        Ok(())
     }
 
     #[test]
-    fn find_and_replaces() {
+    fn find_and_replaces() -> Result<(), Box<dyn std::error::Error>> {
         let mut strutils = Str::new();
 
         strutils.for_replace(ReplaceAction::FindAndReplace(
@@ -242,9 +254,7 @@ pub mod tests {
             "jotandrehuda".to_string(),
         ));
 
-        assert_eq!(
+        assert_eq!(strutils.apply("wykittens")?, string("wyjotandrehuda").value);
-            strutils.apply("wykittens").unwrap(),
+        Ok(())
-            string("wyjotandrehuda").value
-        );
     }
 }

src/cli.rs

@@ -422,9 +422,11 @@ pub async fn cli() -> Result<(), Box<dyn Error>> {
         };
 
         let prompt = {
-            let bytes = strip_ansi_escapes::strip(&colored_prompt).unwrap();
+            if let Ok(bytes) = strip_ansi_escapes::strip(&colored_prompt) {
-
                 String::from_utf8_lossy(&bytes).to_string()
+            } else {
+                "> ".to_string()
+            }
         };
 
         rl.helper_mut().expect("No helper").colored_prompt = colored_prompt;