refactor: improve error handling

Still work to do, but incrementally improve error-handling to reduce
usage of unwrap and expect.

Also, consolidate file parsing around a Parsable trait.

Signed-off-by: Bruce D'Arcus <[email protected]>

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
 

cli/src/main.rs

@@ -1,9 +1,8 @@
+use anyhow::Context;
 use clap::Parser;
-use csln::bibliography::InputBibliography as Bibliography;
 use csln::citation::Citations;
-use csln::style::Style;
-use csln::HasFile;
-use processor::Processor;
+use csln::from_file;
+use processor::{ProcReferences, Processor};
 
 #[derive(Parser, Default, Debug)]
 #[clap(author = "Bruce D'Arcus", version, about = "A CLI for CSLN")]
@@ -16,24 +15,34 @@ pub struct Opts {
     bibliography: String,
     #[clap(short, long)]
     /// The optional path to the CSLN citation file
-    citation: Option<String>,
+    citations: Option<String>,
     #[clap(short, long)]
     /// The path to the CSLN locale file
     locale: String,
 }
 
 fn main() {
     let opts = Opts::parse();
-    let style: Style = Style::from_file(&opts.style);
-    let bibliography: Bibliography = Bibliography::from_file(&opts.bibliography);
-    let citations: Citations = if opts.citation.is_none() {
+    let style = from_file(&opts.style).context("Style file?");
+    let bibliography = from_file(&opts.bibliography).context("Bibliography file?");
+    let citations: Citations = if opts.citations.is_none() {
         Citations::default()
     } else {
-        Citations::from_file(&opts.citation.unwrap_or_default())
+        from_file(opts.citations.unwrap()).unwrap_or_default()
     };
-    let locale = csln::style::locale::Locale::from_file(&opts.locale);
-    let processor: Processor = Processor::new(style, bibliography, citations, locale);
-    let rendered_refs = processor.process_references();
+    let locale = from_file(&opts.locale).context("Locale file?");
+    let processor: Processor = Processor::new(
+        style.expect("msg"), // REVIEW why?
+        bibliography.expect("msg"),
+        citations,
+        locale.expect("msg"),
+    );
+    let rendered_refs: ProcReferences = processor.process_references();
+    let serialized_refs = serde_json::to_string_pretty(&rendered_refs);
     //println!("{}", refs_to_string(rendered_refs));
-    println!("{}", serde_json::to_string_pretty(&rendered_refs).unwrap());
+    if serialized_refs.is_err() {
+        println!("Error: {:?}", serialized_refs);
+    } else {
+        println!("{}", serialized_refs.unwrap());
+    }
 }

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);

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"] }

csln/src/bibliography/mod.rs

@@ -1,24 +1,7 @@
-use crate::HasFile;
 use std::collections::HashMap;
-use std::fs;
 
 pub mod reference;
 pub use reference::InputReference;
 
 /// A bibliography is a collection of references.
 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")
-        } else if bib_path.ends_with(".yaml") || bib_path.ends_with(".yml") {
-            serde_yaml::from_str(&contents).expect("Failed to parse YAML")
-        } else {
-            panic!("Style file must be in YAML or JSON format")
-        }
-    }
-}

csln/src/citation/mod.rs

@@ -1,21 +1,5 @@
-use crate::HasFile;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
-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")
-        } else if citations_path.ends_with(".yaml") || citations_path.ends_with(".yml") {
-            serde_yaml::from_str(&contents).expect("Failed to parse YAML")
-        } else {
-            panic!("Citations file must be in YAML or JSON format")
-        }
-    }
-}
 
 pub type Citations = Vec<Citation>;
 

csln/src/lib.rs

@@ -1,11 +1,41 @@
 pub mod style;
+use std::path::Path;
+
+use serde::de::DeserializeOwned;
 pub use style::Style;
 
+use std::fs;
+
 pub mod bibliography;
 pub use bibliography::InputBibliography;
+use style::locale::Locale;
+
+use anyhow::{Context, Result};
 
 pub mod citation;
 
-pub trait HasFile {
-    fn from_file(path: &str) -> Self;
+pub trait Parsable: DeserializeOwned {}
+impl Parsable for Style {}
+impl Parsable for Locale {}
+impl Parsable for InputBibliography {}
+impl Parsable for citation::Citations {}
+
+pub fn from_file<T: Parsable, P: AsRef<Path>>(path: P) -> Result<T> {
+    let path = path.as_ref();
+    let contents = fs::read_to_string(path)
+        .with_context(|| format!("Failed to read file: {}", path.display()))?;
+
+    let value = 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 {
+        return Err(anyhow::anyhow!("Unsupported file extension"));
+    };
+
+    Ok(value)
 }

csln/src/style/locale.rs

@@ -1,6 +1,6 @@
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
-use std::{collections::HashMap, fs};
+use std::collections::HashMap;
 //use unic_langid::LanguageIdentifier;
 
 #[derive(Debug, Default, Deserialize, Serialize, Clone, JsonSchema)]
@@ -30,20 +30,6 @@ pub struct Terms {
     pub ibid: Option<String>,
 }
 
-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")
-        } else {
-            panic!("Locale file must be in YAML or JSON format")
-        }
-    }
-}
-
 #[derive(Debug, Default, Deserialize, Serialize, Clone, JsonSchema)]
 pub struct AndAs {
     pub symbol: String,

csln/src/style/mod.rs

@@ -6,7 +6,6 @@ SPDX-FileCopyrightText: © 2023 Bruce D'Arcus
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use std::collections::HashMap;
-use std::fs;
 
 pub mod locale;
 pub mod options;
@@ -15,20 +14,6 @@ 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")
-        } else if style_path.ends_with(".yaml") || style_path.ends_with(".yml") {
-            serde_yaml::from_str(&contents).expect("Failed to parse YAML")
-        } else {
-            panic!("Style file must be in YAML or JSON format")
-        }
-    }
-}
-
 /// The Style model.
 #[derive(Debug, Default, Deserialize, Serialize, Clone, JsonSchema)]
 pub struct Style {

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)]

processor/benches/proc_bench.rs

@@ -1,17 +1,24 @@
 use criterion::{criterion_group, criterion_main, Criterion};
 use csln::bibliography::InputBibliography as Bibliography;
 use csln::citation::Citation;
+use csln::from_file;
 use csln::style::Style;
-use csln::HasFile;
 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 bibliography: Bibliography = Bibliography::from_file("examples/ex1.bib.yaml");
-    let locale = csln::style::locale::Locale::from_file("locales/locale-en.yaml");
+    let style = match from_file("examples/style.csl.yaml") {
+        Ok(style) => style,
+        Err(_) => {
+            println!("Failed to load style");
+            return;
+        }
+    };
+    let bibliography: Bibliography = from_file("examples/ex1.bib.yaml").expect("msg");
+    let locale = from_file("locales/locale-en.yaml");
     let citations: Vec<Citation> = Vec::new();
-    let processor: Processor = Processor::new(style, bibliography, citations, locale);
+    let processor: Processor =
+        Processor::new(style, bibliography, citations, locale.expect("msg"));
     c.bench_function("sorting references", |b| {
         b.iter(|| {
             let refs = processor.get_references();

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 {

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 }
     }