From e3be60e29166d95126af3f2ee7ba6f87218aaf92 Mon Sep 17 00:00:00 2001
From: Xavier L'Heureux <xavier.lheureux@icloud.com>
Date: Fri, 15 Feb 2019 23:39:46 -0500
Subject: [PATCH] Refactor the assignements to reduce complexity

Reorder the branches to extract the common patterns
Use Results to move error handling upper in the chain.
Use as much internal functions, such as `map` to reduce match nesting.
Use iterators when applicable.
---
 members/braces/src/lib.rs             |   7 +-
 members/builtins/src/calc.rs          |   6 +-
 members/lexers/src/assignments/mod.rs |  32 +-
 src/lib/shell/assignments.rs          | 743 ++++++++++----------------
 4 files changed, 310 insertions(+), 478 deletions(-)

diff --git a/members/braces/src/lib.rs b/members/braces/src/lib.rs
index 8b4b9a09..8b24cc49 100644
--- a/members/braces/src/lib.rs
+++ b/members/braces/src/lib.rs
@@ -20,11 +20,8 @@ pub fn expand<'a>(
         let multiple_brace_expand = MultipleBraceExpand::new(tokens, expanders);
         Box::new(multiple_brace_expand)
     } else if expanders.len() == 1 {
-        let single_brace_expand = SingleBraceExpand {
-            elements: expanders[0].iter().map(|element| *element),
-            tokens,
-            loop_count: 0,
-        };
+        let single_brace_expand =
+            SingleBraceExpand { elements: expanders[0].iter().cloned(), tokens, loop_count: 0 };
         Box::new(single_brace_expand)
     } else {
         Box::new(::std::iter::empty())
diff --git a/members/builtins/src/calc.rs b/members/builtins/src/calc.rs
index f1ae0e8e..18ff32ca 100644
--- a/members/builtins/src/calc.rs
+++ b/members/builtins/src/calc.rs
@@ -2,7 +2,7 @@ use calculate::{eval, eval_polish, CalcError, Value};
 use small;
 use std::io::{self, Write};
 
-const REPL_GUIDE: &'static str = r#"ion-calc
+const REPL_GUIDE: &str = r#"ion-calc
 Type in expressions to have them evaluated.
 Type "help" for help."#;
 
@@ -26,10 +26,10 @@ SPECIAL EXPRESSIONS
         exits the program
 
 NOTATIONS
-    infix notation 
+    infix notation
         e.g. 3 * 4 + 5
 
-    polish notation 
+    polish notation
         e.g. + * 3 4 5
 
 EXAMPLES
diff --git a/members/lexers/src/assignments/mod.rs b/members/lexers/src/assignments/mod.rs
index 0aa783e1..3006f23f 100644
--- a/members/lexers/src/assignments/mod.rs
+++ b/members/lexers/src/assignments/mod.rs
@@ -10,9 +10,7 @@ pub use self::{
 
 /// Given an valid assignment expression, this will split it into `keys`,
 /// `operator`, `values`.
-pub fn assignment_lexer<'a>(
-    statement: &'a str,
-) -> (Option<&'a str>, Option<Operator>, Option<&'a str>) {
+pub fn assignment_lexer(statement: &str) -> (Option<&str>, Option<Operator>, Option<&str>) {
     let statement = statement.trim();
     if statement.is_empty() {
         return (None, None, None);
@@ -20,29 +18,27 @@ pub fn assignment_lexer<'a>(
 
     let (mut read, mut start) = (0, 0);
     let as_bytes = statement.as_bytes();
-    let mut bytes = statement.bytes();
+    let mut bytes = statement.bytes().peekable();
     let mut operator = None;
 
     while let Some(byte) = bytes.next() {
+        operator = Some(Operator::Equal);
         if b'=' == byte {
-            operator = Some(Operator::Equal);
-            if as_bytes.get(read + 1).is_none() {
-                return (Some(&statement[..read].trim()), operator, None);
+            if bytes.peek().is_none() {
+                return (Some(&statement[..read].trim()), Some(Operator::Equal), None);
             }
             start = read;
             read += 1;
             break;
-        } else {
-            match find_operator(as_bytes, read) {
-                None => (),
-                Some((op, found)) => {
-                    operator = Some(op);
-                    start = read;
-                    read = found;
-                    break;
-                }
-            }
         }
+
+        if let Some((op, found)) = find_operator(as_bytes, read) {
+            operator = Some(op);
+            start = read;
+            read = found;
+            break;
+        }
+
         read += 1;
     }
 
@@ -62,7 +58,7 @@ fn find_operator(bytes: &[u8], read: usize) -> Option<(Operator, usize)> {
     } else if bytes[read + 1] == b'=' {
         Operator::parse_single(bytes[read]).map(|op| (op, read + 2))
     } else if bytes[read + 2] == b'=' {
-        Operator::parse_double(&bytes[read..read + 2]).map(|op| (op, read + 3))
+        Operator::parse_double(&bytes[read..=read + 1]).map(|op| (op, read + 3))
     } else {
         None
     }
diff --git a/src/lib/shell/assignments.rs b/src/lib/shell/assignments.rs
index cc2e1e90..a8ebd74d 100644
--- a/src/lib/shell/assignments.rs
+++ b/src/lib/shell/assignments.rs
@@ -8,7 +8,6 @@ use itoa;
 use lexers::assignments::{Operator, Primitive};
 use parser::assignments::*;
 use shell::{history::ShellHistory, variables::VariableType};
-use small;
 use std::{
     env,
     ffi::OsStr,
@@ -16,43 +15,42 @@ use std::{
     io::{self, BufWriter, Write},
     mem,
     os::unix::ffi::OsStrExt,
+    result::Result,
     str,
 };
 use types;
 
-fn list_vars(shell: &Shell) {
+fn list_vars(shell: &Shell) -> Result<(), io::Error> {
     let stdout = io::stdout();
     let mut buffer = BufWriter::new(stdout.lock());
 
-    // Small function for formatting and append an array entry to a string buffer.
-    fn print_array<W: Write>(buffer: &mut W, key: &str, array: &[small::String]) {
-        let _ = buffer.write([key, " = [ "].concat().as_bytes());
-        if array.len() > 1 {
-            let mut vars = array.iter();
-            if let Some(ref var) = vars.next() {
-                let _ = buffer.write(["'", var, "', "].concat().as_bytes());
-                vars.for_each(|var| {
-                    let _ = buffer.write(["'", var, "' "].concat().as_bytes());
-                });
-            }
-            let _ = buffer.write(b"]\n");
-        } else if array.len() == 1 {
-            let _ = buffer.write(["'", &array[0], "' ]\n"].concat().as_bytes());
-        } else {
-            let _ = buffer.write("]\n".as_bytes());
-        }
-    }
-
     // Write all the string variables to the buffer.
-    let _ = buffer.write(b"# String Variables\n");
+    let _ = buffer.write(b"# String Variables\n")?;
     for (key, val) in shell.variables.string_vars() {
-        let _ = buffer.write([key, " = ", val.as_str(), "\n"].concat().as_bytes());
+        writeln!(buffer, "{} = {}", key, val)?;
     }
 
     // Then immediately follow that with a list of array variables.
-    let _ = buffer.write(b"\n# Array Variables\n");
+    let _ = buffer.write(b"\n# Array Variables\n")?;
     for (key, val) in shell.variables.arrays() {
-        print_array(&mut buffer, &key, &**val)
+        write!(buffer, "{} = [ ", key)?;
+        let mut vars = val.iter();
+        if let Some(ref var) = vars.next() {
+            write!(buffer, "'{}' ", var)?;
+            vars.map(|var| write!(buffer, ", '{}' ", var)).collect::<Result<Vec<_>, _>>()?;
+        }
+        writeln!(buffer, "]")?;
+    }
+    Ok(())
+}
+
+fn arithmetic_op(operator: Operator, value: f64) -> Result<Box<dyn Fn(f64) -> f64>, String> {
+    match operator {
+        Operator::Add => Ok(Box::new(move |src| src + value)),
+        Operator::Divide => Ok(Box::new(move |src| src / value)),
+        Operator::Subtract => Ok(Box::new(move |src| src - value)),
+        Operator::Multiply => Ok(Box::new(move |src| src * value)),
+        _ => Err("operator does not work on arrays".to_string()),
     }
 }
 
@@ -83,80 +81,66 @@ impl VariableStore for Shell {
                 let stdout = io::stdout();
                 let mut stdout = stdout.lock();
                 for (key, val) in env::vars() {
-                    let _ = writeln!(stdout, "{} =\"{}\"", key, val);
+                    let _ = writeln!(stdout, "{} = \"{}\"", key, val);
                 }
                 return SUCCESS;
             }
         };
 
         for action in actions {
-            match action {
-                Ok(Action::UpdateArray(key, Operator::Equal, expression)) => {
-                    match value_check(self, &expression, &key.kind) {
-                        Ok(VariableType::Array(values)) => env::set_var(key.name, values.join(" ")),
-                        Err(why) => {
-                            eprintln!("ion: assignment error: {}: {}", key.name, why);
-                            return FAILURE;
-                        }
-                        _ => {
-                            eprintln!(
-                                "ion: assignment error: export of type '{}' is not supported",
-                                key.kind
-                            );
-                            return FAILURE;
-                        }
-                    }
-                }
-                Ok(Action::UpdateArray(..)) => {
-                    eprintln!(
-                        "ion: arithmetic operators on array expressions aren't supported yet."
-                    );
-                    return FAILURE;
+            let err = action.map_err(|e| e.to_string()).and_then(|act| match act {
+                Action::UpdateArray(key, Operator::Equal, expression) => {
+                    value_check(self, &expression, &key.kind)
+                        .map_err(|e| e.to_string())
+                        .and_then(|val| match val {
+                            VariableType::Array(values) => {
+                                env::set_var(key.name, values.join(" "));
+                                Ok(())
+                            }
+                            _ => Err(format!("export of type '{}' is not supported", key.kind)),
+                        })
+                        .map_err(|why| format!("{}: {}", key.name, why))
                 }
-                Ok(Action::UpdateString(key, operator, expression)) => {
+                Action::UpdateArray(..) => Err("arithmetic operators on array expressions aren't \
+                                                supported yet."
+                    .to_string()),
+                Action::UpdateString(key, operator, expression) => {
                     match value_check(self, &expression, &key.kind) {
                         Ok(VariableType::Str(value)) => {
                             let key_name: &str = &key.name;
-                            let lhs: types::Str = self
+                            let lhs = self
                                 .variables
                                 .get::<types::Str>(key_name)
                                 .unwrap_or_else(|| "0".into());
 
-                            let result = math(&lhs, &key.kind, operator, &value, |value| {
-                                let str_value = unsafe { str::from_utf8_unchecked(value) };
+                            math(&lhs, &key.kind, operator, &value, |value| {
+                                let mut str_value =
+                                    unsafe { str::from_utf8_unchecked(value) }.to_string();
+
                                 if key_name == "PATH" && str_value.find('~').is_some() {
-                                    let final_value = str_value.replace(
+                                    str_value = str_value.replace(
                                         "~",
                                         env::var("HOME")
                                             .as_ref()
                                             .map(|s| s.as_str())
                                             .unwrap_or("~"),
-                                    );
-                                    env::set_var(
-                                        key_name,
-                                        &OsStr::from_bytes(final_value.as_bytes()),
                                     )
-                                } else {
-                                    env::set_var(key_name, &OsStr::from_bytes(value))
                                 }
-                            });
 
-                            if let Err(why) = result {
-                                eprintln!("ion: assignment error: {}", why);
-                                return FAILURE;
-                            }
-                        }
-                        Err(why) => {
-                            eprintln!("ion: assignment error: {}: {}", key.name, why);
-                            return FAILURE;
+                                env::set_var(key_name, &OsStr::from_bytes(str_value.as_bytes()));
+                                Ok(())
+                            })
+                            .map_err(|e| e.to_string())
                         }
+                        Err(why) => Err(format!("{}: {}", key.name, why)),
                         _ => unreachable!(),
                     }
                 }
-                Err(why) => {
-                    eprintln!("ion: assignment error: {}", why);
-                    return FAILURE;
-                }
+            });
+
+            if let Err(why) = err {
+                eprintln!("ion: assignment error: {}", why);
+                return FAILURE;
             }
         }
 
@@ -167,7 +151,7 @@ impl VariableStore for Shell {
         let mut collected: HashMap<&str, VariableType> = HashMap::new();
         let (actions_step1, actions_step2) = match action {
             LocalAction::List => {
-                list_vars(&self);
+                let _ = list_vars(&self);
                 return SUCCESS;
             }
             LocalAction::Assign(ref keys, op, ref vals) => {
@@ -175,371 +159,231 @@ impl VariableStore for Shell {
             }
         };
         for action in actions_step1 {
-            match action {
-                Ok(Action::UpdateArray(key, operator, expression)) => {
-                    match operator {
-                        Operator::Equal | Operator::OptionalEqual => {
-                            match value_check(self, &expression, &key.kind) {
-                                Ok(VariableType::Array(values)) => {
-                                    // When we changed the HISTORY_IGNORE variable, update the
-                                    // ignore patterns. This happens first because `set_array`
-                                    // consumes 'values'
-                                    if key.name == "HISTORY_IGNORE" {
-                                        self.update_ignore_patterns(&values);
-                                    }
-                                    collected.insert(key.name, VariableType::Array(values));
-                                }
-                                Ok(VariableType::Str(value)) => {
-                                    collected.insert(key.name, VariableType::Str(value));
-                                }
-                                Ok(VariableType::HashMap(hmap)) => {
-                                    collected.insert(key.name, VariableType::HashMap(hmap));
-                                }
-                                Ok(VariableType::BTreeMap(bmap)) => {
-                                    collected.insert(key.name, VariableType::BTreeMap(bmap));
-                                }
-                                Err(why) => {
-                                    eprintln!("ion: assignment error: {}: {}", key.name, why);
-                                    return FAILURE;
-                                }
-                                _ => (),
-                            }
-                        }
-                        Operator::Concatenate => match value_check(self, &expression, &key.kind) {
-                            Ok(VariableType::Array(values)) => {
-                                match self.variables.get_mut(key.name) {
-                                    Some(VariableType::Array(ref mut array)) => {
-                                        array.extend(values);
-                                    }
-                                    None => {
-                                        eprintln!(
-                                            "ion: assignment error: {}: cannot concatenate \
-                                             non-array variable",
-                                            key.name
-                                        );
-                                        return FAILURE;
+            let err = action.map_err(|e| format!("{}", e)).and_then(|act| match act {
+                Action::UpdateArray(key, operator, expression) => {
+                    let right_hand_side = value_check(self, &expression, &key.kind);
+
+                    right_hand_side.map_err(|why| format!("{}: {}", key.name, why)).and_then(
+                        |val| {
+                            if [Operator::Equal, Operator::OptionalEqual].contains(&operator) {
+                                // When we changed the HISTORY_IGNORE variable, update the
+                                // ignore patterns. This happens first because `set_array`
+                                // consumes 'values'
+                                if key.name == "HISTORY_IGNORE" {
+                                    if let VariableType::Array(values) = &val {
+                                        self.update_ignore_patterns(values);
                                     }
-                                    _ => (),
                                 }
+                                collected.insert(key.name, val);
+                                return Ok(());
                             }
-                            Err(why) => {
-                                eprintln!("ion: assignment error: {}: {}", key.name, why);
-                                return FAILURE;
-                            }
-                            _ => (),
-                        },
-                        Operator::ConcatenateHead => {
-                            match value_check(self, &expression, &key.kind) {
-                                Ok(VariableType::Array(values)) => {
-                                    match self.variables.get_mut(key.name) {
-                                        Some(VariableType::Array(ref mut array)) => {
-                                            for (index, value) in values.into_iter().enumerate() {
-                                                array.insert(index, value);
+
+                            let left_hand_side = self
+                                .variables
+                                .get_mut(key.name)
+                                .ok_or_else(|| "cannot concatenate non-array variable".to_string());
+                            if let VariableType::Array(values) = val {
+                                left_hand_side.map(|v| {
+                                    if let VariableType::Array(ref mut array) = v {
+                                        match operator {
+                                            Operator::Concatenate => array.extend(values),
+                                            Operator::ConcatenateHead => values
+                                                .into_iter()
+                                                .rev()
+                                                .for_each(|value| array.insert(0, value)),
+                                            Operator::Filter => {
+                                                array.retain(|item| !values.contains(item))
                                             }
+                                            _ => {}
                                         }
-                                        None => {
-                                            eprintln!(
-                                                "ion: assignment error: {}: cannot head \
-                                                 concatenate non-array variable",
-                                                key.name
-                                            );
-                                            return FAILURE;
-                                        }
-                                        _ => (),
                                     }
-                                }
-                                Err(why) => {
-                                    eprintln!("ion: assignment error: {}: {}", key.name, why);
-                                    return FAILURE;
-                                }
-                                _ => (),
-                            }
-                        }
-                        Operator::Filter => match value_check(self, &expression, &key.kind) {
-                            Ok(VariableType::Array(values)) => {
-                                match self.variables.get_mut(key.name) {
-                                    Some(VariableType::Array(ref mut array)) => {
-                                        *array = array
-                                            .iter()
-                                            .filter(|item| {
-                                                values.iter().all(|value| *item != value)
-                                            })
-                                            .cloned()
-                                            .collect();
-                                    }
-                                    None => {
-                                        eprintln!(
-                                            "ion: assignment error: {}: cannot head concatenate \
-                                             non-array variable",
-                                            key.name
-                                        );
-                                        return FAILURE;
-                                    }
-                                    _ => (),
-                                }
-                            }
-                            Err(why) => {
-                                eprintln!("ion: assignment error: {}: {}", key.name, why);
-                                return FAILURE;
+                                })
+                            } else {
+                                Ok(())
                             }
-                            _ => (),
                         },
-                        _ => (),
-                    }
+                    )
                 }
-                Ok(Action::UpdateString(key, operator, expression)) => {
+                Action::UpdateString(key, operator, expression) => {
                     if ["HOME", "HOST", "PWD", "MWD", "SWD", "?"].contains(&key.name) {
-                        eprintln!("ion: not allowed to set {}", key.name);
-                        return FAILURE;
+                        return Err(format!("not allowed to set {}", key.name));
                     }
 
-                    match value_check(self, &expression, &key.kind) {
-                        Ok(VariableType::Str(value)) => {
-                            match operator {
-                                Operator::Equal | Operator::OptionalEqual => {
-                                    collected.insert(key.name, VariableType::Str(value));
-                                    continue;
-                                }
-                                Operator::Concatenate => {
-                                    match self.variables.get_mut(key.name) {
-                                        Some(VariableType::Array(ref mut array)) => {
-                                            array.push(value);
-                                        }
-                                        None => {
-                                            eprintln!(
-                                                "ion: assignment error: {}: cannot concatenate \
-                                                 non-array variable",
-                                                key.name
-                                            );
-                                            return FAILURE;
-                                        }
-                                        _ => (),
-                                    }
-                                    continue;
-                                }
-                                Operator::ConcatenateHead => {
-                                    match self.variables.get_mut(key.name) {
-                                        Some(VariableType::Array(ref mut array)) => {
-                                            array.insert(0, value);
-                                        }
-                                        None => {
-                                            eprintln!(
-                                                "ion: assignment error: {}: cannot head \
-                                                 concatenate non-array variable",
-                                                key.name
-                                            );
-                                            return FAILURE;
-                                        }
-                                        _ => (),
-                                    }
-                                    continue;
-                                }
-                                Operator::Filter => {
-                                    match self.variables.get_mut(key.name) {
-                                        Some(VariableType::Array(ref mut array)) => {
-                                            *array = array
-                                                .iter()
-                                                .filter(move |item| **item != value)
-                                                .cloned()
-                                                .collect();
-                                        }
-                                        None => {
-                                            eprintln!(
-                                                "ion: assignment error: {}: cannot head \
-                                                 concatenate non-array variable",
-                                                key.name
-                                            );
-                                            return FAILURE;
-                                        }
-                                        _ => (),
-                                    }
-                                    continue;
-                                }
-                                _ => (),
+                    let right_hand_side = value_check(self, &expression, &key.kind);
+
+                    let left_hand_side = self.variables.get_mut(key.name).ok_or_else(|| {
+                        format!("{}: cannot concatenate non-array variable", key.name)
+                    });
+                    right_hand_side.map_err(|why| format!("{}: {}", key.name, why)).and_then(
+                        |val| {
+                            if [Operator::Equal, Operator::OptionalEqual].contains(&operator) {
+                                collected.insert(key.name, val);
+                                return Ok(());
                             }
-                            match self.variables.get_mut(key.name) {
-                                Some(VariableType::Str(lhs)) => {
-                                    let result = math(&lhs, &key.kind, operator, &value, |value| {
-                                        collected.insert(
-                                            key.name,
-                                            VariableType::Str(
-                                                unsafe { str::from_utf8_unchecked(value) }.into(),
-                                            ),
-                                        );
-                                    });
 
-                                    if let Err(why) = result {
-                                        eprintln!("ion: assignment error: {}", why);
-                                        return FAILURE;
-                                    }
-                                }
-                                Some(VariableType::Array(array)) => {
-                                    let value = match value.parse::<f64>() {
-                                        Ok(n) => n,
-                                        Err(_) => {
-                                            eprintln!(
-                                                "ion: assignment error: value is not a float"
+                            left_hand_side.and_then(|v| match val {
+                                VariableType::Str(value) => match v {
+                                    VariableType::Str(lhs) => {
+                                        math(&lhs, &key.kind, operator, &value, |value| {
+                                            collected.insert(
+                                                key.name,
+                                                VariableType::Str(
+                                                    unsafe { str::from_utf8_unchecked(value) }
+                                                        .into(),
+                                                ),
                                             );
-                                            return FAILURE;
+                                            Ok(())
+                                        })
+                                        .map_err(|e| format!("{}", e))
+                                    }
+                                    VariableType::Array(ref mut array) => match operator {
+                                        Operator::Concatenate => {
+                                            array.push(value);
+                                            Ok(())
                                         }
-                                    };
-
-                                    let action: Box<dyn Fn(f64) -> f64> = match operator {
-                                        Operator::Add => Box::new(|src| src + value),
-                                        Operator::Divide => Box::new(|src| src / value),
-                                        Operator::Subtract => Box::new(|src| src - value),
-                                        Operator::Multiply => Box::new(|src| src * value),
-                                        _ => {
-                                            eprintln!(
-                                                "ion: assignment error: operator does not work on \
-                                                 arrays"
-                                            );
-                                            return FAILURE;
+                                        Operator::ConcatenateHead => {
+                                            array.insert(0, value);
+                                            Ok(())
                                         }
-                                    };
-
-                                    for element in array {
-                                        match element.parse::<f64>() {
-                                            Ok(number) => {
-                                                *element = action(number).to_string().into()
-                                            }
-                                            Err(_) => {
-                                                eprintln!(
-                                                    "ion: assignment error: {} is not a float",
-                                                    element
-                                                );
-                                                return FAILURE;
-                                            }
+                                        Operator::Filter => {
+                                            array.retain(|item| item != &value);
+                                            Ok(())
                                         }
-                                    }
-                                }
-                                _ => {
-                                    eprintln!(
-                                        "ion: assignment error: type does not support this \
-                                         operator"
-                                    );
-                                    return FAILURE;
-                                }
-                            }
-                        }
-                        Err(why) => {
-                            eprintln!("ion: assignment error: {}: {}", key.name, why);
-                            return FAILURE;
-                        }
-                        _ => unreachable!(),
-                    }
-                }
-                Err(why) => {
-                    eprintln!("ion: assignment error: {}", why);
-                    return FAILURE;
+                                        _ => value
+                                            .parse::<f64>()
+                                            .map_err(|_| format!("`{}` is not a float", value))
+                                            .and_then(|value| arithmetic_op(operator, value))
+                                            .and_then(|action| {
+                                                array
+                                                    .iter_mut()
+                                                    .map(|element| {
+                                                        element
+                                                            .parse::<f64>()
+                                                            .map_err(|_| {
+                                                                format!(
+                                                                    "`{}` is not a float",
+                                                                    element
+                                                                )
+                                                            })
+                                                            .map(|n| action(n))
+                                                            .map(|result| {
+                                                                *element = result.to_string().into()
+                                                            })
+                                                    })
+                                                    .find(|e| e.is_err())
+                                                    .unwrap_or(Ok(()))
+                                            }),
+                                    },
+                                    _ => Err("type does not support this operator".to_string()),
+                                },
+                                _ => unreachable!(),
+                            })
+                        },
+                    )
                 }
+            });
+
+            if let Err(why) = err {
+                eprintln!("ion: assignment error: {}", why);
+                return FAILURE;
             }
         }
 
         for action in actions_step2 {
-            match action {
-                Ok(Action::UpdateArray(key, operator, ..)) => {
+            match action.unwrap() {
+                Action::UpdateArray(key, operator, ..) => {
                     if operator == Operator::OptionalEqual
                         && self.variables.get_ref(key.name).is_some()
                     {
                         continue;
                     }
-                    match collected.remove(key.name) {
-                        hmap @ Some(VariableType::HashMap(_)) => {
-                            if let Primitive::HashMap(_) = key.kind {
-                                self.variables.set(key.name, hmap.unwrap());
-                            } else if let Primitive::Indexed(..) = key.kind {
-                                eprintln!("ion: cannot insert hmap into index");
-                                return FAILURE;
+
+                    let err = collected
+                        .remove(key.name)
+                        .map(|var| match (&var, &key.kind) {
+                            (VariableType::HashMap(_), Primitive::Indexed(..)) => {
+                                Err("cannot insert hmap into index".to_string())
                             }
-                        }
-                        bmap @ Some(VariableType::BTreeMap(_)) => {
-                            if let Primitive::BTreeMap(_) = key.kind {
-                                self.variables.set(key.name, bmap.unwrap());
-                            } else if let Primitive::Indexed(..) = key.kind {
-                                eprintln!("ion: cannot insert bmap into index");
-                                return FAILURE;
+                            (VariableType::BTreeMap(_), Primitive::Indexed(..)) => {
+                                Err("cannot insert bmap into index".to_string())
                             }
-                        }
-                        array @ Some(VariableType::Array(_)) => {
-                            if let Primitive::Indexed(..) = key.kind {
-                                eprintln!("ion: multi-dimensional arrays are not yet supported");
-                                return FAILURE;
-                            } else {
-                                self.variables.set(key.name, array.unwrap());
+                            (VariableType::Array(_), Primitive::Indexed(..)) => {
+                                Err("multi-dimensional arrays are not yet supported".to_string())
                             }
-                        }
-                        Some(VariableType::Str(value)) => {
-                            if let Primitive::Indexed(ref index_value, ref index_kind) = key.kind {
-                                match value_check(self, index_value, index_kind) {
-                                    Ok(VariableType::Str(ref index)) => match self
-                                        .variables
-                                        .get_mut(key.name)
-                                    {
-                                        Some(VariableType::HashMap(hmap)) => {
-                                            hmap.insert(index.clone(), VariableType::Str(value));
-                                        }
-                                        Some(VariableType::BTreeMap(bmap)) => {
-                                            bmap.insert(index.clone(), VariableType::Str(value));
-                                        }
-                                        Some(VariableType::Array(array)) => {
-                                            let index_num = match index.parse::<usize>() {
-                                                Ok(num) => num,
-                                                Err(_) => {
-                                                    eprintln!(
-                                                        "ion: index variable does not contain a \
+                            (VariableType::HashMap(_), Primitive::HashMap(_))
+                            | (VariableType::BTreeMap(_), Primitive::BTreeMap(_))
+                            | (VariableType::Array(_), _) => {
+                                self.variables.set(key.name, var);
+                                Ok(())
+                            }
+                            (
+                                VariableType::Str(_),
+                                Primitive::Indexed(ref index_value, ref index_kind),
+                            ) => value_check(self, index_value, index_kind)
+                                .map_err(|why| format!("assignment error: {}: {}", key.name, why))
+                                .and_then(|val| match val {
+                                    VariableType::Str(ref index) => {
+                                        match self.variables.get_mut(key.name) {
+                                            Some(VariableType::HashMap(hmap)) => {
+                                                hmap.insert(index.clone(), var);
+                                                Ok(())
+                                            }
+                                            Some(VariableType::BTreeMap(bmap)) => {
+                                                bmap.insert(index.clone(), var);
+                                                Ok(())
+                                            }
+                                            Some(VariableType::Array(array)) => index
+                                                .parse::<usize>()
+                                                .map_err(|_| {
+                                                    format!(
+                                                        "index variable does not contain a \
                                                          numeric value: {}",
                                                         index
-                                                    );
-                                                    return FAILURE;
-                                                }
-                                            };
-                                            if let Some(val) = array.get_mut(index_num) {
-                                                *val = value;
-                                            }
+                                                    )
+                                                })
+                                                .map(|index_num| {
+                                                    if let (Some(val), VariableType::Str(value)) =
+                                                        (array.get_mut(index_num), var)
+                                                    {
+                                                        *val = value;
+                                                    }
+                                                }),
+                                            _ => Ok(()),
                                         }
-                                        _ => (),
-                                    },
-                                    Ok(VariableType::Array(_)) => {
-                                        eprintln!("ion: index variable cannot be an array");
-                                        return FAILURE;
                                     }
-                                    Ok(VariableType::HashMap(_)) => {
-                                        eprintln!("ion: index variable cannot be a hmap");
-                                        return FAILURE;
+                                    VariableType::Array(_) => {
+                                        Err("index variable cannot be an array".to_string())
                                     }
-                                    Ok(VariableType::BTreeMap(_)) => {
-                                        eprintln!("ion: index variable cannot be a bmap");
-                                        return FAILURE;
+                                    VariableType::HashMap(_) => {
+                                        Err("index variable cannot be a hmap".to_string())
                                     }
-                                    Err(why) => {
-                                        eprintln!("ion: assignment error: {}: {}", key.name, why);
-                                        return FAILURE;
+                                    VariableType::BTreeMap(_) => {
+                                        Err("index variable cannot be a bmap".to_string())
                                     }
-                                    _ => (),
-                                }
-                            }
-                        }
-                        _ => (),
+                                    _ => Ok(()),
+                                }),
+                            _ => Ok(()),
+                        })
+                        .unwrap_or(Ok(()));
+
+                    if let Err(why) = err {
+                        eprintln!("ion: {}", why);
+                        return FAILURE;
                     }
                 }
-                Ok(Action::UpdateString(key, operator, ..)) => {
+                Action::UpdateString(key, operator, ..) => {
                     if operator == Operator::OptionalEqual
                         && self.variables.get_ref(key.name).is_some()
                     {
                         continue;
                     }
                     match collected.remove(key.name) {
-                        str_ @ Some(VariableType::Str(_)) => {
-                            self.variables.set(key.name, str_.unwrap());
-                        }
-                        array @ Some(VariableType::Array(_)) => {
-                            self.variables.set(key.name, array.unwrap());
+                        Some(var @ VariableType::Str(_)) | Some(var @ VariableType::Array(_)) => {
+                            self.variables.set(key.name, var);
                         }
                         _ => (),
                     }
                 }
-                _ => unreachable!(),
             }
         }
 
@@ -601,13 +445,16 @@ fn parse_i64<F: Fn(i64, i64) -> Option<i64>>(
     }
 }
 
-fn write_integer<F: FnMut(&[u8])>(integer: i64, mut func: F) {
+fn write_integer<F: FnMut(&[u8]) -> Result<(), MathError>>(
+    integer: i64,
+    mut func: F,
+) -> Result<(), MathError> {
     let mut buffer: [u8; 20] = unsafe { mem::uninitialized() };
     let capacity = itoa::write(&mut buffer[..], integer).unwrap();
-    func(&buffer[..capacity]);
+    func(&buffer[..capacity])
 }
 
-fn math<'a, F: FnMut(&[u8])>(
+fn math<'a, F: FnMut(&[u8]) -> Result<(), MathError>>(
     lhs: &str,
     key: &Primitive,
     operator: Operator,
@@ -615,72 +462,64 @@ fn math<'a, F: FnMut(&[u8])>(
     mut writefn: F,
 ) -> Result<(), MathError> {
     match operator {
-        Operator::Add => {
-            if Primitive::Any == *key || Primitive::Float == *key {
-                writefn(parse_f64(lhs, value, |lhs, rhs| lhs + rhs)?.to_string().as_bytes());
-            } else if let Primitive::Integer = key {
-                write_integer(parse_i64(lhs, value, |lhs, rhs| Some(lhs + rhs))?, writefn);
-            } else {
-                return Err(MathError::Unsupported);
+        Operator::Add => match key {
+            Primitive::Any | Primitive::Float => {
+                writefn(parse_f64(lhs, value, |lhs, rhs| lhs + rhs)?.to_string().as_bytes())
             }
-        }
-        Operator::Divide => {
-            if Primitive::Any == *key || Primitive::Float == *key || Primitive::Integer == *key {
-                writefn(parse_f64(lhs, value, |lhs, rhs| lhs / rhs)?.to_string().as_bytes());
-            } else {
-                return Err(MathError::Unsupported);
+            Primitive::Integer => {
+                write_integer(parse_i64(lhs, value, |lhs, rhs| Some(lhs + rhs))?, writefn)
             }
-        }
-        Operator::IntegerDivide => {
-            if Primitive::Any == *key || Primitive::Float == *key {
-                write_integer(
-                    parse_i64(lhs, value, |lhs, rhs| {
-                        // We want to make sure we don't divide by zero, so instead, we give them a None as a result to signify that we were unable to calculate the result.
-                        if rhs == 0 {
-                            None
-                        } else {
-                            Some(lhs / rhs)
-                        }
-                    })?,
-                    writefn,
-                );
-            } else {
-                return Err(MathError::Unsupported);
+            _ => Err(MathError::Unsupported),
+        },
+        Operator::Divide => match key {
+            Primitive::Any | Primitive::Float | Primitive::Integer => {
+                writefn(parse_f64(lhs, value, |lhs, rhs| lhs / rhs)?.to_string().as_bytes())
             }
-        }
-        Operator::Subtract => {
-            if Primitive::Any == *key || Primitive::Float == *key {
-                writefn(parse_f64(lhs, value, |lhs, rhs| lhs - rhs)?.to_string().as_bytes());
-            } else if let Primitive::Integer = key {
-                write_integer(parse_i64(lhs, value, |lhs, rhs| Some(lhs - rhs))?, writefn);
-            } else {
-                return Err(MathError::Unsupported);
+            _ => Err(MathError::Unsupported),
+        },
+        Operator::IntegerDivide => match key {
+            Primitive::Any | Primitive::Float => write_integer(
+                parse_i64(lhs, value, |lhs, rhs| {
+                    // We want to make sure we don't divide by zero, so instead, we give them a None
+                    // as a result to signify that we were unable to calculate the result.
+                    if rhs == 0 {
+                        None
+                    } else {
+                        Some(lhs / rhs)
+                    }
+                })?,
+                writefn,
+            ),
+            _ => Err(MathError::Unsupported),
+        },
+        Operator::Subtract => match key {
+            Primitive::Any | Primitive::Float => {
+                writefn(parse_f64(lhs, value, |lhs, rhs| lhs - rhs)?.to_string().as_bytes())
             }
-        }
-        Operator::Multiply => {
-            if Primitive::Any == *key || Primitive::Float == *key {
-                writefn(parse_f64(lhs, value, |lhs, rhs| lhs * rhs)?.to_string().as_bytes());
-            } else if let Primitive::Integer = key {
-                write_integer(parse_i64(lhs, value, |lhs, rhs| Some(lhs * rhs))?, writefn);
-            } else {
-                return Err(MathError::Unsupported);
+            Primitive::Integer => {
+                write_integer(parse_i64(lhs, value, |lhs, rhs| Some(lhs - rhs))?, writefn)
             }
-        }
-        Operator::Exponent => {
-            if Primitive::Any == *key || Primitive::Float == *key {
-                writefn(parse_f64(lhs, value, |lhs, rhs| lhs.powf(rhs))?.to_string().as_bytes());
-            } else if let Primitive::Integer = key {
-                write_integer(
-                    parse_i64(lhs, value, |lhs, rhs| Some(lhs.pow(rhs as u32)))?,
-                    writefn,
-                );
-            } else {
-                return Err(MathError::Unsupported);
+            _ => Err(MathError::Unsupported),
+        },
+        Operator::Multiply => match key {
+            Primitive::Any | Primitive::Float => {
+                writefn(parse_f64(lhs, value, |lhs, rhs| lhs * rhs)?.to_string().as_bytes())
             }
-        }
+            Primitive::Integer => {
+                write_integer(parse_i64(lhs, value, |lhs, rhs| Some(lhs * rhs))?, writefn)
+            }
+            _ => Err(MathError::Unsupported),
+        },
+        Operator::Exponent => match key {
+            Primitive::Any | Primitive::Float => {
+                writefn(parse_f64(lhs, value, |lhs, rhs| lhs.powf(rhs))?.to_string().as_bytes())
+            }
+            Primitive::Integer => {
+                write_integer(parse_i64(lhs, value, |lhs, rhs| Some(lhs.pow(rhs as u32)))?, writefn)
+            }
+            _ => Err(MathError::Unsupported),
+        },
         Operator::Equal => writefn(value.as_bytes()),
-        _ => return Err(MathError::Unsupported),
-    };
-
-    Ok(())
+        _ => Err(MathError::Unsupported),
+    }
 }
-- 
GitLab