Merge pull request #107 from ISibboI/sebschmi/i106_identifier_is_variable_bug

Split VariableIdentifier node into read and write variants

CHANGELOG.md

@@ -9,6 +9,7 @@
  * Builtin functions to check for nan, infinity and subnormality in floats (#101)
  * Builtin random function (#102)
  * Implement `TryFrom<Value>` 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
 

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),
         }
     }

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 {
@@ -123,9 +132,9 @@ impl Operator {
             Tuple => 40,
             Chain => 0,
 
-            Const { value: _ } => 200,
-            VariableIdentifier { identifier: _ } => 200,
-            FunctionIdentifier { identifier: _ } => 190,
+            Const { .. } => 200,
+            VariableIdentifierWrite { .. } | VariableIdentifierRead { .. } => 200,
+            FunctionIdentifier { .. } => 190,
         }
     }
 
@@ -134,7 +143,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 +167,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),
+            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)?;

src/token/mod.rs

@@ -444,7 +444,7 @@ pub(crate) fn tokenize(string: &str) -> EvalexprResult<Vec<Token>> {
 
 #[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)
+            ]
+        );
+    }
 }

src/tree/mod.rs

@@ -69,7 +69,8 @@ impl Node {
     /// ```
     pub fn iter_identifiers(&self) -> impl Iterator<Item = &str> {
         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<Item = &str> {
         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<Token>) -> EvalexprResult<Node
             Token::Semicolon => 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)));
                     }

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_with_context_mut(&mut context).unwrap();
+    assert_eq!(context.get_value("a"), Some(&Value::Int(1)));
+
+    assert!(
+        matches!(
+            operators.as_slice(),
+            [
+                Operator::Assign,
+                Operator::VariableIdentifierWrite { identifier: value },
+                Operator::Const {
+                    value: Value::Int(1)
+                }
+            ] if value == "a"
+        ),
+        "actual: {:#?}",
+        operators
+    );
+}