Use transcation with for loops

Author: dolphinau

src/ps/forward.rs

@@ -72,6 +72,7 @@ impl<'a> RuleMut<'a> for Forward {
             | "command_name_expr"
             | "expression_with_unary_operator"
             | "while_condition"
+            | "for_condition"
             | "member_name" => {
                 if view.child_count() == 1 {
                     if let Some(child_data) = view.child(0).ok_or(Error::invalid_child())?.data() {

src/ps/loops.rs

@@ -1,6 +1,5 @@
 use crate::{
     ps::{
-        comparison::infer_comparison,
         LoopStatus::{Dead, Inifite, OneTurn},
         Powershell::{self, Loop, Raw},
         Value::{self, Bool},
@@ -33,15 +32,19 @@ impl IteratorVariable {
 /// use minusone::ps::build_powershell_tree;
 /// use minusone::ps::linter::Linter;
 /// use minusone::ps::forward::Forward;
+/// use minusone::ps::bool::Comparison;
 /// use minusone::ps::integer::ParseInt;
 /// use minusone::ps::integer::AddInt;
+/// use minusone::ps::var::Var;
 /// use minusone::ps::loops::ForStatementCondition;
 ///
 /// let mut tree = build_powershell_tree("for ($i = 132 + 324 - 3; $i -lt 200 - 190; $i++) {echo $i}").unwrap();
 /// tree.apply_mut(&mut (
 ///     ParseInt::default(),
 ///     AddInt::default(),
+///     Comparison::default(),
 ///     Forward::default(),
+///     Var::default(),
 ///     ForStatementCondition::default(),
 /// )).unwrap();
 ///
@@ -71,48 +74,7 @@ impl<'a> RuleMut<'a> for ForStatementCondition {
     ) -> crate::error::MinusOneResult<()> {
         let view = node.view();
         match view.kind() {
-            "for_statement" => {
-                if let (Some(initialisation), Some(comparison)) = (
-                    view.named_child("for_initializer")
-                        .map(|n| n.smallest_child()),
-                    view.named_child("for_condition")
-                        .map(|n| n.smallest_child()),
-                ) {
-                    if comparison.kind() == "comparison_expression"
-                        && initialisation.kind() == "assignment_expression"
-                    {
-                        if let (
-                            Some(var_name),
-                            Some(value),
-                            Some(comp_left),
-                            Some(operator),
-                            Some(comp_right),
-                        ) = (
-                            initialisation
-                                .child(0)
-                                .and_then(|n| Some(n.text().ok()?.to_lowercase())),
-                            initialisation.child(2),
-                            comparison.child(0),
-                            comparison.child(1),
-                            comparison.child(2),
-                        ) {
-                            if let Some(false) =
-                                infer_comparison(&comp_left, &operator, &comp_right)
-                                    .or((comp_left.text().unwrap_or_default().to_lowercase()
-                                        == var_name)
-                                        .then_some(1)
-                                        .and(infer_comparison(&value, &operator, &comp_right)))
-                                    .or((comp_right.text().unwrap_or_default() == var_name)
-                                        .then_some(1)
-                                        .and(infer_comparison(&comp_left, &operator, &value)))
-                            {
-                                node.set(Loop(Dead));
-                            }
-                        }
-                    }
-                }
-            }
-            "while_condition" => {
+            "while_condition" | "for_condition" => {
                 if let Some(&Raw(Bool(false))) = view.data() {
                     node.set_by_node_id(view.parent().unwrap().id(), Loop(Dead));
                     node.apply_transaction();
@@ -255,10 +217,12 @@ impl<'a> RuleMut<'a> for ForStatementFlowControl {
 
 #[cfg(test)]
 mod test {
+    use crate::ps::bool::Comparison;
     use crate::ps::build_powershell_tree;
     use crate::ps::forward::Forward;
     use crate::ps::integer::ParseInt;
     use crate::ps::loops::{ForStatementCondition, ForStatementFlowControl};
+    use crate::ps::var::Var;
     use crate::ps::LoopStatus::{Dead, OneTurn};
     use crate::ps::Powershell::{Loop, Raw};
     use crate::ps::Value::Num;
@@ -267,8 +231,10 @@ mod test {
     fn test_dead_for_statement() {
         let mut tree = build_powershell_tree("for ($i = 0; $i -gt 1; $i++) {}").unwrap();
         tree.apply_mut(&mut (
-            ParseInt::default(),
             Forward::default(),
+            ParseInt::default(),
+            Comparison::default(),
+            Var::default(),
             ForStatementCondition::default(),
         ))
         .unwrap();

src/ps/strategy.rs

@@ -15,23 +15,12 @@ impl Strategy<Powershell> for PowershellStrategy {
             "statement_block" => {
                 let parent = node.parent().unwrap();
                 match parent.kind() {
-                    "while_statement" => {
-                        // Don't visit node inferred node with a branch set to false
-                        if let Some(condition) = parent.named_child("condition") {
-                            if let Some(Raw(Bool(false))) = condition.data() {
-                                return Ok(Break);
-                            }
-                        }
-                        // Any other inferred type led to unpredictable branch visit
-                        Ok(Continue(Unpredictable))
-                    }
-                    "for_statement" => {
-                        if let Some(Powershell::Loop(LoopStatus::Dead)) = parent.data() {
-                            return Ok(Break);
-                        }
+                    "for_statement" | "while_statement" => match parent.data() {
+                        Some(Powershell::Loop(LoopStatus::Dead)) => Ok(Break),
+                        Some(Powershell::Loop(LoopStatus::OneTurn)) => Ok(Continue(Predictable)),
+                        _ => Ok(Continue(Unpredictable)),
+                    },
 
-                        Ok(Continue(Unpredictable))
-                    }
                     "if_statement" => {
                         // if ($true) control flow
                         if let Some(condition) = parent.named_child("condition") {
@@ -119,6 +108,8 @@ impl Strategy<Powershell> for PowershellStrategy {
                 // We were not able to infer type so we are in an unpredictable case
                 Ok(Continue(Unpredictable))
             }
+            // Do not visit for iterator, as it is unsure if and when they are executed
+            "for_iterator" => Ok(Break),
             // All this statement are labeled and not inferred at this moment
             "trap_statement" | "try_statement" | "catch_clause" | "finally_clause"
             | "data_statement" | "parallel_statement" | "sequence_statement"

src/ps/var.rs

@@ -236,7 +236,7 @@ impl<'a> RuleMut<'a> for Var {
                 }
             }
 
-            "while_statement" => {
+            "while_statement" | "for_statement" => {
                 node.start_transaction()?;
             }
 
@@ -281,21 +281,14 @@ impl<'a> RuleMut<'a> for Var {
                             if let (current_value, Some(new_value)) =
                                 (scope.get_var(&var_name), right.data())
                             {
-                                // disable anything from for_initializer
-                                if view.get_parent_of_types(vec!["for_initializer"]).is_some() {
-                                    scope.forget(&var_name);
-                                } else {
-                                    // only predictable assignment is handled of local var
-                                    let is_local = scope.is_local(&var_name).unwrap_or(true);
-                                    if flow == ControlFlow::Continue(BranchFlow::Predictable)
-                                        || is_local
-                                    {
-                                        match assign_handler(current_value, operator, new_value) {
-                                            Some(assign_value) => {
-                                                scope.assign(&var_name, assign_value)
-                                            }
-                                            _ => scope.forget(&var_name),
-                                        }
+                                // only predictable assignment is handled of local var
+                                let is_local = scope.is_local(&var_name).unwrap_or(true);
+                                if flow == ControlFlow::Continue(BranchFlow::Predictable)
+                                    || is_local
+                                {
+                                    match assign_handler(current_value, operator, new_value) {
+                                        Some(assign_value) => scope.assign(&var_name, assign_value),
+                                        _ => scope.forget(&var_name),
                                     }
                                 }
                             }

src/tree.rs

@@ -201,9 +201,6 @@ impl<T> Storage for HashMapStorage<T> {
         Ok(())
     }
 
-    /// use to monitor if the storage was updated
-    /// between the start and the end function
-    ///
     /// # Example
     /// ```
     /// extern crate tree_sitter;
@@ -226,12 +223,6 @@ impl<T> Storage for HashMapStorage<T> {
     /// assert_eq!(storage.get(root_id), Some(&42));
     /// storage.apply_transaction();
     /// assert_eq!(storage.get(root_id), Some(&42));
-    ///
-    /// storage.start_transaction();
-    /// storage.set(root_id, 0);
-    /// assert_eq!(storage.get(root_id), Some(&0));
-    /// storage.abort_transaction();
-    /// assert_eq!(storage.get(root_id), Some(&42));
     /// ```
     fn apply_transaction(&mut self) {
         let node_ids: Vec<usize> = self.pending.keys().cloned().collect();
@@ -244,8 +235,42 @@ impl<T> Storage for HashMapStorage<T> {
             });
         self.is_ongoing_transaction = false;
     }
+
+    /// Abort transaction by forget all the variables to update
+    ///
+    /// # Example
+    /// ```
+    /// extern crate tree_sitter;
+    /// extern crate tree_sitter_powershell;
+    /// use tree_sitter::{Parser, Language};
+    /// use tree_sitter_powershell::LANGUAGE as powershell_language;
+    /// use minusone::tree::{Storage, HashMapStorage};
+    ///
+    /// let mut parser = Parser::new();
+    /// parser.set_language(&powershell_language.into()).unwrap();
+    ///
+    /// let ts_tree = parser.parse("4+5", None).unwrap();
+    /// let mut storage = HashMapStorage::<u32>::default();
+    /// let root_id = ts_tree.root_node().id();
+    ///
+    /// storage.set(root_id, 42);
+    /// storage.start_transaction();
+    ///
+    /// storage.start_transaction();
+    /// storage.set(root_id, 0);
+    /// assert_eq!(storage.get(root_id), Some(&0));
+    /// storage.abort_transaction();
+    /// assert_eq!(storage.get(root_id), None);
+    /// ```
     fn abort_transaction(&mut self) {
-        self.pending.clear();
+        let keys: Vec<usize> = self.pending.keys().cloned().collect();
+        let keys_to_forget: Vec<usize> = keys
+            .iter()
+            .filter(|k| matches!(self.pending.remove(k), Some(PendingAction::Add(_))))
+            .cloned()
+            .collect();
+        keys_to_forget.iter().for_each(|k| _ = self.map.remove(k));
+
         self.is_ongoing_transaction = false;
     }
 }