refactor: remove unwrap/expect

Refactor to properly handle possible errors. Signed-off-by: Bruce D'Arcus <bdarcus@gmail.com>
bdarcus · Jan 17, 2024 · bbc6e15 · bbc6e15
1 parent 2c7a5e4
commit bbc6e15
Show file tree

Hide file tree

Showing 12 changed files with 107 additions and 59 deletions.
diff --git a/cli/Cargo.toml b/cli/Cargo.toml
@@ -20,7 +20,6 @@ schemars = "0.8"
 serde_json = "1.0"
 csln = { path = "../csln", package = "csln" }
 processor = { path = "../processor", package = "csln-processor" }
+anyhow = "1.0.79"
 
-[lints]
-workspace = true
 
diff --git a/cli/src/main.rs b/cli/src/main.rs
@@ -1,3 +1,4 @@
+use anyhow::Context;
 use clap::Parser;
 use csln::bibliography::InputBibliography as Bibliography;
 use csln::citation::Citations;
@@ -24,16 +25,20 @@ pub struct Opts {
 
 fn main() {
     let opts = Opts::parse();
-    let style: Style = Style::from_file(&opts.style);
+    let style = Style::from_file(&opts.style).context("Style file?");
     let bibliography: Bibliography = Bibliography::from_file(&opts.bibliography);
     let citations: Citations = if opts.citation.is_none() {
         Citations::default()
     } else {
         Citations::from_file(&opts.citation.unwrap_or_default())
     };
-    let locale = csln::style::locale::Locale::from_file(&opts.locale);
+    let locale =
+        csln::style::locale::Locale::from_file(&opts.locale).context("Locale file?");
     let processor: Processor = Processor::new(style, bibliography, citations, locale);
     let rendered_refs = processor.process_references();
     //println!("{}", refs_to_string(rendered_refs));
-    println!("{}", serde_json::to_string_pretty(&rendered_refs).unwrap());
+    match serde_json::to_string_pretty(&rendered_refs) {
+        Ok(json) => println!("{}", json),
+        Err(_) => eprintln!("Failed to serialize rendered_refs to JSON"),
+    }
 }
diff --git a/cli/src/makeschemas.rs b/cli/src/makeschemas.rs
@@ -9,7 +9,7 @@ use csln::style::locale::Locale;
 use csln::style::Style;
 
 fn main() {
-    fs::create_dir_all("schemas").unwrap();
+    fs::create_dir_all("schemas").expect("Failed to create directory 'schemas'");
 
     let style_schema = schema_for!(Style);
     let citation_schema = schema_for!(CitationList);

diff --git a/csln/Cargo.toml b/csln/Cargo.toml
@@ -26,6 +26,7 @@ chrono = { version = "0.4", features = ["unstable-locales"] }
 unic-langid = { version = "0.9.1", features = ["serde"] }
 itertools = "0.11.0"
 rayon = "1.7.0"
+anyhow = "1.0.79"
 #icu = { version = "1.2.0", features = ["icu_datetime_experimental"] }
 #icu_testdata = { version = "1.2.0", features = ["icu_datetime_experimental"] }
 #indexmap = { version = "2.0.0", features = ["std"] }

diff --git a/csln/src/bibliography/mod.rs b/csln/src/bibliography/mod.rs
@@ -11,14 +11,29 @@ pub type InputBibliography = HashMap<String, InputReference>;
 impl HasFile for InputBibliography {
     /// Load and parse a YAML or JSON bibliography file.
     fn from_file(bib_path: &str) -> InputBibliography {
-        let contents =
-            fs::read_to_string(bib_path).expect("Failed to read bibliography file");
-        if bib_path.ends_with(".json") {
-            serde_json::from_str(&contents).expect("Failed to parse JSON")
+        let contents = match fs::read_to_string(bib_path) {
+            Ok(contents) => contents,
+            Err(error) => panic!("Failed to read file: {}", error),
+        };
+        let bibliography = if bib_path.ends_with(".json") {
+            match serde_json::from_str(&contents) {
+                Ok(bibliography) => bibliography,
+                Err(_) => {
+                    println!("Failed to parse JSON");
+                    return HashMap::new();
+                }
+            }
         } else if bib_path.ends_with(".yaml") || bib_path.ends_with(".yml") {
-            serde_yaml::from_str(&contents).expect("Failed to parse YAML")
+            match serde_yaml::from_str(&contents) {
+                Ok(bibliography) => bibliography,
+                Err(_) => {
+                    println!("Failed to parse YAML");
+                    return HashMap::new();
+                }
+            }
         } else {
-            panic!("Style file must be in YAML or JSON format")
-        }
+            panic!("Bibliography file must be in YAML or JSON format")
+        };
+        bibliography
     }
 }
diff --git a/csln/src/citation/mod.rs b/csln/src/citation/mod.rs
@@ -5,15 +5,28 @@ use std::fs;
 
 impl HasFile for Citations {
     fn from_file(citations_path: &str) -> Citations {
-        let contents =
-            fs::read_to_string(citations_path).expect("Failed to read citations file");
-        if citations_path.ends_with(".json") {
-            serde_json::from_str(&contents).expect("Failed to parse JSON")
+        let contents = fs::read_to_string(citations_path)
+            .unwrap_or_else(|_| panic!("Failed to read citations file"));
+        let citations = if citations_path.ends_with(".json") {
+            match serde_json::from_str(&contents) {
+                Ok(citations) => citations,
+                Err(_) => {
+                    println!("Failed to parse JSON");
+                    return Citations::default();
+                }
+            }
         } else if citations_path.ends_with(".yaml") || citations_path.ends_with(".yml") {
-            serde_yaml::from_str(&contents).expect("Failed to parse YAML")
+            match serde_yaml::from_str(&contents) {
+                Ok(citations) => citations,
+                Err(_) => {
+                    println!("Failed to parse YAML");
+                    return Citations::default();
+                }
+            }
         } else {
             panic!("Citations file must be in YAML or JSON format")
-        }
+        };
+        citations
     }
 }
 

diff --git a/csln/src/style/locale.rs b/csln/src/style/locale.rs
@@ -1,6 +1,7 @@
+use anyhow::{Context, Result};
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
-use std::{collections::HashMap, fs};
+use std::{collections::HashMap, fs, path::Path};
 //use unic_langid::LanguageIdentifier;
 
 #[derive(Debug, Default, Deserialize, Serialize, Clone, JsonSchema)]
@@ -31,16 +32,24 @@ pub struct Terms {
 }
 
 impl Locale {
-    pub fn from_file(locale_path: &str) -> Locale {
-        let contents =
-            fs::read_to_string(locale_path).expect("Failed to read locale file");
-        if locale_path.ends_with(".json") {
-            serde_json::from_str(&contents).expect("Failed to parse JSON")
-        } else if locale_path.ends_with(".yaml") || locale_path.ends_with(".yml") {
-            serde_yaml::from_str(&contents).expect("Failed to parse YAML")
+    pub fn from_file<P: AsRef<Path>>(path: P) -> Result<Self> {
+        let path = path.as_ref();
+        let contents = fs::read_to_string(path)
+            .with_context(|| format!("Failed to read file: {}", path.display()))?;
+
+        let locale = if path.extension().and_then(|s| s.to_str()) == Some("json") {
+            serde_json::from_str(&contents).with_context(|| {
+                format!("Failed to parse JSON from file: {}", path.display())
+            })?
+        } else if path.extension().and_then(|s| s.to_str()) == Some("yaml") {
+            serde_yaml::from_str(&contents).with_context(|| {
+                format!("Failed to parse YAML from file: {}", path.display())
+            })?
         } else {
-            panic!("Locale file must be in YAML or JSON format")
-        }
+            return Err(anyhow::anyhow!("Unsupported file extension"));
+        };
+
+        Ok(locale)
     }
 }
 

diff --git a/csln/src/style/mod.rs b/csln/src/style/mod.rs
@@ -10,22 +10,25 @@ use std::fs;
 
 pub mod locale;
 pub mod options;
+use anyhow::{Context, Result};
 use options::Config;
 
 pub mod template;
 use template::TemplateComponent;
 
 impl Style {
     /// Load and parse a YAML or JSON style file.
-    pub fn from_file(style_path: &str) -> Style {
-        let contents = fs::read_to_string(style_path).expect("Failed to read style file");
-        if style_path.ends_with(".json") {
-            serde_json::from_str(&contents).expect("Failed to parse JSON")
+    pub fn from_file(style_path: &str) -> Result<Style> {
+        let contents =
+            fs::read_to_string(style_path).context("Failed to read style file")?;
+        let style: Style = if style_path.ends_with(".json") {
+            serde_json::from_str(&contents).context("Failed to parse JSON")
         } else if style_path.ends_with(".yaml") || style_path.ends_with(".yml") {
-            serde_yaml::from_str(&contents).expect("Failed to parse YAML")
+            serde_yaml::from_str(&contents).context("Failed to parse YAML")
         } else {
-            panic!("Style file must be in YAML or JSON format")
-        }
+            anyhow::bail!("Unknown file extension")
+        }?;
+        Ok(style)
     }
 }
 

diff --git a/csln/src/style/options.rs b/csln/src/style/options.rs
@@ -99,7 +99,7 @@ fn author_date_config() {
     let sort = config.sort.unwrap_or_default();
     assert_eq!(sort.template[0].key, SortKey::Author);
     assert_eq!(sort.template[1].key, SortKey::Year);
-    assert!(config.disambiguate.unwrap().year_suffix);
+    assert!(config.disambiguate.unwrap_or_default().year_suffix);
 }
 
 #[derive(JsonSchema, Debug, PartialEq, Clone, Serialize, Deserialize)]

diff --git a/processor/benches/proc_bench.rs b/processor/benches/proc_bench.rs
@@ -7,7 +7,13 @@ use csln_processor::Processor;
 use std::time::Duration;
 
 fn proc_benchmark(c: &mut Criterion) {
-    let style: Style = Style::from_file("examples/style.csl.yaml");
+    let style = match Style::from_file("examples/style.csl.yaml") {
+        Ok(style) => style,
+        Err(_) => {
+            println!("Failed to load style");
+            return;
+        }
+    };
     let bibliography: Bibliography = Bibliography::from_file("examples/ex1.bib.yaml");
     let locale = csln::style::locale::Locale::from_file("locales/locale-en.yaml");
     let citations: Vec<Citation> = Vec::new();

diff --git a/processor/src/lib.rs b/processor/src/lib.rs
@@ -21,6 +21,7 @@ use rayon::prelude::*;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 //use std::cmp::Ordering;
+//use anyhow::Result;
 use std::collections::HashMap;
 use std::fmt::{self, Debug, Display, Formatter};
 use std::option::Option;
@@ -438,7 +439,8 @@ impl ComponentValues for TemplateContributor {
                 } else {
                     // TODO generalize the substitution
                     let add_role_form =
-                        options.global.substitute.clone().unwrap().contributor_role_form;
+                        // REVIEW is this correct?
+                        options.global.substitute.clone()?.contributor_role_form;
                     let editor = reference.editor()?;
                     let editor_length = editor.names(options.global.clone(), true).len();
                     // get the role string; if it's in fact author, it will be None
@@ -449,16 +451,8 @@ impl ComponentValues for TemplateContributor {
                             role_form,
                             editor_length,
                         )
-                        // FIXME
-                        .unwrap()
                     });
-                    let suffix_padded = suffix.and_then(|s| {
-                        if s.is_empty() {
-                            None
-                        } else {
-                            Some(format!(" {}", s))
-                        }
-                    }); // TODO extract this into separate method
+                    let suffix_padded = suffix.and_then(|s| Some(format!(" {}", s?))); // TODO extract this into separate method
                     Some(ProcValues {
                         value: editor.format(options.global.clone(), locale),
                         prefix: None,
@@ -565,7 +559,11 @@ impl ComponentValues for TemplateDate {
 
         // TODO: implement this along with localized dates
         fn _config_fmt(options: &RenderOptions) -> DateTimeFormatterOptions {
-            match options.global.dates.as_ref().unwrap().month {
+            let date_options = match options.global.dates.clone() {
+                Some(dates) => dates,
+                None => return DateTimeFormatterOptions::default(), // or handle the None case accordingly
+            };
+            match date_options.month {
                 MonthFormat::Long => todo!("long"),
                 MonthFormat::Short => todo!("short"),
                 MonthFormat::Numeric => todo!("numeric"),
@@ -584,7 +582,7 @@ impl ComponentValues for TemplateDate {
             // TODO need to check form here also
             // && self.form == style::template::DateForm::Year
             // REVIEW: ugly, and needs to be smarter
-            && options.global.processing.clone().unwrap().config().disambiguate.unwrap().year_suffix
+            && options.global.processing.clone().unwrap_or_default().config().disambiguate.unwrap_or_default().year_suffix
             && formatted_date.len() == 4
         {
             int_to_letter((hints.group_index % 26) as u32)
@@ -673,7 +671,10 @@ impl Processor {
     ) -> Option<ProcCitationItem> {
         let citation_style = self.style.citation.clone();
         // FIXME below is returning None
-        let reference = self.get_reference(&citation_item.ref_id).unwrap();
+        let reference = match self.get_reference(&citation_item.ref_id) {
+            Ok(reference) => reference,
+            Err(_) => return None, // or handle the error in a different way
+        };
         let proc_template =
             self.process_template(&reference, citation_style?.template.as_slice());
         println!("proc_template: {:?}", proc_template);
@@ -685,9 +686,9 @@ impl Processor {
         &self,
         reference: &InputReference,
     ) -> Vec<ProcTemplateComponent> {
-        let bibliography_style = self.style.bibliography.clone();
+        let bibliography_style = self.style.bibliography.clone().unwrap();
         // TODO bibliography should probably be Optional
-        self.process_template(reference, bibliography_style.unwrap().template.as_slice())
+        self.process_template(reference, bibliography_style.template.as_slice())
     }
 
     fn get_render_options(&self, style: Style, locale: Locale) -> RenderOptions {

diff --git a/processor/tests/processor_test.rs b/processor/tests/processor_test.rs
@@ -20,12 +20,8 @@ mod tests {
             csln::bibliography::InputBibliography::from_file("examples/ex1.bib.yaml");
         let citations: Citations =
             csln::citation::Citations::from_file("examples/citation.yaml");
-        let processor = csln_processor::Processor::new(
-            style.clone(),
-            bibliography.clone(),
-            citations.clone(),
-            locale.clone(),
-        );
+        let processor =
+            csln_processor::Processor::new(style, bibliography, citations, locale);
 
         TestFixture { style, locale, bibliography, citations, processor }
     }