Skip to content

Commit

Permalink
refactor: remove unwrap/expect
Browse files Browse the repository at this point in the history
Refactor to properly handle possible errors.

Signed-off-by: Bruce D'Arcus <[email protected]>
  • Loading branch information
bdarcus committed Jan 12, 2024
1 parent 2c7a5e4 commit 70a02f3
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 36 deletions.
9 changes: 8 additions & 1 deletion cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,14 @@ pub struct Opts {

fn main() {
let opts = Opts::parse();
let style: Style = Style::from_file(&opts.style);
let style_result = Style::from_file(&opts.style);
let style: Style = match style_result {
Ok(style) => style,
Err(err) => {
eprintln!("Error loading style file: {}", err);
return;
}
};
let bibliography: Bibliography = Bibliography::from_file(&opts.bibliography);
let citations: Citations = if opts.citation.is_none() {
Citations::default()
Expand Down
25 changes: 19 additions & 6 deletions csln/src/citation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
24 changes: 17 additions & 7 deletions csln/src/style/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,25 @@ 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, &'static str> {
let contents = match fs::read_to_string(style_path) {
Ok(contents) => contents,
Err(_) => return Err("Failed to read style file"),
};
let style: Style = if style_path.ends_with(".json") {
match serde_json::from_str(&contents) {
Ok(style) => style,
Err(_) => return Err("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")
match serde_yaml::from_str(&contents) {
Ok(style) => style,
Err(_) => return Err("Failed to parse YAML"),
}
} else {
panic!("Style file must be in YAML or JSON format")
}
return Err("Style file must be in YAML or JSON format");
};
Ok(style)
}
}

Expand Down
2 changes: 1 addition & 1 deletion csln/src/style/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
8 changes: 7 additions & 1 deletion processor/benches/proc_bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
28 changes: 14 additions & 14 deletions processor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,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
Expand All @@ -449,16 +450,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,
Expand Down Expand Up @@ -565,7 +558,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"),
Expand Down Expand Up @@ -673,7 +670,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);
Expand All @@ -685,9 +685,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 {
Expand Down
8 changes: 2 additions & 6 deletions processor/tests/processor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Check failure on line 24 in processor/tests/processor_test.rs

View workflow job for this annotation

GitHub Actions / Test Suite

mismatched types

TestFixture { style, locale, bibliography, citations, processor }

Check failure on line 26 in processor/tests/processor_test.rs

View workflow job for this annotation

GitHub Actions / Test Suite

mismatched types
}
Expand Down

0 comments on commit 70a02f3

Please sign in to comment.