refactor(parser): Remove explicit parsers outside pc package (#117)

* refactor(parser): Remove explicit parser implementation from expression

* refactor(parser): Improved binary operator parsing

* refactor(parser): Improved `guard` implementation

* refactor(parser): Removed explicit parser implementation for PrintArgs

* chore(parser): Use IifParser

* refactor(parser): Use MapToUnit

* Removed explicit parser implementation from sub_call

* Closes #116

Author: ngeor

rusty_parser/src/core/expression.rs

@@ -999,7 +999,7 @@ mod built_in_function_call {
 
 mod binary_expression {
     use rusty_common::Positioned;
-    use rusty_pc::and::opt_and_tuple;
+    use rusty_pc::and::{TupleCombiner, opt_and_keep_right};
     use rusty_pc::*;
 
     use super::{
@@ -1013,115 +1013,139 @@ mod binary_expression {
 
     // result ::= <non-bin-expr> <operator> <expr>
     pub fn parser() -> impl Parser<StringView, Output = ExpressionPos, Error = ParserError> {
-        BinaryExprParser
+        non_bin_expr()
+            .then_with_in_context(
+                second_parser(),
+                |first| first.is_parenthesis(),
+                TupleCombiner,
+            )
+            .map(|(l, r)| match r {
+                Some((op, r)) => l.apply_priority_order(r, op.element, op.pos),
+                None => l,
+            })
+            .map(ExpressionPos::simplify_unary_minus_literals)
     }
 
-    struct BinaryExprParser;
+    fn second_parser() -> impl Parser<
+        StringView,
+        bool,
+        Output = Option<(Positioned<Operator>, ExpressionPos)>,
+        Error = ParserError,
+    > + SetContext<bool> {
+        operator()
+            .then_with_in_context(third_parser(), |op| is_keyword_op(op), TupleCombiner)
+            .to_option()
+    }
 
-    impl Parser<StringView> for BinaryExprParser {
-        type Output = ExpressionPos;
-        type Error = ParserError;
+    fn is_keyword_op(op: &Positioned<Operator>) -> bool {
+        op.element == Operator::And || op.element == Operator::Or || op.element == Operator::Modulo
+    }
 
-        fn parse(&mut self, tokenizer: &mut StringView) -> Result<Self::Output, ParserError> {
-            self.do_parse(tokenizer)
-                .map(ExpressionPos::simplify_unary_minus_literals)
-        }
+    fn third_parser()
+    -> impl Parser<StringView, bool, Output = ExpressionPos, Error = ParserError> + SetContext<bool>
+    {
+        IifParser::new(
+            guard::parser().to_fatal(),
+            guard::parser().to_option().map_to_unit(),
+        )
+        .and_keep_right(right_side_expr().no_context())
     }
 
-    impl BinaryExprParser {
-        fn do_parse(&self, tokenizer: &mut StringView) -> Result<ExpressionPos, ParserError> {
-            let first = Self::non_bin_expr().parse(tokenizer)?;
+    /// Parses the right side expression, after having parsed the binary operator
+    fn right_side_expr() -> impl Parser<StringView, Output = ExpressionPos, Error = ParserError> {
+        // boxed breaks apart the recursive type evaluation
+        expression_pos_p()
+            .or_expected("expression after operator")
+            .boxed()
+    }
 
-            let is_paren = first.is_parenthesis();
-            match Self::operator(is_paren).parse(tokenizer) {
-                Ok(Positioned {
-                    element: op,
-                    pos: op_pos,
-                }) => {
-                    if let Err(err) = guard::parser().parse(tokenizer) {
-                        if err.is_soft() {
-                            let is_keyword_op =
-                                op == Operator::And || op == Operator::Or || op == Operator::Modulo;
-                            if is_keyword_op {
-                                return Err(ParserError::expected("whitespace or (").to_fatal());
-                            }
-                        } else {
-                            return Err(err);
-                        }
-                    }
+    fn non_bin_expr() -> impl Parser<StringView, Output = ExpressionPos, Error = ParserError> {
+        OrParser::new(vec![
+            Box::new(single_or_double_literal::parser()),
+            Box::new(string_literal::parser()),
+            Box::new(integer_or_long_literal::parser()),
+            // property internally uses variable and function_call_or_array_element so they can be skipped
+            Box::new(property::parser()),
+            Box::new(built_in_function_call::parser()),
+            Box::new(parenthesis::parser()),
+            Box::new(unary_expression::parser()),
+        ])
+    }
 
-                    expression_pos_p()
-                        .or_expected("expression after operator")
-                        .parse(tokenizer)
-                        .map(|right| first.apply_priority_order(right, op, op_pos))
-                }
-                Err(err) if err.is_soft() => Ok(first),
-                Err(err) => Err(err),
-            }
-        }
+    /// Parses an operator.
+    /// The parameter indicates if the previously parsed expression was wrapped in
+    /// parenthesis. If that is the case, leading whitespace is not required for
+    /// keyword based operators.
+    fn operator()
+    -> impl Parser<StringView, bool, Output = Positioned<Operator>, Error = ParserError>
+    + SetContext<bool> {
+        IifParser::new(
+            // no whitespace needed
+            opt_and_keep_right(whitespace_ignoring(), operator_p()),
+            // whitespace needed
+            whitespace_ignoring()
+                .and_keep_right(operator_p())
+                .or(opt_and_keep_right(
+                    whitespace_ignoring(),
+                    symbol_operator_p(),
+                )),
+        )
+    }
 
-        fn non_bin_expr() -> impl Parser<StringView, Output = ExpressionPos, Error = ParserError> {
-            OrParser::new(vec![
-                Box::new(single_or_double_literal::parser()),
-                Box::new(string_literal::parser()),
-                Box::new(integer_or_long_literal::parser()),
-                // property internally uses variable and function_call_or_array_element so they can be skipped
-                Box::new(property::parser()),
-                Box::new(built_in_function_call::parser()),
-                Box::new(parenthesis::parser()),
-                Box::new(unary_expression::parser()),
-            ])
-        }
-
-        fn operator(
-            is_paren: bool,
-        ) -> impl Parser<StringView, Output = Positioned<Operator>, Error = ParserError> {
-            opt_and_tuple(
-                whitespace_ignoring(),
-                any_token()
-                    .filter_map(Self::map_token_to_operator)
-                    .with_pos(),
-            )
-            .and_then(move |(leading_ws, op_pos)| {
-                let had_whitespace = leading_ws.is_some();
-                let needs_whitespace = matches!(
-                    &op_pos.element,
-                    Operator::Modulo | Operator::And | Operator::Or
-                );
-                if had_whitespace || is_paren || !needs_whitespace {
-                    Ok(op_pos)
-                } else {
-                    Err(ParserError::syntax_error(&format!(
-                        "Expected: parenthesis before operator {:?}",
-                        op_pos.element()
-                    )))
-                }
-            })
+    /// Parses an operator.
+    /// Does not check for leading whitespace, this needs to be done at the caller!
+    fn operator_p() -> impl Parser<StringView, Output = Positioned<Operator>, Error = ParserError> {
+        any_token().filter_map(map_token_to_operator).with_pos()
+    }
+
+    /// Parses a symbol operator (i.e. excludes keyword based operators).
+    /// Does not check for leading whitespace, this needs to be done at the caller!
+    fn symbol_operator_p()
+    -> impl Parser<StringView, Output = Positioned<Operator>, Error = ParserError> {
+        any_token()
+            .filter_map(map_token_to_symbol_operator)
+            .with_pos()
+    }
+
+    /// Maps the given token to an operator.
+    fn map_token_to_operator(token: &Token) -> Option<Operator> {
+        map_token_to_symbol_operator(token).or_else(|| map_token_to_keyword_operator(token))
+    }
+
+    /// Maps the given token to an operator, considering only operators
+    /// that are based on symbols (i.e. excludes keywords).
+    /// Symbol based operators do not require leading whitespace.
+    fn map_token_to_symbol_operator(token: &Token) -> Option<Operator> {
+        match TokenType::from_token(token) {
+            TokenType::LessEquals => Some(Operator::LessOrEqual),
+            TokenType::Less => Some(Operator::Less),
+            TokenType::GreaterEquals => Some(Operator::GreaterOrEqual),
+            TokenType::Greater => Some(Operator::Greater),
+            TokenType::Equals => Some(Operator::Equal),
+            TokenType::NotEquals => Some(Operator::NotEqual),
+            TokenType::Symbol => match token.demand_single_char() {
+                '+' => Some(Operator::Plus),
+                '-' => Some(Operator::Minus),
+                '*' => Some(Operator::Multiply),
+                '/' => Some(Operator::Divide),
+                _ => None,
+            },
+            _ => None,
         }
+    }
 
-        fn map_token_to_operator(token: &Token) -> Option<Operator> {
-            match TokenType::from_token(token) {
-                TokenType::LessEquals => Some(Operator::LessOrEqual),
-                TokenType::Less => Some(Operator::Less),
-                TokenType::GreaterEquals => Some(Operator::GreaterOrEqual),
-                TokenType::Greater => Some(Operator::Greater),
-                TokenType::Equals => Some(Operator::Equal),
-                TokenType::NotEquals => Some(Operator::NotEqual),
-                TokenType::Keyword => match Keyword::try_from(token.as_str()).unwrap() {
-                    Keyword::Mod => Some(Operator::Modulo),
-                    Keyword::And => Some(Operator::And),
-                    Keyword::Or => Some(Operator::Or),
-                    _ => None,
-                },
-                TokenType::Symbol => match token.demand_single_char() {
-                    '+' => Some(Operator::Plus),
-                    '-' => Some(Operator::Minus),
-                    '*' => Some(Operator::Multiply),
-                    '/' => Some(Operator::Divide),
-                    _ => None,
-                },
+    /// Maps the given token to an operator, considering only operators
+    /// that are based on keywords (i.e. excludes symbols).
+    /// Keyword based operators require leading whitespace.
+    fn map_token_to_keyword_operator(token: &Token) -> Option<Operator> {
+        match TokenType::from_token(token) {
+            TokenType::Keyword => match Keyword::try_from(token.as_str()).unwrap() {
+                Keyword::Mod => Some(Operator::Modulo),
+                Keyword::And => Some(Operator::And),
+                Keyword::Or => Some(Operator::Or),
                 _ => None,
-            }
+            },
+            _ => None,
         }
     }
 }
@@ -1218,34 +1242,24 @@ pub mod guard {
 
     use crate::ParserError;
     use crate::input::StringView;
-    use crate::tokens::{TokenMatcher, peek_token, whitespace_ignoring};
-
-    #[derive(Default)]
-    pub enum Guard {
-        #[default]
-        Peeked,
-        Whitespace,
-    }
+    use crate::pc_specific::WithExpected;
+    use crate::tokens::{any_symbol_of, any_token_of, whitespace_ignoring};
 
     /// `result ::= " " | "("`
     ///
     /// The "(" will be undone.
-    pub fn parser() -> impl Parser<StringView, Output = Guard, Error = ParserError> {
-        whitespace_guard().or(lparen_guard())
+    pub fn parser() -> impl Parser<StringView, Output = (), Error = ParserError> {
+        whitespace_guard()
+            .or(lparen_guard())
+            .with_expected_message("Expected: '(' or whitespace")
     }
 
-    fn whitespace_guard() -> impl Parser<StringView, Output = Guard, Error = ParserError> {
-        whitespace_ignoring().map(|_| Guard::Whitespace)
+    fn whitespace_guard() -> impl Parser<StringView, Output = (), Error = ParserError> {
+        whitespace_ignoring()
     }
 
-    fn lparen_guard() -> impl Parser<StringView, Output = Guard, Error = ParserError> {
-        peek_token().and_then(|token| {
-            if '('.matches_token(&token) {
-                Ok(Guard::Peeked)
-            } else {
-                default_parse_error()
-            }
-        })
+    fn lparen_guard() -> impl Parser<StringView, Output = (), Error = ParserError> {
+        any_symbol_of!('(').map_to_unit().peek()
     }
 }
 
@@ -1867,7 +1881,7 @@ mod tests {
                 ExpressionType::Unresolved
             )
         );
-        assert_parser_err!("PRINT 1AND 2", expected("parenthesis before operator And"));
+        assert_parser_err!("PRINT 1AND 2", expected("end-of-statement"));
         assert_expression!(
             "(1 OR 2)AND 3",
             Expression::BinaryExpression(
@@ -1897,7 +1911,7 @@ mod tests {
                 ExpressionType::Unresolved
             )
         );
-        assert_parser_err!("PRINT 1OR 2", expected("parenthesis before operator Or"));
+        assert_parser_err!("PRINT 1OR 2", expected("end-of-statement"));
         assert_expression!(
             "(1 AND 2)OR 3",
             Expression::BinaryExpression(

rusty_parser/src/core/opt_second_expression.rs

@@ -1,11 +1,11 @@
 use rusty_pc::and::TupleCombiner;
-use rusty_pc::{Parser, ParserErrorTrait, SetContext, Token, ctx_parser};
+use rusty_pc::{IifParser, Parser, ParserErrorTrait, SetContext};
 
 use crate::core::expression::ws_expr_pos_p;
 use crate::error::ParserError;
 use crate::input::StringView;
 use crate::pc_specific::keyword;
-use crate::tokens::whitespace;
+use crate::tokens::whitespace_ignoring;
 use crate::{ExpressionPos, Keyword};
 
 /// Parses an optional second expression that follows the first expression
@@ -69,12 +69,11 @@ fn err(keyword: Keyword) -> ParserError {
 /// * `(1 + 2)AND` no whitespace is required before `AND`
 /// * `1 + 2AND` the lack of whitespace before `AND` is an error
 pub(super) fn conditionally_opt_whitespace()
--> impl Parser<StringView, bool, Output = Option<Token>, Error = ParserError> + SetContext<bool> {
-    ctx_parser()
-        .map(|allow_none| {
-            whitespace()
-                .map(Some)
-                .and_then_err(move |err| if allow_none { Ok(None) } else { Err(err) })
-        })
-        .flatten()
+-> impl Parser<StringView, bool, Output = (), Error = ParserError> + SetContext<bool> {
+    IifParser::new(
+        // allow none
+        whitespace_ignoring().to_option().map_to_unit(),
+        // whitespace is required
+        whitespace_ignoring(),
+    )
 }