Cargo.lock

@@ -840,7 +840,7 @@ dependencies = [
  "httparse",
  "httpdate",
  "itoa",
- "pin-project 1.0.5",
+ "pin-project 1.0.4",
  "socket2",
  "tokio",
  "tower-service",
@@ -1036,7 +1036,7 @@ checksum = "7ffc5c5338469d4d3ea17d269fa8ea3512ad247247c30bd2df69e68309ed0a08"
 [[package]]
 name = "matrix-sdk"
 version = "0.2.0"
-source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#bdaed6237e104b738b5ad25c063675373d9fa60d"
+source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#b66c666997cbcf32071d24ffe2484a215de8d0e4"
 dependencies = [
  "dashmap",
  "futures",
@@ -1058,7 +1058,7 @@ dependencies = [
 [[package]]
 name = "matrix-sdk-base"
 version = "0.2.0"
-source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#bdaed6237e104b738b5ad25c063675373d9fa60d"
+source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#b66c666997cbcf32071d24ffe2484a215de8d0e4"
 dependencies = [
  "chacha20poly1305",
  "dashmap",
@@ -1081,7 +1081,7 @@ dependencies = [
 [[package]]
 name = "matrix-sdk-common"
 version = "0.2.0"
-source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#bdaed6237e104b738b5ad25c063675373d9fa60d"
+source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#b66c666997cbcf32071d24ffe2484a215de8d0e4"
 dependencies = [
  "async-trait",
  "futures-locks",
@@ -1095,7 +1095,7 @@ dependencies = [
 [[package]]
 name = "matrix-sdk-crypto"
 version = "0.2.0"
-source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#bdaed6237e104b738b5ad25c063675373d9fa60d"
+source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#b66c666997cbcf32071d24ffe2484a215de8d0e4"
 dependencies = [
  "aes-ctr",
  "aes-gcm",
@@ -1425,11 +1425,11 @@ dependencies = [
 
 [[package]]
 name = "pin-project"
-version = "1.0.5"
+version = "1.0.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "96fa8ebb90271c4477f144354485b8068bd8f6b78b428b01ba892ca26caf0b63"
+checksum = "95b70b68509f17aa2857863b6fa00bf21fc93674c7a8893de2f469f6aa7ca2f2"
 dependencies = [
- "pin-project-internal 1.0.5",
+ "pin-project-internal 1.0.4",
 ]
 
 [[package]]
@@ -1445,9 +1445,9 @@ dependencies = [
 
 [[package]]
 name = "pin-project-internal"
-version = "1.0.5"
+version = "1.0.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "758669ae3558c6f74bd2a18b41f7ac0b5a195aea6639d6a9b5e5d1ad5ba24c0b"
+checksum = "caa25a6393f22ce819b0f50e0be89287292fda8d425be38ee0ca14c4931d9e71"
 dependencies = [
  "proc-macro2 1.0.24",
  "quote 1.0.8",

Cargo.toml

@@ -22,7 +22,7 @@ async-trait = "0.1"
 url = "2.1"
 dirs = "3.0"
 indoc = "1.0"
-combine = "4.5"
+combine = "4.3"
 sled = "0.34"
 zerocopy = "0.3"
 byteorder = "1.3"

src/error.rs

@@ -68,7 +68,7 @@ pub enum BotError {
     VariablesNotSupported,
 
     #[error("database error")]
-    DatabaseError(#[from] sled::Error),
+    DatabaseErrror(#[from] sled::Error),
 
     #[error("too many commands or message was too large")]
     MessageTooLarge,

src/parser.rs

@@ -1,11 +1,11 @@
-use combine::error::ParseError;
 use combine::parser::char::{digit, letter, spaces};
-use combine::stream::Stream;
 use combine::{many, many1, one_of, Parser};
 use thiserror::Error;
 
-/// Errors for dice parsing.
+//******************************
-#[derive(Debug, Clone, PartialEq, Copy, Error)]
+//New hotness
+//******************************
+#[derive(Debug, Clone, Copy, PartialEq, Error)]
 pub enum DiceParsingError {
     #[error("invalid amount")]
     InvalidAmount,
@@ -18,21 +18,8 @@ pub enum DiceParsingError {
 
     #[error("{0}")]
     InternalParseError(#[from] combine::error::StringStreamError),
-
-    #[error("number parsing error (too large?)")]
-    ConversionError,
 }
 
-impl From<std::num::ParseIntError> for DiceParsingError {
-    fn from(_error: std::num::ParseIntError) -> Self {
-        DiceParsingError::ConversionError
-    }
-}
-
-type ParseResult<T> = Result<T, DiceParsingError>;
-
-/// A parsed operator for a number. Whether to add or remove it from
-/// the total amount of dice rolled.
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub enum Operator {
     Plus,
@@ -40,8 +27,6 @@ pub enum Operator {
 }
 
 impl Operator {
-    /// Calculate multiplier for how to convert the number. Returns 1
-    /// for positive, and -1 for negative.
     pub fn mult(&self) -> i32 {
         match self {
             Operator::Plus => 1,
@@ -50,98 +35,18 @@ impl Operator {
     }
 }
 
-/// One part of the dice amount in an expression. Can be a number or a
-/// variable name.
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub enum Element {
-    /// This element in the expression is a variable, which will be
-    /// resolved to a number by consulting the dtaabase.
     Variable(String),
-
-    /// This element is a simple number, and will be added or
-    /// subtracted from the total dice amount depending on its
-    /// corresponding Operator.
     Number(i32),
 }
 
-/// One part of the parsed dice rolling expression. Combines an
-/// operator and an element into one struct. Examples of Amounts would
-/// be "+4" or "- myvariable", which translate to Operator::Plus and
-/// Element::Number(4), and Operator::Minus and
-/// Element::Variable("myvariable"), respectively.
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub struct Amount {
     pub operator: Operator,
     pub element: Element,
 }
 
-/// Parser that attempt to convert the text at the start of the dice
-/// parsing into an Amount instance.
-fn first_amount_parser<Input>() -> impl Parser<Input, Output = ParseResult<Amount>>
-where
-    Input: Stream<Token = char>,
-    Input::Error: ParseError<Input::Token, Input::Range, Input::Position>,
-{
-    let map_first_amount = |value: String| {
-        if value.chars().all(char::is_numeric) {
-            let num = value.parse::<i32>()?;
-            Ok(Amount {
-                operator: Operator::Plus,
-                element: Element::Number(num),
-            })
-        } else {
-            Ok(Amount {
-                operator: Operator::Plus,
-                element: Element::Variable(value),
-            })
-        }
-    };
-
-    many1(letter())
-        .or(many1(digit()))
-        .skip(spaces().silent()) //Consume any space after first amount
-        .map(map_first_amount)
-}
-
-/// Attempt to convert some text in the middle or end of the dice roll
-/// string into an Amount.
-fn amount_parser<Input>() -> impl Parser<Input, Output = ParseResult<Amount>>
-where
-    Input: Stream<Token = char>,
-    Input::Error: ParseError<Input::Token, Input::Range, Input::Position>,
-{
-    let plus_or_minus = one_of("+-".chars());
-    let parse_operator = plus_or_minus.map(|sign: char| match sign {
-        '+' => Operator::Plus,
-        '-' => Operator::Minus,
-        _ => Operator::Plus,
-    });
-
-    // Element must either be a proper i32, or a variable name.
-    let map_element = |value: String| -> ParseResult<Element> {
-        if value.chars().all(char::is_numeric) {
-            let num = value.parse::<i32>()?;
-            Ok(Element::Number(num))
-        } else {
-            Ok(Element::Variable(value))
-        }
-    };
-
-    let parse_element = many1(letter()).or(many1(digit())).map(map_element);
-
-    let element_parser = parse_operator
-        .skip(spaces().silent())
-        .and(parse_element)
-        .skip(spaces().silent());
-
-    let convert_to_amount = |(operator, element_result)| match element_result {
-        Ok(element) => Ok(Amount { operator, element }),
-        Err(e) => Err(e),
-    };
-
-    element_parser.map(convert_to_amount)
-}
-
 /// 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
@@ -152,15 +57,55 @@ where
 /// 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) -> ParseResult<Vec<Amount>> {
+pub fn parse_amounts(input: &str) -> Result<Vec<Amount>, DiceParsingError> {
     let input = input.trim();
 
-    let remaining_amounts = many(amount_parser()).map(|amounts: Vec<ParseResult<Amount>>| amounts);
+    let plus_or_minus = one_of("+-".chars());
-    let mut parser = first_amount_parser().and(remaining_amounts);
+    let maybe_sign = plus_or_minus.map(|sign: char| match sign {
+        '+' => Operator::Plus,
+        '-' => Operator::Minus,
+        _ => Operator::Plus,
+    });
 
-    // Collapses first amount + remaining amounts into a single Vec,
+    //TODO make this a macro or something
-    // while collecting extraneous input.
+    let first = many1(letter())
-    type ParsedAmountExpr = (ParseResult<Amount>, Vec<ParseResult<Amount>>);
+        .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)| {
@@ -170,8 +115,7 @@ pub fn parse_amounts(input: &str) -> ParseResult<Vec<Amount>> {
         })?;
 
     if rest.len() == 0 {
-        // Any ParseResult errors will short-circuit the collect.
+        Ok(results)
-        results.into_iter().collect()
     } else {
         Err(DiceParsingError::UnconsumedInput)
     }
@@ -205,15 +149,6 @@ mod tests {
         );
     }
 
-    #[test]
-    fn parsing_huge_number_should_error() {
-        // A number outside the bounds of i32 should not be a valid
-        // parse.
-        let result = parse_amounts("159875294375198734982379875392");
-        assert!(result.is_err());
-        assert!(result.unwrap_err() == DiceParsingError::ConversionError);
-    }
-
     #[test]
     fn parse_single_variable_amount_test() {
         let result = parse_amounts("asdf");