From 65fadfd1648fbc8e4e5dd3734bc62ad35f94e07d Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Mon, 4 Jul 2022 16:50:39 +0300 Subject: [PATCH 1/4] Add tests for left-hand side of assignment being an identifier. Relates to #106 --- src/token/mod.rs | 15 ++++++++++++++- tests/integration.rs | 25 +++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/token/mod.rs b/src/token/mod.rs index 7b9d504..3bb49cd 100644 --- a/src/token/mod.rs +++ b/src/token/mod.rs @@ -444,7 +444,7 @@ pub(crate) fn tokenize(string: &str) -> EvalexprResult> { #[cfg(test)] mod tests { - use crate::token::{char_to_partial_token, tokenize}; + use crate::token::{char_to_partial_token, tokenize, Token}; use std::fmt::Write; #[test] @@ -474,4 +474,17 @@ mod tests { assert_eq!(token_string, result_string); } + + #[test] + fn assignment_lhs_is_identifier() { + let tokens = tokenize("a = 1").unwrap(); + assert_eq!( + tokens.as_slice(), + [ + Token::Identifier("a".to_string()), + Token::Assign, + Token::Int(1) + ] + ); + } } diff --git a/tests/integration.rs b/tests/integration.rs index 6a096d3..30f20d4 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -2083,3 +2083,28 @@ fn test_try_from() { ); assert_eq!(EmptyType::try_from(value.clone()), Ok(())); } + +#[test] +fn assignment_lhs_is_identifier() { + let tree = build_operator_tree("a = 1").unwrap(); + let operators: Vec<_> = tree.iter().map(|node| node.operator().clone()).collect(); + + let mut context = HashMapContext::new(); + tree.eval_empty_with_context_mut(&mut context).unwrap(); + assert_eq!(context.get_value("a"), Some(&Value::Int(1))); + + assert!( + matches!( + operators.as_slice(), + [ + Operator::Assign, + Operator::VariableIdentifier { identifier: value }, + Operator::Const { + value: Value::Int(1) + } + ] if value == "a" + ), + "actual: {:#?}", + operators + ); +} From 54c428667240042ac95e56f0ea47d17fda81e990 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Mon, 4 Jul 2022 17:03:32 +0300 Subject: [PATCH 2/4] Use double dot syntax in `operator/mod.rs`. --- src/operator/mod.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/operator/mod.rs b/src/operator/mod.rs index 3ebf4a1..a8ae045 100644 --- a/src/operator/mod.rs +++ b/src/operator/mod.rs @@ -123,9 +123,9 @@ impl Operator { Tuple => 40, Chain => 0, - Const { value: _ } => 200, - VariableIdentifier { identifier: _ } => 200, - FunctionIdentifier { identifier: _ } => 190, + Const { .. } => 200, + VariableIdentifier { .. } => 200, + FunctionIdentifier { .. } => 190, } } @@ -134,7 +134,7 @@ impl Operator { /// Left-to-right chaining has priority if operators with different order but same precedence are chained. pub(crate) const fn is_left_to_right(&self) -> bool { use crate::operator::Operator::*; - !matches!(self, Assign | FunctionIdentifier { identifier: _ }) + !matches!(self, Assign | FunctionIdentifier { .. }) } /// Returns true if chains of this operator should be flattened into one operator with many arguments. @@ -158,9 +158,9 @@ impl Operator { | AndAssign | OrAssign => Some(2), Tuple | Chain => None, Not | Neg | RootNode => Some(1), - Const { value: _ } => Some(0), - VariableIdentifier { identifier: _ } => Some(0), - FunctionIdentifier { identifier: _ } => Some(1), + Const { .. } => Some(0), + VariableIdentifier { .. } => Some(0), + FunctionIdentifier { .. } => Some(1), } } From e70e9d366ab33a0bb2d30f41b607f9711aa7c0bd Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Mon, 4 Jul 2022 17:10:28 +0300 Subject: [PATCH 3/4] Use explicit operator type for reading and writing variables. Before, variables written to were represented with the same operator as constants in the AST. This was hacky and confusing to some users. Closes #106 --- src/operator/display.rs | 4 +++- src/operator/mod.rs | 30 ++++++++++++++++++++++-------- src/tree/mod.rs | 14 ++++++++++---- tests/integration.rs | 4 ++-- 4 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/operator/display.rs b/src/operator/display.rs index d4291e7..12351dd 100644 --- a/src/operator/display.rs +++ b/src/operator/display.rs @@ -41,7 +41,9 @@ impl Display for Operator { Chain => write!(f, "; "), Const { value } => write!(f, "{}", value), - VariableIdentifier { identifier } => write!(f, "{}", identifier), + VariableIdentifierWrite { identifier } | VariableIdentifierRead { identifier } => { + write!(f, "{}", identifier) + }, FunctionIdentifier { identifier } => write!(f, "{}", identifier), } } diff --git a/src/operator/mod.rs b/src/operator/mod.rs index a8ae045..5f2be51 100644 --- a/src/operator/mod.rs +++ b/src/operator/mod.rs @@ -75,8 +75,13 @@ pub enum Operator { /** The value of the constant. */ value: Value, }, - /// A variable identifier. - VariableIdentifier { + /// A write to a variable identifier. + VariableIdentifierWrite { + /// The identifier of the variable. + identifier: String, + }, + /// A read from a variable identifier. + VariableIdentifierRead { /// The identifier of the variable. identifier: String, }, @@ -92,8 +97,12 @@ impl Operator { Operator::Const { value } } - pub(crate) fn variable_identifier(identifier: String) -> Self { - Operator::VariableIdentifier { identifier } + pub(crate) fn variable_identifier_write(identifier: String) -> Self { + Operator::VariableIdentifierWrite { identifier } + } + + pub(crate) fn variable_identifier_read(identifier: String) -> Self { + Operator::VariableIdentifierRead { identifier } } pub(crate) fn function_identifier(identifier: String) -> Self { @@ -124,7 +133,7 @@ impl Operator { Chain => 0, Const { .. } => 200, - VariableIdentifier { .. } => 200, + VariableIdentifierWrite { .. } | VariableIdentifierRead { .. } => 200, FunctionIdentifier { .. } => 190, } } @@ -159,7 +168,7 @@ impl Operator { Tuple | Chain => None, Not | Neg | RootNode => Some(1), Const { .. } => Some(0), - VariableIdentifier { .. } => Some(0), + VariableIdentifierWrite { .. } | VariableIdentifierRead { .. } => Some(0), FunctionIdentifier { .. } => Some(1), } } @@ -422,7 +431,12 @@ impl Operator { Ok(value.clone()) }, - VariableIdentifier { identifier } => { + VariableIdentifierWrite { identifier } => { + expect_operator_argument_amount(arguments.len(), 0)?; + + Ok(identifier.clone().into()) + }, + VariableIdentifierRead { identifier } => { expect_operator_argument_amount(arguments.len(), 0)?; if let Some(value) = context.get_value(identifier).cloned() { @@ -473,7 +487,7 @@ impl Operator { expect_operator_argument_amount(arguments.len(), 2)?; let target = arguments[0].as_string()?; - let left_value = Operator::VariableIdentifier { + let left_value = Operator::VariableIdentifierRead { identifier: target.clone(), } .eval(&Vec::new(), context)?; diff --git a/src/tree/mod.rs b/src/tree/mod.rs index 69f4494..1956e56 100644 --- a/src/tree/mod.rs +++ b/src/tree/mod.rs @@ -69,7 +69,8 @@ impl Node { /// ``` pub fn iter_identifiers(&self) -> impl Iterator { self.iter().filter_map(|node| match node.operator() { - Operator::VariableIdentifier { identifier } + Operator::VariableIdentifierWrite { identifier } + | Operator::VariableIdentifierRead { identifier } | Operator::FunctionIdentifier { identifier } => Some(identifier.as_str()), _ => None, }) @@ -92,7 +93,8 @@ impl Node { /// ``` pub fn iter_variable_identifiers(&self) -> impl Iterator { self.iter().filter_map(|node| match node.operator() { - Operator::VariableIdentifier { identifier } => Some(identifier.as_str()), + Operator::VariableIdentifierWrite { identifier } + | Operator::VariableIdentifierRead { identifier } => Some(identifier.as_str()), _ => None, }) } @@ -616,10 +618,14 @@ pub(crate) fn tokens_to_operator_tree(tokens: Vec) -> EvalexprResult Some(Node::new(Operator::Chain)), Token::Identifier(identifier) => { - let mut result = Some(Node::new(Operator::variable_identifier(identifier.clone()))); + let mut result = Some(Node::new(Operator::variable_identifier_read( + identifier.clone(), + ))); if let Some(next) = next { if next.is_assignment() { - result = Some(Node::new(Operator::value(identifier.clone().into()))); + result = Some(Node::new(Operator::variable_identifier_write( + identifier.clone(), + ))); } else if next.is_leftsided_value() { result = Some(Node::new(Operator::function_identifier(identifier))); } diff --git a/tests/integration.rs b/tests/integration.rs index 30f20d4..9524da3 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -2090,7 +2090,7 @@ fn assignment_lhs_is_identifier() { let operators: Vec<_> = tree.iter().map(|node| node.operator().clone()).collect(); let mut context = HashMapContext::new(); - tree.eval_empty_with_context_mut(&mut context).unwrap(); + tree.eval_with_context_mut(&mut context).unwrap(); assert_eq!(context.get_value("a"), Some(&Value::Int(1))); assert!( @@ -2098,7 +2098,7 @@ fn assignment_lhs_is_identifier() { operators.as_slice(), [ Operator::Assign, - Operator::VariableIdentifier { identifier: value }, + Operator::VariableIdentifierWrite { identifier: value }, Operator::Const { value: Value::Int(1) } From 7e3fdef92e32bc29b3016ed84db516ca634c1c87 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 6 Jul 2022 11:14:29 +0300 Subject: [PATCH 4/4] Update changelog. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25d69a5..02c2106 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * Builtin functions to check for nan, infinity and subnormality in floats (#101) * Builtin random function (#102) * Implement `TryFrom` for all types a value can hold (#105) + * Split VariableIdentifier node into read and write variants (#106) ### Removed @@ -24,6 +25,7 @@ My warmhearted thanks goes to: * [Ophir LOJKINE](https://github.com/lovasoa) * [Joe Grund](https://github.com/jgrund) + * [Luka Maljic](https://github.com/malj) ## [7.2.0](https://github.com/ISibboI/evalexpr/compare/7.1.1...7.2.0) - 2022-03-16