Centralize common parsing code.

Needed for further development of different systems that rely on these
kind of expressions, and lays groundwork for future changes.

src/cofd/dice.rs

@@ -1,5 +1,6 @@
 use crate::context::Context;
 use crate::error::BotError;
+use crate::parser::{Amount, Element, Operator};
 use crate::roll::Rolled;
 use futures::stream::{self, StreamExt, TryStreamExt};
 use itertools::Itertools;
@@ -16,33 +17,6 @@ pub enum DiceRollingError {
     ExpressionTooLarge,
 }
 
-#[derive(Debug, PartialEq, Eq, Clone)]
-pub enum Operator {
-    Plus,
-    Minus,
-}
-
-impl Operator {
-    pub fn mult(&self) -> i32 {
-        match self {
-            Operator::Plus => 1,
-            Operator::Minus => -1,
-        }
-    }
-}
-
-#[derive(Debug, PartialEq, Eq, Clone)]
-pub enum Element {
-    Variable(String),
-    Number(i32),
-}
-
-#[derive(Debug, PartialEq, Eq, Clone)]
-pub struct Amount {
-    pub operator: Operator,
-    pub element: Element,
-}
-
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 pub enum DicePoolQuality {
     TenAgain,

src/cofd/parser.rs

@@ -1,8 +1,8 @@
-use crate::cofd::dice::{Amount, DicePool, DicePoolModifiers, DicePoolQuality, Element, Operator};
+use crate::cofd::dice::{DicePool, DicePoolModifiers, DicePoolQuality};
 use crate::error::BotError;
-use combine::error::StringStreamError;
+use crate::parser::{parse_amounts, DiceParsingError};
-use combine::parser::char::{digit, letter, spaces, string};
+use combine::parser::char::{digit, spaces, string};
-use combine::{choice, count, many, many1, one_of, Parser};
+use combine::{choice, count, many1, one_of, Parser};
 
 #[derive(Debug, Clone, Copy, PartialEq)]
 enum ParsedInfo {
@@ -10,37 +10,7 @@ enum ParsedInfo {
     ExceptionalOn(i32),
 }
 
-#[derive(Debug, Clone, Copy, PartialEq)]
+pub fn parse_modifiers(input: &str) -> Result<DicePoolModifiers, DiceParsingError> {
-pub enum DiceParsingError {
-    InvalidAmount,
-    InvalidModifiers,
-    UnconsumedInput,
-}
-
-impl std::fmt::Display for DiceParsingError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}", self.as_str())
-    }
-}
-
-impl std::error::Error for DiceParsingError {
-    fn description(&self) -> &str {
-        self.as_str()
-    }
-}
-
-impl DiceParsingError {
-    fn as_str(&self) -> &str {
-        use self::DiceParsingError::*;
-        match *self {
-            InvalidAmount => "invalid amount of dice",
-            InvalidModifiers => "dice pool modifiers not specified properly",
-            UnconsumedInput => "extraneous input detected",
-        }
-    }
-}
-
-pub fn parse_modifiers(input: &str) -> Result<DicePoolModifiers, BotError> {
     if input.len() == 0 {
         return Ok(DicePoolModifiers::default());
     }
@@ -77,13 +47,11 @@ pub fn parse_modifiers(input: &str) -> Result<DicePoolModifiers, BotError> {
     if rest.len() == 0 {
         convert_to_info(&result)
     } else {
-        Err(BotError::DiceParsingError(
+        Err(DiceParsingError::UnconsumedInput)
-            DiceParsingError::UnconsumedInput,
-        ))
     }
 }
 
-fn convert_to_info(parsed: &Vec<ParsedInfo>) -> Result<DicePoolModifiers, BotError> {
+fn convert_to_info(parsed: &Vec<ParsedInfo>) -> Result<DicePoolModifiers, DiceParsingError> {
     use ParsedInfo::*;
     if parsed.len() == 0 {
         Ok(DicePoolModifiers::default())
@@ -110,77 +78,6 @@ fn convert_to_info(parsed: &Vec<ParsedInfo>) -> Result<DicePoolModifiers, BotErr
     }
 }
 
-/// Parse dice pool amounts into elements coupled with operators,
-/// where an operator is "+" or "-", and an element is either a number
-/// or variable name. The first element should not have an operator,
-/// but every one after that should. Accepts expressions like "8", "10
-/// + variablename", "variablename - 3", etc.
-fn parse_pool_amount(input: &str) -> Result<Vec<Amount>, BotError> {
-    let input = input.trim();
-
-    let plus_or_minus = one_of("+-".chars());
-    let maybe_sign = plus_or_minus.map(|sign: char| match sign {
-        '+' => Operator::Plus,
-        '-' => Operator::Minus,
-        _ => Operator::Plus,
-    });
-
-    //TODO make this a macro or something
-    let first = many1(letter())
-        .or(many1(digit()))
-        .skip(spaces().silent()) //Consume any space after first amount
-        .map(|value: String| match value.parse::<i32>() {
-            Ok(num) => Amount {
-                operator: Operator::Plus,
-                element: Element::Number(num),
-            },
-            _ => Amount {
-                operator: Operator::Plus,
-                element: Element::Variable(value),
-            },
-        });
-
-    let variable_or_number =
-        many1(letter())
-            .or(many1(digit()))
-            .map(|value: String| match value.parse::<i32>() {
-                Ok(num) => Element::Number(num),
-                _ => Element::Variable(value),
-            });
-
-    let sign_and_word = maybe_sign
-        .skip(spaces().silent())
-        .and(variable_or_number)
-        .skip(spaces().silent())
-        .map(|parsed: (Operator, Element)| Amount {
-            operator: parsed.0,
-            element: parsed.1,
-        });
-
-    let rest = many(sign_and_word).map(|expr: Vec<_>| expr);
-
-    let mut parser = first.and(rest);
-
-    //Maps the found expression into a Vec of Amount instances,
-    //tacking the first one on.
-    type ParsedAmountExpr = (Amount, Vec<Amount>);
-    let (results, rest) = parser
-        .parse(input)
-        .map(|mut results: (ParsedAmountExpr, &str)| {
-            let mut amounts = vec![(results.0).0];
-            amounts.append(&mut (results.0).1);
-            (amounts, results.1)
-        })?;
-
-    if rest.len() == 0 {
-        Ok(results)
-    } else {
-        Err(BotError::DiceParsingError(
-            DiceParsingError::UnconsumedInput,
-        ))
-    }
-}
-
 pub fn parse_dice_pool(input: &str) -> Result<DicePool, BotError> {
     //The "modifiers:" part is optional. Assume amounts if no modifier
     //section found.
@@ -194,11 +91,11 @@ pub fn parse_dice_pool(input: &str) -> Result<DicePool, BotError> {
     })?;
 
     let modifiers = parse_modifiers(modifiers_str)?;
-    let amounts = parse_pool_amount(&amounts_str)?;
+    let amounts = parse_amounts(&amounts_str)?;
     Ok(DicePool::new(amounts, modifiers))
 }
 
-pub fn create_chance_die() -> Result<DicePool, StringStreamError> {
+pub fn create_chance_die() -> Result<DicePool, BotError> {
     Ok(DicePool::chance_die())
 }
 
@@ -206,57 +103,6 @@ pub fn create_chance_die() -> Result<DicePool, StringStreamError> {
 mod tests {
     use super::*;
 
-    #[test]
-    fn parse_single_number_amount_test() {
-        let result = parse_pool_amount("1");
-        assert!(result.is_ok());
-        assert_eq!(
-            result.unwrap(),
-            vec![Amount {
-                operator: Operator::Plus,
-                element: Element::Number(1)
-            }]
-        );
-
-        let result = parse_pool_amount("10");
-        assert!(result.is_ok());
-        assert_eq!(
-            result.unwrap(),
-            vec![Amount {
-                operator: Operator::Plus,
-                element: Element::Number(10)
-            }]
-        );
-    }
-
-    #[test]
-    fn parse_single_variable_amount_test() {
-        let result = parse_pool_amount("asdf");
-        assert!(result.is_ok());
-        assert_eq!(
-            result.unwrap(),
-            vec![Amount {
-                operator: Operator::Plus,
-                element: Element::Variable("asdf".to_string())
-            }]
-        );
-
-        let result = parse_pool_amount("nosis");
-        assert!(result.is_ok());
-        assert_eq!(
-            result.unwrap(),
-            vec![Amount {
-                operator: Operator::Plus,
-                element: Element::Variable("nosis".to_string())
-            }]
-        );
-    }
-
-    #[test]
-    fn parse_complex_amount_expression() {
-        assert!(parse_pool_amount("1 + myvariable - 2").is_ok());
-    }
-
     #[test]
     fn quality_test() {
         let result = parse_modifiers("n");
@@ -289,24 +135,14 @@ mod tests {
 
         let result = parse_modifiers("b");
         assert!(result.is_err());
-        assert!(matches!(
+        assert!(matches!(result, Err(DiceParsingError::UnconsumedInput)));
-            result,
-            Err(BotError::DiceParsingError(
-                DiceParsingError::UnconsumedInput
-            ))
-        ));
     }
 
     #[test]
     fn multiple_quality_failure_test() {
         let result = parse_modifiers("ne");
         assert!(result.is_err());
-        assert!(matches!(
+        assert!(matches!(result, Err(DiceParsingError::InvalidModifiers)));
-            result,
-            Err(BotError::DiceParsingError(
-                DiceParsingError::InvalidModifiers
-            ))
-        ));
     }
 
     #[test]
@@ -324,12 +160,7 @@ mod tests {
 
         let result = parse_modifiers("s3q");
         assert!(result.is_err());
-        assert!(matches!(
+        assert!(matches!(result, Err(DiceParsingError::UnconsumedInput)));
-            result,
-            Err(BotError::DiceParsingError(
-                DiceParsingError::UnconsumedInput
-            ))
-        ));
     }
 
     #[test]
@@ -370,6 +201,7 @@ mod tests {
 
     #[test]
     fn dice_pool_complex_expression_test() {
+        use crate::parser::*;
         let modifiers = DicePoolModifiers::custom(DicePoolQuality::Rote, 3);
         let amounts = vec![
             Amount {

src/commands/parser.rs

@@ -17,6 +17,13 @@ use crate::variables::parse_set_variable;
 use combine::parser::char::{char, letter, space};
 use combine::{any, many1, optional, Parser};
 use nom::Err as NomErr;
+use thiserror::Error;
+
+#[derive(Debug, Clone, Copy, PartialEq, Error)]
+pub enum CommandParsingError {
+    #[error("parser error: {0}")]
+    InternalParseError(#[from] combine::error::StringStreamError),
+}
 
 // Parse a roll expression.
 fn parse_roll(input: &str) -> Result<Box<dyn Command>, BotError> {
@@ -75,7 +82,7 @@ fn help(topic: &str) -> Result<Box<dyn Command>, BotError> {
 /// else" parts. Extracts the command separately from its input (i.e.
 /// rest of the line) and returns a tuple of (command_input, command).
 /// Whitespace at the start and end of the command input is removed.
-fn split_command(input: &str) -> Result<(String, String), BotError> {
+fn split_command(input: &str) -> Result<(String, String), CommandParsingError> {
     let input = input.trim();
 
     let exclamation = char('!');
@@ -121,7 +128,7 @@ pub fn parse_command(input: &str) -> Result<Option<Box<dyn Command>>, BotError>
             _ => Ok(None),
         },
         //All other errors passed up.
-        Err(e) => Err(e),
+        Err(e) => Err(e.into()),
     }
 }
 

src/cthulhu/dice.rs

@@ -144,7 +144,7 @@ impl fmt::Display for RolledDice {
 pub struct AdvancementRoll {
     /// The amount (0 to 100) of the existing skill. We must beat this
     /// target number to advance the skill, or roll above a 95.
-    existing_skill: u32,
+    pub existing_skill: u32,
 }
 
 /// A completed advancement roll.

src/cthulhu/parser.rs

@@ -0,0 +1,16 @@
+use super::dice::{AdvancementRoll, DiceRoll};
+use crate::error::BotError;
+use combine::error::StringStreamError;
+use combine::parser::char::{digit, letter, spaces, string};
+use combine::{choice, count, many, many1, one_of, Parser};
+
+pub fn parse_roll(input: &str) -> Result<DiceRoll, ParsingError> {
+    Ok(DiceRoll {
+        target: 50,
+        modifier: DiceRollModifier::Normal,
+    })
+}
+
+pub fn parse_advancement_roll(input: &str) -> Result<AdvancementRoll, ParsingError> {
+    Ok(AdvancementRoll { existing_skill: 50 })
+}

src/error.rs

@@ -44,11 +44,11 @@ pub enum BotError {
     #[error("i/o error: {0}")]
     IoError(#[from] std::io::Error),
 
-    #[error("parsing error")]
-    ParserError(#[from] combine::error::StringStreamError),
-
     #[error("dice parsing error: {0}")]
-    DiceParsingError(#[from] crate::cofd::parser::DiceParsingError),
+    DiceParsingError(#[from] crate::parser::DiceParsingError),
+
+    #[error("command parsing error: {0}")]
+    CommandParsingError(#[from] crate::commands::parser::CommandParsingError),
 
     #[error("dice pool roll error: {0}")]
     DiceRollingError(#[from] DiceRollingError),

src/parser.rs

@@ -1,4 +1,130 @@
+use combine::parser::char::{digit, letter, spaces};
+use combine::{many, many1, one_of, Parser};
 use nom::{bytes::complete::take_while, IResult};
+use thiserror::Error;
+
+//******************************
+//New hotness
+//******************************
+#[derive(Debug, Clone, Copy, PartialEq, Error)]
+pub enum DiceParsingError {
+    #[error("invalid amount of dice")]
+    InvalidAmount,
+
+    #[error("modifiers not specified properly")]
+    InvalidModifiers,
+
+    #[error("extraneous input detected")]
+    UnconsumedInput,
+
+    #[error("parser error: {0}")]
+    InternalParseError(#[from] combine::error::StringStreamError),
+}
+
+#[derive(Debug, PartialEq, Eq, Clone)]
+pub enum Operator {
+    Plus,
+    Minus,
+}
+
+impl Operator {
+    pub fn mult(&self) -> i32 {
+        match self {
+            Operator::Plus => 1,
+            Operator::Minus => -1,
+        }
+    }
+}
+
+#[derive(Debug, PartialEq, Eq, Clone)]
+pub enum Element {
+    Variable(String),
+    Number(i32),
+}
+
+#[derive(Debug, PartialEq, Eq, Clone)]
+pub struct Amount {
+    pub operator: Operator,
+    pub element: Element,
+}
+
+/// Parse an expression of numbers and/or variables into elements
+/// coupled with operators, where an operator is "+" or "-", and an
+/// element is either a number or variable name. The first element
+/// should not have an operator, but every one after that should.
+/// Accepts expressions like "8", "10 + variablename", "variablename -
+/// 3", etc. This function is currently common to systems that don't
+/// deal with XdY rolls. Support for that will be added later. Parsers
+/// utilzing this function should layer their own checks on top of
+/// this; perhaps they do not want more than one expression, or some
+/// other rules.
+pub fn parse_amounts(input: &str) -> Result<Vec<Amount>, DiceParsingError> {
+    let input = input.trim();
+
+    let plus_or_minus = one_of("+-".chars());
+    let maybe_sign = plus_or_minus.map(|sign: char| match sign {
+        '+' => Operator::Plus,
+        '-' => Operator::Minus,
+        _ => Operator::Plus,
+    });
+
+    //TODO make this a macro or something
+    let first = many1(letter())
+        .or(many1(digit()))
+        .skip(spaces().silent()) //Consume any space after first amount
+        .map(|value: String| match value.parse::<i32>() {
+            Ok(num) => Amount {
+                operator: Operator::Plus,
+                element: Element::Number(num),
+            },
+            _ => Amount {
+                operator: Operator::Plus,
+                element: Element::Variable(value),
+            },
+        });
+
+    let variable_or_number =
+        many1(letter())
+            .or(many1(digit()))
+            .map(|value: String| match value.parse::<i32>() {
+                Ok(num) => Element::Number(num),
+                _ => Element::Variable(value),
+            });
+
+    let sign_and_word = maybe_sign
+        .skip(spaces().silent())
+        .and(variable_or_number)
+        .skip(spaces().silent())
+        .map(|parsed: (Operator, Element)| Amount {
+            operator: parsed.0,
+            element: parsed.1,
+        });
+
+    let rest = many(sign_and_word).map(|expr: Vec<_>| expr);
+
+    let mut parser = first.and(rest);
+
+    //Maps the found expression into a Vec of Amount instances,
+    //tacking the first one on.
+    type ParsedAmountExpr = (Amount, Vec<Amount>);
+    let (results, rest) = parser
+        .parse(input)
+        .map(|mut results: (ParsedAmountExpr, &str)| {
+            let mut amounts = vec![(results.0).0];
+            amounts.append(&mut (results.0).1);
+            (amounts, results.1)
+        })?;
+
+    if rest.len() == 0 {
+        Ok(results)
+    } else {
+        Err(DiceParsingError::UnconsumedInput)
+    }
+}
+
+//******************************
+//Legacy Code
+//******************************
 
 fn is_whitespace(input: char) -> bool {
     input == ' ' || input == '\n' || input == '\t' || input == '\r'
@@ -9,3 +135,60 @@ pub fn eat_whitespace(input: &str) -> IResult<&str, &str> {
     let (input, whitespace) = take_while(is_whitespace)(input)?;
     Ok((input, whitespace))
 }
+
+#[cfg(test)]
+mod tests {
+
+    use super::*;
+
+    #[test]
+    fn parse_single_number_amount_test() {
+        let result = parse_amounts("1");
+        assert!(result.is_ok());
+        assert_eq!(
+            result.unwrap(),
+            vec![Amount {
+                operator: Operator::Plus,
+                element: Element::Number(1)
+            }]
+        );
+
+        let result = parse_amounts("10");
+        assert!(result.is_ok());
+        assert_eq!(
+            result.unwrap(),
+            vec![Amount {
+                operator: Operator::Plus,
+                element: Element::Number(10)
+            }]
+        );
+    }
+
+    #[test]
+    fn parse_single_variable_amount_test() {
+        let result = parse_amounts("asdf");
+        assert!(result.is_ok());
+        assert_eq!(
+            result.unwrap(),
+            vec![Amount {
+                operator: Operator::Plus,
+                element: Element::Variable("asdf".to_string())
+            }]
+        );
+
+        let result = parse_amounts("nosis");
+        assert!(result.is_ok());
+        assert_eq!(
+            result.unwrap(),
+            vec![Amount {
+                operator: Operator::Plus,
+                element: Element::Variable("nosis".to_string())
+            }]
+        );
+    }
+
+    #[test]
+    fn parse_complex_amount_expression() {
+        assert!(parse_amounts("1 + myvariable - 2").is_ok());
+    }
+}

src/variables.rs

@@ -1,4 +1,3 @@
-use crate::error::BotError;
 use combine::parser::char::{char, digit, letter, spaces};
 use combine::{many1, optional, Parser};
 use thiserror::Error;
@@ -15,9 +14,12 @@ pub enum VariableParsingError {
 
     #[error("unconsumed input")]
     UnconsumedInput,
+
+    #[error("parser error: {0}")]
+    InternalParseError(#[from] combine::error::StringStreamError),
 }
 
-pub fn parse_set_variable(input: &str) -> Result<(String, i32), BotError> {
+pub fn parse_set_variable(input: &str) -> Result<(String, i32), VariableParsingError> {
     let name = many1(letter()).map(|value: String| value);
 
     let maybe_minus = optional(char('-')).map(|value: Option<char>| match value {
@@ -41,14 +43,10 @@ pub fn parse_set_variable(input: &str) -> Result<(String, i32), BotError> {
     if rest.len() == 0 {
         match result {
             (variable_name, ParsedValue::Valid(value)) => Ok((variable_name, value)),
-            _ => Err(BotError::VariableParsingError(
+            _ => Err(VariableParsingError::InvalidValue),
-                VariableParsingError::InvalidValue,
-            )),
         }
     } else {
-        Err(BotError::VariableParsingError(
+        Err(VariableParsingError::UnconsumedInput)
-            VariableParsingError::UnconsumedInput,
-        ))
     }
 }