From 2ffc88a22e18ae399cd3676cd30a754197ee3464 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 13 Jan 2022 13:52:51 +0200 Subject: [PATCH 1/6] Add failing examples from issue. Relates to #94 --- tests/integration.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/integration.rs b/tests/integration.rs index 9d536c2..d6697b2 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -1806,3 +1806,14 @@ fn test_value_type() { Ok(Value::String(String::new())) ); } + +#[test] +fn test_parenthese_combinations() { + // These are from issue #94 + dbg!(build_operator_tree("123(1*2)").unwrap()); + assert!(dbg!(eval("123(1*2)")).is_err()); + assert!(dbg!(eval("1()")).is_err()); + assert!(dbg!(eval("1()()()()")).is_err()); + assert!(dbg!(eval("1()()()(9)()()")).is_err()); + assert!(dbg!(eval_with_context("a+100+(a*2)", &context_map! {"a" => 4}.unwrap())).is_err()); +} From 4e5e218d3e43d554c3e7cf6ed78c435006ab119b Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 13 Jan 2022 14:50:38 +0200 Subject: [PATCH 2/6] Fix #94. Introduce a new error type for illegal parenthese expressions such as `4(5)`. --- CHANGELOG.md | 4 ++++ src/error/display.rs | 6 ++++++ src/error/mod.rs | 5 +++++ src/token/mod.rs | 2 +- src/tree/mod.rs | 48 +++++++++++++++++++++++++++++++++++++++++--- tests/integration.rs | 28 ++++++++++++++++++++------ 6 files changed, 83 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dffdde0..ea439fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,12 @@ ### Changed + * Made the `EvalexprError` enum non_exhaustive. + ### Fixed + * Expressions that have dangling parenthese expressions such as `4(5)` now produce an error. + ### Deprecated ### Contributors diff --git a/src/error/display.rs b/src/error/display.rs index 3de998f..61e53b9 100644 --- a/src/error/display.rs +++ b/src/error/display.rs @@ -69,6 +69,12 @@ impl fmt::Display for EvalexprError { ), UnmatchedLBrace => write!(f, "Found an unmatched opening 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 } => { if let Some(second) = second { write!( diff --git a/src/error/mod.rs b/src/error/mod.rs index 73f8d47..46cd116 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -15,6 +15,7 @@ mod display; /// Errors used in this crate. #[derive(Debug, PartialEq)] +#[non_exhaustive] pub enum EvalexprError { /// An operator was called with a wrong amount of arguments. WrongOperatorArgumentAmount { @@ -128,6 +129,10 @@ pub enum EvalexprError { /// A closing brace without a matching opening brace was found. 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`. /// 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. diff --git a/src/token/mod.rs b/src/token/mod.rs index 2dd0f22..0cde4e4 100644 --- a/src/token/mod.rs +++ b/src/token/mod.rs @@ -372,7 +372,7 @@ fn partial_tokens_to_tokens(mut tokens: &[PartialToken]) -> EvalexprResult Some(Token::Identifier(literal.to_string())), } } diff --git a/src/tree/mod.rs b/src/tree/mod.rs index d173ad5..69f4494 100644 --- a/src/tree/mod.rs +++ b/src/tree/mod.rs @@ -406,6 +406,14 @@ impl Node { 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<()> { // println!("Inserting {:?} into {:?}", node.operator, self.operator()); if self.operator().precedence() < node.operator().precedence() || is_root_node @@ -415,7 +423,7 @@ impl Node { if self.operator().is_leaf() { Err(EvalexprError::AppendedToLeafNode) } 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(); 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()) { // 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 .last_mut() .unwrap() @@ -436,11 +444,35 @@ impl Node { 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(); + // 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); 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); Ok(()) } @@ -492,6 +524,11 @@ fn collapse_all_sequences(root_stack: &mut Vec) -> EvalexprResult<()> { loop { // println!("Root is: {:?}", root); 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); break; } @@ -501,6 +538,11 @@ fn collapse_all_sequences(root_stack: &mut Vec) -> EvalexprResult<()> { potential_higher_root.children.push(root); root = potential_higher_root; } 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(root); break; diff --git a/tests/integration.rs b/tests/integration.rs index d6697b2..5bf6c24 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -1810,10 +1810,26 @@ fn test_value_type() { #[test] fn test_parenthese_combinations() { // These are from issue #94 - dbg!(build_operator_tree("123(1*2)").unwrap()); - assert!(dbg!(eval("123(1*2)")).is_err()); - assert!(dbg!(eval("1()")).is_err()); - assert!(dbg!(eval("1()()()()")).is_err()); - assert!(dbg!(eval("1()()()(9)()()")).is_err()); - assert!(dbg!(eval_with_context("a+100+(a*2)", &context_map! {"a" => 4}.unwrap())).is_err()); + assert_eq!( + eval("123(1*2)"), + Err(EvalexprError::MissingOperatorOutsideOfBrace) + ); + assert_eq!( + 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)); } From 7af8da5cdb3c7421f7ce62ada4c79a4bbb598d4c Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 13 Jan 2022 14:54:34 +0200 Subject: [PATCH 3/6] Prepare 7.0.0 release. --- CHANGELOG.md | 16 ++++++++++++++-- src/lib.rs | 2 +- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea439fc..10f59b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,16 @@ ### Removed +### Changed + +### Fixed + +### Deprecated + +### Contributors + +## [7.0.0](https://github.com/ISibboI/evalexpr/compare/6.6.0...7.0.0) - 2022-01-13 + ### Changed * Made the `EvalexprError` enum non_exhaustive. @@ -16,10 +26,12 @@ * Expressions that have dangling parenthese expressions such as `4(5)` now produce an error. -### Deprecated - ### Contributors +My warmhearted thanks goes to + + * [dbr/Ben](https://github.com/dbr) + ## [6.6.0](https://github.com/ISibboI/evalexpr/compare/6.5.0...6.6.0) - 2021-10-13 ### Added diff --git a/src/lib.rs b/src/lib.rs index 187c101..f931cfa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,7 +5,7 @@ //! //! ```toml //! [dependencies] -//! evalexpr = "6" +//! evalexpr = "7" //! ``` //! //! Then you can use `evalexpr` to **evaluate expressions** like this: From fcbf284d1810677ecf335bdd31de7f6c675b95a9 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 13 Jan 2022 15:02:02 +0200 Subject: [PATCH 4/6] Release 7.0.0 evalexpr@7.0.0 Generated by cargo-workspaces --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 878aecb..ac1ac17 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "evalexpr" -version = "6.6.0" +version = "7.0.0" description = "A powerful arithmetic and boolean expression evaluator" keywords = ["expression", "evaluate", "evaluator", "arithmetic", "boolean"] categories = ["parsing", "game-engines"] From 79cb25bc9dcb70fbb1e339c8d58bee7fef4018cf Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 13 Jan 2022 15:18:06 +0200 Subject: [PATCH 5/6] Increase test coverage. --- src/error/mod.rs | 33 +++++++++++++++++++++++++++++++++ tests/integration.rs | 2 ++ 2 files changed, 35 insertions(+) diff --git a/src/error/mod.rs b/src/error/mod.rs index 46cd116..a4b2d44 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -366,3 +366,36 @@ impl std::error::Error for EvalexprError {} /// Standard result type used by this crate. pub type EvalexprResult = Result; + +#[cfg(test)] +mod tests { + use crate::{EvalexprError, Value, ValueType}; + + /// Tests whose only use is to bring test coverage of trivial lines up, like trivial constructors. + #[test] + fn trivial_coverage_tests() { + assert_eq!( + EvalexprError::type_error(Value::Int(3), vec![ValueType::String]), + EvalexprError::TypeError { + actual: Value::Int(3), + expected: vec![ValueType::String] + } + ); + assert_eq!( + EvalexprError::expected_type(&Value::String("abc".to_string()), Value::Empty), + EvalexprError::expected_string(Value::Empty) + ); + assert_eq!( + EvalexprError::expected_type(&Value::Boolean(false), Value::Empty), + EvalexprError::expected_boolean(Value::Empty) + ); + assert_eq!( + EvalexprError::expected_type(&Value::Tuple(vec![]), Value::Empty), + EvalexprError::expected_tuple(Value::Empty) + ); + assert_eq!( + EvalexprError::expected_type(&Value::Empty, Value::String("abc".to_string())), + EvalexprError::expected_empty(Value::String("abc".to_string())) + ); + } +} diff --git a/tests/integration.rs b/tests/integration.rs index 5bf6c24..a6f244c 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -582,6 +582,7 @@ fn test_shortcut_functions() { eval_number("abc"), Err(EvalexprError::VariableIdentifierNotFound("abc".to_owned())) ); + assert_eq!(eval_number_with_context("3.5", &context), Ok(3.5)); assert_eq!(eval_number_with_context("3", &context), Ok(3.0)); assert_eq!( eval_number_with_context("true", &context), @@ -593,6 +594,7 @@ fn test_shortcut_functions() { eval_number_with_context("abc", &context), Err(EvalexprError::VariableIdentifierNotFound("abc".to_owned())) ); + assert_eq!(eval_number_with_context_mut("3.5", &mut context), Ok(3.5)); assert_eq!(eval_number_with_context_mut("3", &mut context), Ok(3.0)); assert_eq!( eval_number_with_context_mut("true", &mut context), From 98a7ba23c3037d59a31062efb3fcfca041dc3214 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 13 Jan 2022 16:38:19 +0200 Subject: [PATCH 6/6] Exclude benches from coverage. --- benches/benchs.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/benches/benchs.rs b/benches/benchs.rs index a1d29c4..d86920d 100644 --- a/benches/benchs.rs +++ b/benches/benchs.rs @@ -1,5 +1,6 @@ #![feature(test)] #![feature(bench_black_box)] +#![cfg(not(tarpaulin_include))] extern crate rand; extern crate rand_pcg;