Introduce a new error type for illegal parenthese expressions such as `4(5)`.
This commit is contained in:
Sebastian Schmidt 2022-01-13 14:50:38 +02:00
parent 2ffc88a22e
commit 4e5e218d3e
6 changed files with 83 additions and 10 deletions

View File

@ -10,8 +10,12 @@
### Changed ### Changed
* Made the `EvalexprError` enum non_exhaustive.
### Fixed ### Fixed
* Expressions that have dangling parenthese expressions such as `4(5)` now produce an error.
### Deprecated ### Deprecated
### Contributors ### Contributors

View File

@ -69,6 +69,12 @@ impl fmt::Display for EvalexprError {
), ),
UnmatchedLBrace => write!(f, "Found an unmatched opening parenthesis '('."), UnmatchedLBrace => write!(f, "Found an unmatched opening parenthesis '('."),
UnmatchedRBrace => write!(f, "Found an unmatched closing parenthesis ')'."), UnmatchedRBrace => write!(f, "Found an unmatched closing parenthesis ')'."),
MissingOperatorOutsideOfBrace { .. } => write!(
f,
"Found an opening parenthesis that is preceded by something that does not take \
any arguments on the right, or found a closing parenthesis that is succeeded by \
something that does not take any arguments on the left."
),
UnmatchedPartialToken { first, second } => { UnmatchedPartialToken { first, second } => {
if let Some(second) = second { if let Some(second) = second {
write!( write!(

View File

@ -15,6 +15,7 @@ mod display;
/// Errors used in this crate. /// Errors used in this crate.
#[derive(Debug, PartialEq)] #[derive(Debug, PartialEq)]
#[non_exhaustive]
pub enum EvalexprError { pub enum EvalexprError {
/// An operator was called with a wrong amount of arguments. /// An operator was called with a wrong amount of arguments.
WrongOperatorArgumentAmount { WrongOperatorArgumentAmount {
@ -128,6 +129,10 @@ pub enum EvalexprError {
/// A closing brace without a matching opening brace was found. /// A closing brace without a matching opening brace was found.
UnmatchedRBrace, UnmatchedRBrace,
/// Left of an opening brace or right of a closing brace is a token that does not expect the brace next to it.
/// For example, writing `4(5)` would yield this error, as the `4` does not have any operands.
MissingOperatorOutsideOfBrace,
/// A `PartialToken` is unmatched, such that it cannot be combined into a full `Token`. /// A `PartialToken` is unmatched, such that it cannot be combined into a full `Token`.
/// This happens if for example a single `=` is found, surrounded by whitespace. /// This happens if for example a single `=` is found, surrounded by whitespace.
/// It is not a token, but it is part of the string representation of some tokens. /// It is not a token, but it is part of the string representation of some tokens.

View File

@ -372,7 +372,7 @@ fn partial_tokens_to_tokens(mut tokens: &[PartialToken]) -> EvalexprResult<Vec<T
} else { } else {
Some(Token::Identifier(literal.to_string())) Some(Token::Identifier(literal.to_string()))
} }
} },
_ => Some(Token::Identifier(literal.to_string())), _ => Some(Token::Identifier(literal.to_string())),
} }
} }

View File

@ -406,6 +406,14 @@ impl Node {
Some(self.children().len()) == self.operator().max_argument_amount() Some(self.children().len()) == self.operator().max_argument_amount()
} }
fn has_too_many_children(&self) -> bool {
if let Some(max_argument_amount) = self.operator().max_argument_amount() {
self.children().len() > max_argument_amount
} else {
false
}
}
fn insert_back_prioritized(&mut self, node: Node, is_root_node: bool) -> EvalexprResult<()> { fn insert_back_prioritized(&mut self, node: Node, is_root_node: bool) -> EvalexprResult<()> {
// println!("Inserting {:?} into {:?}", node.operator, self.operator()); // println!("Inserting {:?} into {:?}", node.operator, self.operator());
if self.operator().precedence() < node.operator().precedence() || is_root_node if self.operator().precedence() < node.operator().precedence() || is_root_node
@ -415,7 +423,7 @@ impl Node {
if self.operator().is_leaf() { if self.operator().is_leaf() {
Err(EvalexprError::AppendedToLeafNode) Err(EvalexprError::AppendedToLeafNode)
} else if self.has_enough_children() { } else if self.has_enough_children() {
// Unwrap cannot fail because of has_enough_children // Unwrap cannot fail because is_leaf being false and has_enough_children being true implies that the operator wants and has at least one child
let last_child_operator = self.children.last().unwrap().operator(); let last_child_operator = self.children.last().unwrap().operator();
if last_child_operator.precedence() if last_child_operator.precedence()
@ -425,7 +433,7 @@ impl Node {
== node.operator().precedence() && !last_child_operator.is_left_to_right() && !node.operator().is_left_to_right()) == node.operator().precedence() && !last_child_operator.is_left_to_right() && !node.operator().is_left_to_right())
{ {
// println!("Recursing into {:?}", self.children.last().unwrap().operator()); // println!("Recursing into {:?}", self.children.last().unwrap().operator());
// Unwrap cannot fail because of has_enough_children // Unwrap cannot fail because is_leaf being false and has_enough_children being true implies that the operator wants and has at least one child
self.children self.children
.last_mut() .last_mut()
.unwrap() .unwrap()
@ -436,11 +444,35 @@ impl Node {
return Err(EvalexprError::AppendedToLeafNode); return Err(EvalexprError::AppendedToLeafNode);
} }
// Unwrap cannot fail because of has_enough_children // Unwrap cannot fail because is_leaf being false and has_enough_children being true implies that the operator wants and has at least one child
let last_child = self.children.pop().unwrap(); let last_child = self.children.pop().unwrap();
// Root nodes have at most one child
// TODO I am not sure if this is the correct error
if self.operator() == &Operator::RootNode && !self.children().is_empty() {
return Err(EvalexprError::MissingOperatorOutsideOfBrace);
}
// Do not insert root nodes into root nodes.
// TODO I am not sure if this is the correct error
if self.operator() == &Operator::RootNode
&& node.operator() == &Operator::RootNode
{
return Err(EvalexprError::MissingOperatorOutsideOfBrace);
}
self.children.push(node); self.children.push(node);
let node = self.children.last_mut().unwrap(); let node = self.children.last_mut().unwrap();
// Root nodes have at most one child
// TODO I am not sure if this is the correct error
if node.operator() == &Operator::RootNode && !node.children().is_empty() {
return Err(EvalexprError::MissingOperatorOutsideOfBrace);
}
// Do not insert root nodes into root nodes.
// TODO I am not sure if this is the correct error
if node.operator() == &Operator::RootNode
&& last_child.operator() == &Operator::RootNode
{
return Err(EvalexprError::MissingOperatorOutsideOfBrace);
}
node.children.push(last_child); node.children.push(last_child);
Ok(()) Ok(())
} }
@ -492,6 +524,11 @@ fn collapse_all_sequences(root_stack: &mut Vec<Node>) -> EvalexprResult<()> {
loop { loop {
// println!("Root is: {:?}", root); // println!("Root is: {:?}", root);
if root.operator() == &Operator::RootNode { if root.operator() == &Operator::RootNode {
// This should fire if parsing something like `4(5)`
if root.has_too_many_children() {
return Err(EvalexprError::MissingOperatorOutsideOfBrace);
}
root_stack.push(root); root_stack.push(root);
break; break;
} }
@ -501,6 +538,11 @@ fn collapse_all_sequences(root_stack: &mut Vec<Node>) -> EvalexprResult<()> {
potential_higher_root.children.push(root); potential_higher_root.children.push(root);
root = potential_higher_root; root = potential_higher_root;
} else { } else {
// This should fire if parsing something like `4(5)`
if root.has_too_many_children() {
return Err(EvalexprError::MissingOperatorOutsideOfBrace);
}
root_stack.push(potential_higher_root); root_stack.push(potential_higher_root);
root_stack.push(root); root_stack.push(root);
break; break;

View File

@ -1810,10 +1810,26 @@ fn test_value_type() {
#[test] #[test]
fn test_parenthese_combinations() { fn test_parenthese_combinations() {
// These are from issue #94 // These are from issue #94
dbg!(build_operator_tree("123(1*2)").unwrap()); assert_eq!(
assert!(dbg!(eval("123(1*2)")).is_err()); eval("123(1*2)"),
assert!(dbg!(eval("1()")).is_err()); Err(EvalexprError::MissingOperatorOutsideOfBrace)
assert!(dbg!(eval("1()()()()")).is_err()); );
assert!(dbg!(eval("1()()()(9)()()")).is_err()); assert_eq!(
assert!(dbg!(eval_with_context("a+100+(a*2)", &context_map! {"a" => 4}.unwrap())).is_err()); eval("1()"),
Err(EvalexprError::MissingOperatorOutsideOfBrace)
);
assert_eq!(
eval("1()()()()"),
Err(EvalexprError::MissingOperatorOutsideOfBrace)
);
assert_eq!(
eval("1()()()(9)()()"),
Err(EvalexprError::MissingOperatorOutsideOfBrace)
);
assert_eq!(
eval_with_context("a+100(a*2)", &context_map! {"a" => 4}.unwrap()),
Err(EvalexprError::MissingOperatorOutsideOfBrace)
);
assert_eq!(eval_int("(((1+2)*(3+4)+(5-(6)))/((7-8)))"), Ok(-20));
assert_eq!(eval_int("(((((5)))))"), Ok(5));
} }