Add a new edge kind - ParentIterative (#617)

src/models/edit.rs

@@ -11,12 +11,14 @@ Copyright (c) 2023 Uber Technologies, Inc.
  limitations under the License.
 */
 
+use std::collections::HashMap;
 use std::fmt;
 
 use super::matches::Range;
 use super::{
   matches::Match, rule::InstantiatedRule, rule_store::RuleStore, source_code_unit::SourceCodeUnit,
 };
+use crate::models::rule_graph::{PARENT, PARENT_ITERATIVE};
 use crate::utilities::{
   gen_py_str_methods,
   tree_sitter_utilities::{get_context, get_node_for_range},
@@ -105,18 +107,20 @@ impl fmt::Display for Edit {
 impl SourceCodeUnit {
   // Apply all the `rules` to the node, parent, grand parent and great grand parent.
   // Short-circuit on the first match.
-  pub(crate) fn get_edit_for_context(
-    &self, previous_edit_start: usize, previous_edit_end: usize, rules_store: &mut RuleStore,
-    rules: &Vec<InstantiatedRule>,
+  pub(crate) fn get_edit_for_ancestors(
+    &self, previous_edit_range: &Range, rules_store: &mut RuleStore,
+    next_rules: &HashMap<String, Vec<InstantiatedRule>>,
   ) -> Option<Edit> {
     let number_of_ancestors_in_parent_scope = *self
       .piranha_arguments()
       .number_of_ancestors_in_parent_scope();
-    let changed_node = get_node_for_range(self.root_node(), previous_edit_start, previous_edit_end);
-    debug!(
-      "\n{}",
-      format!("Changed node kind {}", changed_node.kind()).blue()
+    let changed_node = get_node_for_range(
+      self.root_node(),
+      *previous_edit_range.start_byte(),
+      *previous_edit_range.end_byte(),
     );
+    debug!("\nChanged node kind {}", changed_node.kind().blue());
+
     // Context contains -  the changed node in the previous edit, its's parent, grand parent and great grand parent
     let context = || {
       get_context(
@@ -125,13 +129,24 @@ impl SourceCodeUnit {
         number_of_ancestors_in_parent_scope,
       )
     };
-    for rule in rules {
+    //  we apply the rules in the order they are provided to each ancestor in the context
+    for rule in &next_rules[PARENT] {
       for ancestor in &context() {
         if let Some(edit) = self.get_edit(rule, rules_store, *ancestor, false) {
           return Some(edit);
         }
       }
     }
+
+    // we apply the rules to each ancestor in the context in the order they are provided
+    for ancestor in &context() {
+      for rule in &next_rules[PARENT_ITERATIVE] {
+        if let Some(edit) = self.get_edit(rule, rules_store, *ancestor, false) {
+          return Some(edit);
+        }
+      }
+    }
+
     None
   }
 

src/models/rule_graph.rs

@@ -35,6 +35,7 @@ use pyo3::prelude::{pyclass, pymethods};
 
 pub(crate) static GLOBAL: &str = "Global";
 pub(crate) static PARENT: &str = "Parent";
+pub(crate) static PARENT_ITERATIVE: &str = "ParentIterative";
 
 #[derive(Debug, Default, Getters, MutGetters, Builder, Clone, PartialEq)]
 #[builder(build_fn(name = "create"))]
@@ -194,7 +195,7 @@ impl RuleGraph {
       }
     }
     // Add empty entry, incase no next rule was found for a particular scope
-    for scope in [PARENT, GLOBAL] {
+    for scope in [PARENT, PARENT_ITERATIVE, GLOBAL] {
       next_rules.entry(scope.to_string()).or_default();
     }
     next_rules

src/models/source_code_unit.rs

@@ -23,7 +23,7 @@ use tree_sitter::{InputEdit, Node, Parser, Tree};
 
 use crate::{
   models::capture_group_patterns::CGPattern,
-  models::rule_graph::{GLOBAL, PARENT},
+  models::rule_graph::{GLOBAL, PARENT, PARENT_ITERATIVE},
   utilities::tree_sitter_utilities::{
     get_node_for_range, get_replace_range, get_tree_sitter_edit, number_of_errors,
   },
@@ -240,33 +240,20 @@ impl SourceCodeUnit {
 
       // Process the parent
       // Find the rules to be applied in the "Parent" scope that match any parent (context) of the changed node in the previous edit
-      if let Some(edit) = self.get_edit_for_context(
-        *current_replace_range.start_byte(),
-        *current_replace_range.end_byte(),
-        rules_store,
-        &next_rules_by_scope[PARENT],
-      ) {
+      if let Some(edit) =
+        self.get_edit_for_ancestors(&current_replace_range, rules_store, &next_rules_by_scope)
+      {
         self.rewrites_mut().push(edit.clone());
-        debug!(
-          "\n{}",
-          format!(
-            "Cleaning up the context, by applying the rule - {}",
-            edit.matched_rule()
-          )
-          .green()
-        );
         // Apply the matched rule to the parent
         let applied_edit = self.apply_edit(&edit, parser);
         current_replace_range = get_replace_range(applied_edit);
         current_rule = edit.matched_rule().to_string();
         // Add the (tag, code_snippet) mapping to substitution table.
         self.substitutions.extend(edit.p_match().matches().clone());
       } else {
-        // No more parents found for cleanup
         break;
       }
     }
-
     // Apply the next rules from the stack
     for (sq, rle) in &next_rules_stack {
       self.apply_rule(rle.clone(), rules_store, parser, &Some(sq.clone()));
@@ -280,8 +267,8 @@ impl SourceCodeUnit {
     stack: &mut VecDeque<(CGPattern, InstantiatedRule)>,
   ) {
     for (scope_level, rules) in next_rules_by_scope {
-      // Scope level is not "PArent" or "Global"
-      if ![PARENT, GLOBAL].contains(&scope_level.as_str()) {
+      // Scope level is not "Parent", "ParentIterative" or "Global"
+      if ![PARENT, PARENT_ITERATIVE, GLOBAL].contains(&scope_level.as_str()) {
         for rule in rules {
           let scope_query = self.get_scope_query(
             scope_level,

src/models/unit_tests/rule_test.rs

@@ -16,8 +16,9 @@ use crate::{
     default_configs::{JAVA, UNUSED_CODE_PATH},
     filter::Filter,
     language::PiranhaLanguage,
-    matches::Point,
+    matches::{Point, Range},
     piranha_arguments::PiranhaArgumentsBuilder,
+    rule_graph::{PARENT, PARENT_ITERATIVE},
   },
   utilities::eq_without_whitespace,
 };
@@ -225,9 +226,17 @@ fn test_get_edit_for_context_positive() {
     PathBuf::new().as_path(),
     &args,
   );
-  let edit =
-    source_code_unit.get_edit_for_context(41_usize, 44_usize, &mut rule_store, &vec![rule]);
-  // let edit = rule.get_edit(&source_code_unit, &mut rule_store, node, true);
+  let range = Range {
+    start_byte: 41_usize,
+    end_byte: 44_usize,
+    ..Default::default()
+  };
+
+  let next_rules = HashMap::from([
+    (String::from(PARENT), vec![rule.clone()]),
+    (String::from(PARENT_ITERATIVE), vec![rule]),
+  ]);
+  let edit = source_code_unit.get_edit_for_ancestors(&range, &mut rule_store, &next_rules);
   assert!(edit.is_some());
 }
 
@@ -266,9 +275,16 @@ fn test_get_edit_for_context_negative() {
     PathBuf::new().as_path(),
     &args,
   );
-  let edit =
-    source_code_unit.get_edit_for_context(29_usize, 33_usize, &mut rule_store, &vec![rule]);
-  // let edit = rule.get_edit(&source_code_unit, &mut rule_store, node, true);
+  let range = Range {
+    start_byte: 29_usize,
+    end_byte: 33_usize,
+    ..Default::default()
+  };
+  let next_rules = HashMap::from([
+    (String::from(PARENT), vec![rule.clone()]),
+    (String::from(PARENT_ITERATIVE), vec![rule]),
+  ]);
+  let edit = source_code_unit.get_edit_for_ancestors(&range, &mut rule_store, &next_rules);
   assert!(edit.is_none());
 }
 

src/tests/test_piranha_java.rs

@@ -67,6 +67,10 @@ create_rewrite_tests! {
   test_java_delete_method_invocation_argument: "delete_method_invocation_argument", 1;
   test_java_delete_method_invocation_argument_no_op: "delete_method_invocation_argument_no_op", 0;
   test_regex_based_matcher: "regex_based_matcher", 1, cleanup_comments = true;
+  test_parent_iterative: "parent_iterative/positive", 1;
+  // This test shows what happens if edges is not PARENT_ITERATIVE
+  // It tests the same scenario as the above test, but with edge kind as PARENT
+  test_parent_iterative_negative: "parent_iterative/negative", 1;
 }
 
 create_match_tests! {

test-resources/java/parent_iterative/negative/configurations/edges.toml

@@ -0,0 +1,21 @@
+# Copyright (c) 2023 Uber Technologies, Inc.
+# 
+# <p>Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file
+# except in compliance with the License. You may obtain a copy of the License at
+# <p>http://www.apache.org/licenses/LICENSE-2.0
+# 
+# <p>Unless required by applicable law or agreed to in writing, software distributed under the
+# License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+# express or implied. See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+[[edges]]
+scope = "Parent"
+from = "spark_conf_change_java_scala"
+to = ["BuilderPattern"]
+
+[[edges]]
+scope = "Parent"
+from = "BuilderPattern"
+to = ["BuilderPattern"]

test-resources/java/parent_iterative/negative/configurations/rules.toml

@@ -0,0 +1,48 @@
+# Copyright (c) 2023 Uber Technologies, Inc.
+# 
+# <p>Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file
+# except in compliance with the License. You may obtain a copy of the License at
+# <p>http://www.apache.org/licenses/LICENSE-2.0
+# 
+# <p>Unless required by applicable law or agreed to in writing, software distributed under the
+# License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+# express or implied. See the License for the specific language governing permissions and
+# limitations under the License.
+
+[[rules]]
+name = "spark_conf_change_java_scala"
+query = "cs new SparkConf()"
+replace_node = "*"
+replace = 'new SparkSession.builder()'
+
+[[rules]]
+name = "set_spark_home_change_java_scala"
+query = "cs :[r].setSparkHome(:[a1])"
+replace_node = "*"
+replace = "@r.sparkHome(@a1)"
+is_seed_rule = false
+groups = ["BuilderPattern"]
+
+[[rules]]
+name = "set_executor_env_change_2_java_scala"
+query = "cs :[r].setExecutorEnv(:[a1], :[a2])"
+replace_node = "*"
+replace = "@r.executorEnv(@a1, @a2)"
+is_seed_rule = false
+groups = ["BuilderPattern"]
+
+[[rules]]
+name = "app_name_change_java_scala"
+query = "cs :[r].setAppName(:[app_name])"
+replace_node = "*"
+replace = "@r.appName(@app_name)"
+is_seed_rule = false
+groups = ["BuilderPattern"]
+
+[[rules]]
+name = "master_name_change_java_scala"
+query = "cs :[r].setMaster(:[master])"
+replace_node = "*"
+replace = "@r.master(@master)"
+is_seed_rule = false
+groups = ["BuilderPattern"]

test-resources/java/parent_iterative/negative/expected/sample.java

@@ -0,0 +1,28 @@
+package com.piranha;
+
+import org.apache.spark.SparkConf;
+import org.apache.spark.sql.SparkSession;
+
+class Sample {
+  void main() {
+    // `setApp` and `setMaster` will not get updated
+    val conf1 = new SparkSession.builder()
+        .setMaster(master1)
+        .setAppName(appName1)
+        .executorEnv("spark.executor.extraClassPath1", "test1");
+
+    // `setApp` and `setMaster` will not get updated
+    val conf2 = new SparkSession.builder()
+        .setAppName(appName2)
+        .setMaster(master2)
+        .sparkHome(sparkHome2)
+        .executorEnv("spark.executor.extraClassPath2", "test2");
+
+  // `setMaster` and `setExecutorEnv` will not get updated
+    val conf3 = new SparkSession.builder()
+        .setExecutorEnv("spark.executor.extraClassPath3", "test3")
+        .setMaster(master3)
+        .sparkHome(sparkHome3)
+        .appName(appName3);
+  }
+}

test-resources/java/parent_iterative/negative/input/sample.java

@@ -0,0 +1,29 @@
+package com.piranha;
+
+import org.apache.spark.SparkConf;
+import org.apache.spark.sql.SparkSession;
+
+class Sample {
+  void main() {
+
+    // `setApp` and `setMaster` will not get updated
+    val conf1 = new SparkConf()
+      .setMaster(master1)
+      .setAppName(appName1)
+      .setExecutorEnv("spark.executor.extraClassPath1", "test1");
+
+  // `setApp` and `setMaster` will not get updated
+    val conf2 = new SparkConf()
+      .setAppName(appName2)
+      .setMaster(master2)
+      .setSparkHome(sparkHome2)
+      .setExecutorEnv("spark.executor.extraClassPath2", "test2");
+
+  // `setMaster` and `setExecutorEnv` will not get updated
+    val conf3 = new SparkConf()
+      .setExecutorEnv("spark.executor.extraClassPath3", "test3")
+      .setMaster(master3)
+      .setSparkHome(sparkHome3)
+      .setAppName(appName3);
+  }
+}

test-resources/java/parent_iterative/positive/configurations/edges.toml

@@ -0,0 +1,21 @@
+# Copyright (c) 2023 Uber Technologies, Inc.
+# 
+# <p>Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file
+# except in compliance with the License. You may obtain a copy of the License at
+# <p>http://www.apache.org/licenses/LICENSE-2.0
+# 
+# <p>Unless required by applicable law or agreed to in writing, software distributed under the
+# License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+# express or implied. See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+[[edges]]
+scope = "ParentIterative"
+from = "spark_conf_change_java_scala"
+to = ["BuilderPattern"]
+
+[[edges]]
+scope = "ParentIterative"
+from = "BuilderPattern"
+to = ["BuilderPattern"]