Fixed aggregation operator

Manage sequence operators, which currently are `Chain` and `Token` on the stack without ever inserting unfinished sequence operator nodes into another node.

Relates to #44

src/context/mod.rs

@@ -103,7 +103,7 @@ impl Context for HashMapContext {
 ///
 /// let ctx = evalexpr::context_map! {
 ///     "x" => 8,
-///     "f" => Function::new(None, Box::new(|_| Ok(42.into()) )) 
+///     "f" => Function::new(None, Box::new(|_| Ok(42.into()) ))
 /// }.unwrap();
 ///
 /// assert_eq!(eval_with_context("x + f()", &ctx), Ok(50.into()));

src/operator/mod.rs

@@ -4,7 +4,7 @@ use crate::{context::Context, error::*, value::Value};
 
 mod display;
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq)]
 pub enum Operator {
     RootNode,
 
@@ -93,9 +93,10 @@ impl Operator {
 
     /// Returns true if chains of this operator should be flattened into one operator with many arguments.
     // Make this a const fn once #57563 is resolved
-    fn is_flatten_chains(&self) -> bool {
+    pub(crate) fn is_sequence(&self) -> bool {
+        use crate::operator::Operator::*;
         match self {
-            Operator::Tuple => true,
+            Tuple | Chain => true,
             _ => false,
         }
     }
@@ -112,8 +113,8 @@ impl Operator {
         use crate::operator::Operator::*;
         match self {
             Add | Sub | Mul | Div | Mod | Exp | Eq | Neq | Gt | Lt | Geq | Leq | And | Or
-            | Assign | Chain => Some(2),
+            | Assign => Some(2),
-            Tuple => None,
+            Tuple | Chain => None,
             Not | Neg | RootNode => Some(1),
             Const { value: _ } => Some(0),
             VariableIdentifier { identifier: _ } => Some(0),
@@ -420,7 +421,9 @@ impl Operator {
                 }
             }
             Tuple => {
-                expect_operator_argument_amount(arguments.len(), 2)?;
+                Ok(Value::Tuple(arguments.into()))
+
+                /*expect_operator_argument_amount(arguments.len(), 2)?;
                 if let Value::Tuple(tuple) = &arguments[0] {
                     let mut tuple = tuple.clone();
                     if let Value::Tuple(tuple2) = &arguments[1] {
@@ -440,7 +443,7 @@ impl Operator {
                             arguments[1].clone(),
                         ]))
                     }
-                }
+                }*/
             }
             Assign => Err(EvalexprError::ContextNotManipulable),
             Chain => {
@@ -448,7 +451,7 @@ impl Operator {
                     return Err(EvalexprError::wrong_operator_argument_amount(0, 1));
                 }
 
-                Ok(arguments.get(1).cloned().unwrap_or(Value::Empty))
+                Ok(arguments.last().cloned().unwrap_or(Value::Empty))
             }
             Const { value } => {
                 expect_operator_argument_amount(arguments.len(), 0)?;

src/tree/mod.rs

@@ -11,8 +11,7 @@ use crate::{
     operator::*,
     value::Value,
 };
-use std::error::Error;
+use std::mem;
-use std::any::Any;
 
 mod display;
 mod iter;
@@ -34,10 +33,10 @@ mod iter;
 /// assert_eq!(node.eval_with_context(&context), Ok(Value::from(3)));
 /// ```
 ///
-#[derive(Debug)]
+#[derive(Debug, PartialEq)]
 pub struct Node {
-    children: Vec<Node>,
     operator: Operator,
+    children: Vec<Node>,
 }
 
 impl Node {
@@ -368,6 +367,7 @@ impl Node {
     }
 
     fn insert_back_prioritized(&mut self, node: Node, is_root_node: bool) -> EvalexprResult<()> {
+        println!("Inserting {:?} into {:?}", node.operator, self.operator());
         if self.operator().precedence() < node.operator().precedence() || is_root_node
             // Right-to-left chaining
             || (self.operator().precedence() == node.operator().precedence() && !self.operator().is_left_to_right() && !node.operator().is_left_to_right())
@@ -381,14 +381,13 @@ impl Node {
                     || (self.children.last().unwrap().operator().precedence()
                     == node.operator().precedence() && !self.children.last().unwrap().operator().is_left_to_right() && !node.operator().is_left_to_right())
                 {
+                    println!("Recursing into {:?}", self.children.last().unwrap().operator());
                     self.children
                         .last_mut()
                         .unwrap()
                         .insert_back_prioritized(node, false)
-                } else if self.children.last().unwrap().operator().type_id() == node.operator().type_id() && node.operator().is_flatten_chains() && !self.children.last().unwrap().has_enough_children() {
-                    // The operators will be chained together, and the next value will be added to this nodes last child.
-                    Ok(())
                 } else {
+                    println!("Rotating");
                     if node.operator().is_leaf() {
                         return Err(EvalexprError::AppendedToLeafNode);
                     }
@@ -401,6 +400,7 @@ impl Node {
                     Ok(())
                 }
             } else {
+                println!("Inserting as specified");
                 self.children.push(node);
                 Ok(())
             }
@@ -410,8 +410,63 @@ impl Node {
     }
 }
 
+fn collapse_root_stack_to(root_stack: &mut Vec<Node>, mut root: Node, collapse_goal: &Node) -> EvalexprResult<Node> {
+    loop {
+        if let Some(mut potential_higher_root) = root_stack.pop() {
+            // TODO I'm not sure about this >, as I have no example for different sequence operators with the same precedence
+            if potential_higher_root.operator().precedence() > collapse_goal.operator().precedence() {
+                potential_higher_root.children.push(root);
+                root = potential_higher_root;
+            } else {
+                root_stack.push(potential_higher_root);
+                break;
+            }
+        } else {
+            // This is the only way the topmost root node could have been removed
+            return Err(EvalexprError::UnmatchedRBrace);
+        }
+    }
+
+    Ok(root)
+}
+
+fn collapse_all_sequences(root_stack: &mut Vec<Node>) -> EvalexprResult<()> {
+    println!("Collapsing all sequences");
+    println!("Initial root stack is: {:?}", root_stack);
+    let mut root = if let Some(root) = root_stack.pop() {
+        root
+    } else {
+        return Err(EvalexprError::UnmatchedRBrace);
+    };
+
+    loop {
+        println!("Root is: {:?}", root);
+        if root.operator() == &Operator::RootNode {
+            root_stack.push(root);
+            break;
+        }
+
+        if let Some(mut potential_higher_root) = root_stack.pop() {
+            if root.operator().is_sequence() {
+                potential_higher_root.children.push(root);
+                root = potential_higher_root;
+            } else {
+                root_stack.push(potential_higher_root);
+                root_stack.push(root);
+                break;
+            }
+        } else {
+            // This is the only way the topmost root node could have been removed
+            return Err(EvalexprError::UnmatchedRBrace);
+        }
+    }
+
+    println!("Root stack after collapsing all sequences is: {:?}", root_stack);
+    Ok(())
+}
+
 pub(crate) fn tokens_to_operator_tree(tokens: Vec<Token>) -> EvalexprResult<Node> {
-    let mut root = vec![Node::root_node()];
+    let mut root_stack = vec![Node::root_node()];
     let mut last_token_is_rightsided_value = false;
     let mut token_iter = tokens.iter().peekable();
 
@@ -443,14 +498,15 @@ pub(crate) fn tokens_to_operator_tree(tokens: Vec<Token>) -> EvalexprResult<Node
             Token::Not => Some(Node::new(Operator::Not)),
 
             Token::LBrace => {
-                root.push(Node::root_node());
+                root_stack.push(Node::root_node());
                 None
             }
             Token::RBrace => {
-                if root.len() < 2 {
+                if root_stack.len() <= 1 {
                     return Err(EvalexprError::UnmatchedRBrace);
                 } else {
-                    root.pop()
+                    collapse_all_sequences(&mut root_stack)?;
+                    root_stack.pop()
                 }
             }
 
@@ -475,9 +531,58 @@ pub(crate) fn tokens_to_operator_tree(tokens: Vec<Token>) -> EvalexprResult<Node
             Token::String(string) => Some(Node::new(Operator::value(Value::String(string)))),
         };
 
-        if let Some(node) = node {
+        if let Some(mut node) = node {
-            if let Some(root) = root.last_mut() {
+            // Need to pop and then repush here, because Rust 1.33.0 cannot release the mutable borrow of root_stack before the end of this complete if-statement
-                root.insert_back_prioritized(node, true)?;
+            if let Some(mut root) = root_stack.pop() {
+                if node.operator().is_sequence() {
+                    println!("Found a sequence operator");
+                    println!("Stack before sequence operation: {:?}, {:?}", root_stack, root);
+                    // If root.operator() and node.operator() are of the same variant, ...
+                    if mem::discriminant(root.operator()) == mem::discriminant(node.operator()) {
+                        // ... we create a new root node for the next expression in the sequence
+                        root.children.push(Node::root_node());
+                        root_stack.push(root);
+                    } else if root.operator() == &Operator::RootNode {
+                        // If the current root is an actual root node, we start a new sequence
+                        node.children.push(root);
+                        node.children.push(Node::root_node());
+                        root_stack.push(Node::root_node());
+                        root_stack.push(node);
+                    } else {
+                        // Otherwise, we combine the sequences based on their precedences
+                        // TODO I'm not sure about this <, as I have no example for different sequence operators with the same precedence
+                        if root.operator().precedence() < node.operator().precedence() {
+                            // If the new sequence has a higher precedence, it is part of the last element of the current root sequence
+                            if let Some(last_root_child) = root.children.pop() {
+                                node.children.push(last_root_child);
+                                node.children.push(Node::root_node());
+                                root_stack.push(root);
+                                root_stack.push(node);
+                            } else {
+                                // Once a sequence has been pushed on top of the stack, it also gets a child
+                                unreachable!()
+                            }
+                        } else {
+                            // If the new sequence doesn't have a higher precedence, then all sequences with a higher precedence are collapsed below this one
+                            root = collapse_root_stack_to(&mut root_stack, root, &node)?;
+                            node.children.push(root);
+                            root_stack.push(node);
+                        }
+                    }
+                    println!("Stack after sequence operation: {:?}", root_stack);
+                } else if root.operator().is_sequence() {
+                    if let Some(mut last_root_child) = root.children.pop() {
+                        last_root_child.insert_back_prioritized(node, true)?;
+                        root.children.push(last_root_child);
+                        root_stack.push(root);
+                    } else {
+                        // Once a sequence has been pushed on top of the stack, it also gets a child
+                        unreachable!()
+                    }
+                } else {
+                    root.insert_back_prioritized(node, true)?;
+                    root_stack.push(root);
+                }
             } else {
                 return Err(EvalexprError::UnmatchedRBrace);
             }
@@ -486,9 +591,12 @@ pub(crate) fn tokens_to_operator_tree(tokens: Vec<Token>) -> EvalexprResult<Node
         last_token_is_rightsided_value = token.is_rightsided_value();
     }
 
-    if root.len() > 1 {
+    // In the end, all sequences are implicitly terminated
+    collapse_all_sequences(&mut root_stack)?;
+
+    if root_stack.len() > 1 {
         Err(EvalexprError::UnmatchedLBrace)
-    } else if let Some(root) = root.pop() {
+    } else if let Some(root) = root_stack.pop() {
         Ok(root)
     } else {
         Err(EvalexprError::UnmatchedRBrace)

src/value/mod.rs

@@ -195,7 +195,9 @@ impl From<Value> for EvalexprResult<Value> {
 }
 
 impl From<()> for Value {
-    fn from(_: ()) -> Self { Value::Empty }
+    fn from(_: ()) -> Self {
+        Value::Empty
+    }
 }
 
 #[cfg(test)]

tests/integration.rs

@@ -251,6 +251,7 @@ fn test_n_ary_functions() {
     context
         .set_value("five".to_string(), Value::Int(5))
         .unwrap();
+    context.set_function("function_four".into(), Function::new(Some(0), Box::new(|_| {Ok(Value::Int(4))}))).unwrap();
 
     assert_eq!(eval_with_context("avg(7, 5)", &context), Ok(Value::Int(6)));
     assert_eq!(
@@ -265,16 +266,20 @@ fn test_n_ary_functions() {
         eval_with_context("sub2 avg(3, 6)", &context),
         Ok(Value::Int(2))
     );
+    dbg!(build_operator_tree("muladd(3, 6, -4)").unwrap());
     assert_eq!(
         eval_with_context("muladd(3, 6, -4)", &context),
         Ok(Value::Int(14))
     );
     assert_eq!(eval_with_context("count()", &context), Ok(Value::Int(0)));
+    assert_eq!(eval_with_context("count((1, 2, 3))", &context), Ok(Value::Int(1)));
     assert_eq!(
         eval_with_context("count(3, 5.5, 2)", &context),
         Ok(Value::Int(3))
     );
     assert_eq!(eval_with_context("count 5", &context), Ok(Value::Int(1)));
+    assert_eq!(eval_with_context("function_four()", &context), Ok(Value::Int(4)));
+    assert_eq!(eval_with_context("function_four(())", &context), Err(EvalexprError::WrongFunctionArgumentAmount{expected: 0, actual: 1}));
 }
 
 #[test]
@@ -610,12 +615,46 @@ fn test_serde() {
 fn test_tuple_definitions() {
     assert_eq!(eval_empty("()"), Ok(()));
     assert_eq!(eval_int("(3)"), Ok(3));
-    assert_eq!(eval_tuple("(3, 4)"), Ok(vec![Value::from(3), Value::from(4)]));
+    assert_eq!(
-    assert_eq!(eval_tuple("2, (5, 6)"), Ok(vec![Value::from(2), Value::from(vec![Value::from(5), Value::from(6)])]));
+        eval_tuple("(3, 4)"),
+        Ok(vec![Value::from(3), Value::from(4)])
+    );
+    assert_eq!(
+        eval_tuple("2, (5, 6)"),
+        Ok(vec![
+            Value::from(2),
+            Value::from(vec![Value::from(5), Value::from(6)])
+        ])
+    );
     assert_eq!(eval_tuple("1, 2"), Ok(vec![Value::from(1), Value::from(2)]));
-    assert_eq!(eval_tuple("1, 2, 3, 4"), Ok(vec![Value::from(1), Value::from(2), Value::from(3), Value::from(4)]));
+    assert_eq!(
-    assert_eq!(eval_tuple("(1, 2, 3), 5, 6, (true, false, 0)"), Ok(vec![Value::from(vec![Value::from(1), Value::from(2), Value::from(3)]), Value::from(5), Value::from(6), Value::from(vec![Value::from(true), Value::from(false), Value::from(0)])]));
+        eval_tuple("1, 2, 3, 4"),
-    assert_eq!(eval_tuple("1, (2)"), Ok(vec![Value::from(1), Value::from(2)]));
+        Ok(vec![
-    assert_eq!(eval_tuple("1, ()"), Ok(vec![Value::from(1), Value::from(())]));
+            Value::from(1),
-    assert_eq!(eval_tuple("1, ((2))"), Ok(vec![Value::from(1), Value::from(2)]));
+            Value::from(2),
+            Value::from(3),
+            Value::from(4)
+        ])
+    );
+    assert_eq!(
+        eval_tuple("(1, 2, 3), 5, 6, (true, false, 0)"),
+        Ok(vec![
+            Value::from(vec![Value::from(1), Value::from(2), Value::from(3)]),
+            Value::from(5),
+            Value::from(6),
+            Value::from(vec![Value::from(true), Value::from(false), Value::from(0)])
+        ])
+    );
+    assert_eq!(
+        eval_tuple("1, (2)"),
+        Ok(vec![Value::from(1), Value::from(2)])
+    );
+    assert_eq!(
+        eval_tuple("1, ()"),
+        Ok(vec![Value::from(1), Value::from(())])
+    );
+    assert_eq!(
+        eval_tuple("1, ((2))"),
+        Ok(vec![Value::from(1), Value::from(2)])
+    );
 }