Skip to content

Replace unwrap() with let Some(...) else constructs for safer option handling #810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
moritz-gross merged 2 commits into from
Sep 24, 2026
Merged

Replace unwrap() with let Some(...) else constructs for safer option handling #810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3073e1e
Replace `unwrap()` with `let Some(...) else` constructs for safer opt…
moritz-gross Sep 23, 2026
8405cd5
simplify destructuring syntax using tuples
moritz-gross Sep 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/interface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,11 +526,10 @@ pub fn do_navigate_command(command: impl AsRef<str>) -> Result<String> {
enable_logs();
let command = command.as_ref().to_string();
let result = catch_unwind(AssertUnwindSafe(|| {
let cmd = NAV_COMMANDS.get_key(&command); // gets a &'static version of the command
if cmd.is_none() {
let Some(cmd) = NAV_COMMANDS.get_key(&command) else { // gets a &'static version of the command.
bail!("Unknown command in call to DoNavigateCommand()");
};
let cmd = *cmd.unwrap();
let cmd = *cmd;
MATHML_INSTANCE.with(|package_instance| {
let package_instance = package_instance.borrow();
let mathml = get_element(&package_instance);
Expand Down
43 changes: 15 additions & 28 deletions src/speech.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,11 +431,9 @@ impl fmt::Display for Replacement {
impl Replacement {
fn build(replacement: &Yaml) -> Result<Replacement> {
// Replacement -- single key/value (see below for allowed values)
let dictionary = replacement.as_hash();
if dictionary.is_none() {
let Some(dictionary) = replacement.as_hash() else {
bail!(" expected a key/value pair. Found {}.", yaml_to_string(replacement, 0));
};
let dictionary = dictionary.unwrap();
if dictionary.is_empty() {
bail!("No key/value pairs found for key 'replace'.\n\
Suggestion: are the following lines indented properly?");
Expand Down Expand Up @@ -898,8 +896,7 @@ impl ReplacementArray {
/// Any errors are passed back out.
pub fn build(replacements: &Yaml) -> Result<ReplacementArray> {
// replacements is either a single replacement or an array of replacements
let result= if replacements.is_array() {
let replacements = replacements.as_vec().unwrap();
let result= if let Some(replacements) = replacements.as_vec() {
replacements
.iter()
.enumerate() // useful for errors
Expand Down Expand Up @@ -1173,12 +1170,9 @@ impl MyXPath {
/// Convert DEBUG(...) input to the internal function which is DEBUG(arg, arg_as_string)
fn add_debug_string_arg(xpath: &str) -> Result<String> {
// do a quick check to see if "DEBUG" is in the string -- this is the common case
let debug_start = xpath.find("DEBUG(");
if debug_start.is_none() {
let Some(debug_start) = xpath.find("DEBUG(") else {
return Ok( xpath.to_string() );
}

let debug_start = debug_start.unwrap();
};
let mut before_paren = xpath[..debug_start+5].to_string(); // includes "DEBUG"
let chars = xpath[debug_start+5..].chars().collect::<Vec<char>>(); // begins at '('
before_paren.push_str(&chars_add_debug_string_arg(&chars).with_context(|| format!("In xpath='{xpath}'"))?);
Expand Down Expand Up @@ -1469,17 +1463,17 @@ impl TestArray {
// 'if:' should only be the first entry in the array; 'else_if' should never be the first entry. Otherwise, they are the same
let mut test_array = vec![];
for test in tests {
if test.as_hash().is_none() {
let Some(test_hash) = test.as_hash() else {
bail!("Value for array entry in 'test:' must be a dictionary/contain keys");
}
};
let if_part = &test[if test_array.is_empty() {"if"} else {"else_if"}];
if !if_part.is_badvalue() {
// first case: if:, then:, optional else:
let condition = Some( MyXPath::build(if_part)? );
let then_part = TestOrReplacements::build(test, "then", "then_test", true)?;
let else_part = TestOrReplacements::build(test, "else", "else_test", false)?;
let n_keys = if else_part.is_none() {2} else {3};
if test.as_hash().unwrap().len() > n_keys {
if test_hash.len() > n_keys {
bail!("A key other than 'if', 'else_if', 'then', 'then_test', 'else', or 'else_test' was found in the 'then' clause of 'test'");
};
test_array.push(
Expand All @@ -1488,7 +1482,7 @@ impl TestArray {
} else {
// second case: should be else/else_test
let else_part = TestOrReplacements::build(test, "else", "else_test", true)?;
if test.as_hash().unwrap().len() > 1 {
if test_hash.len() > 1 {
bail!("A key other than 'if', 'else_if', 'then', 'then_test', 'else', or 'else_test' was found the 'else' clause of 'test'");
};
test_array.push(
Expand Down Expand Up @@ -1715,8 +1709,7 @@ impl VariableDefinitions {
if defs.is_badvalue() {
return Ok( VariableDefinitions::new(0) );
};
if defs.is_array() {
let defs = defs.as_vec().unwrap();
if let Some(defs) = defs.as_vec() {
let mut definitions = VariableDefinitions::new(defs.len());
for def in defs {
let variable_def = VariableDefinition::build(def)
Expand Down Expand Up @@ -1874,12 +1867,9 @@ impl UnicodeDef {
return Ok( Some(process_include(file_name, include_file_name, do_include_fn)?) );
}
// key: char, value is replacement or array of replacements
let dictionary = unicode_def.as_hash();
if dictionary.is_none() {
let Some(dictionary) = unicode_def.as_hash() else {
bail!("Expected a unicode definition (e.g, '+':[t: \"plus\"]'), found {}", yaml_to_string(unicode_def, 0));
}

let dictionary = dictionary.unwrap();
};
if dictionary.len() != 1 {
bail!("Expected a unicode definition (e.g, '+':[t: \"plus\"]'), found {}", yaml_to_string(unicode_def, 0));
}
Expand Down Expand Up @@ -2334,11 +2324,9 @@ impl SpeechRules {

fn build_speech_patterns(&mut self, patterns: &Yaml, file_name: &Path) -> Result<Vec<PathBuf>> {
// Rule::SpeechPatternList
let patterns_vec = patterns.as_vec();
if patterns_vec.is_none() {
let Some(patterns_vec) = patterns.as_vec() else {
bail!(yaml_type_err(patterns, "array"));
}
let patterns_vec = patterns.as_vec().unwrap();
};
let mut files_read = vec![file_name.to_path_buf()];
for entry in patterns_vec.iter() {
if let Some(mut added_files) = SpeechPattern::build(entry, file_name, self)? {
Expand Down Expand Up @@ -2368,12 +2356,11 @@ impl SpeechRules {
// info!("Reading unicode file {}", path.to_str().unwrap());
let unicode_file_contents = read_to_string_shim(&path)?;
let unicode_build_fn = |unicode_def_list: &Yaml| {
let unicode_defs = unicode_def_list.as_vec();
if unicode_defs.is_none() {
let Some(unicode_defs) = unicode_def_list.as_vec() else {
bail!("File '{}' does not begin with an array", yaml_to_type(unicode_def_list));
};
let mut files_read = vec![path.to_path_buf()];
for unicode_def in unicode_defs.unwrap() {
for unicode_def in unicode_defs {
if let Some(mut added_files) = UnicodeDef::build(unicode_def, &path, self, use_short)
.with_context(|| {format!("In file {:?}", path.to_str())})? {
files_read.append(&mut added_files);
Expand Down
7 changes: 1 addition & 6 deletions src/xpath_functions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,14 +266,9 @@ impl IsNode {
return false;
}

let num = children[0].element();
let denom = children[1].element();
if num.is_none() || denom.is_none() {
let (Some(num), Some(denom)) = (children[0].element(), children[1].element()) else {
return false;
};

let num = num.unwrap();
let denom = denom.unwrap();
if !is_tag(num, "mn") || !is_tag(denom, "mn") {
return false
};
Expand Down
Loading