Update on "Add regex stuff"

Implement the logic for matching code with regex. Add tests for : * query as regex * filter as regex * propagate when hole is captured by regex * propagate built in rules when query is regex ------- [ghstack-poisoned]
uber · Jul 11, 2023 · 3ac21b0 · 3ac21b0
2 parents 1454962 + d847e7a
commit 3ac21b0
Show file tree

Hide file tree

Showing 14 changed files with 60 additions and 60 deletions.
diff --git a/.github/workflows/playground_test.yml b/.github/workflows/playground_test.yml
@@ -0,0 +1,30 @@
+name: Playground Piranha
+on:
+  pull_request:
+  push: 
+    branches:
+      - master
+jobs:
+  detect:
+    uses: ./.github/workflows/detect_changes.yml
+
+  build_and_test:
+    name: Playground test
+    runs-on: ubuntu-latest
+    needs: detect
+    steps:
+      - name: Checkout Piranha sources
+        uses: actions/checkout@v3
+      - name: Set up Python 3.9
+        uses: actions/setup-python@v4
+        with:
+          python-version: 3.9
+      - name: Create virtualenv and install dependencies
+        run: |
+          python -m venv .env
+          source .env/bin/activate
+          pip3 install -r experimental/requirements.txt
+      - name: Run Python tests
+        run: |
+          source .env/bin/activate
+          pytest -s -rx .
diff --git a/POLYGLOT_README.md b/POLYGLOT_README.md
@@ -145,7 +145,6 @@ An object of PiranhaArguments can be instantiated with the following arguments:
 - (*optional*) `number_of_ancestors_in_parent_scope` (`usize`): The number of ancestors considered when `PARENT` rules
 - (*optional*) `delete_file_if_empty` (`bool`): User option that determines whether an empty file will be deleted
 - (*optional*) `delete_consecutive_new_lines` (`bool`) : Replaces consecutive `\n`s  with a single `\n`
-- (*optional*) `dry_run` (`bool`) : Disables in-place rewriting of code
 
 <h5> Returns </h5>
 

diff --git a/experimental/piranha_playground/rule_inference/rule_application.py b/experimental/piranha_playground/rule_inference/rule_application.py
@@ -133,10 +133,6 @@ def refactor_codebase(self, dry_run: bool = True) -> List[PiranhaOutputSummary]:
         try:
             toml_dict = toml.loads(self.rules)
             rule_graph = RawRuleGraph.from_toml(toml_dict)
-
-            # Create the Piranha rule graph
-
-            # Create the PiranhaArguments object
             args = PiranhaArguments(
                 language=self.language,
                 path_to_codebase=self.path_to_codebase,
@@ -169,7 +165,6 @@ def refactor_snippet(source_code: str, language: str, rules: str) -> str:
         try:
             toml_dict = toml.loads(rules)
             substitutions = toml_dict.get("substitutions", [{}])[0]
-
             refactored_code, success = run_piranha_with_timeout(
                 source_code,
                 language,

diff --git a/experimental/piranha_playground/rule_inference/tests/test_codebase_refactorer.py b/experimental/piranha_playground/rule_inference/tests/test_codebase_refactorer.py
@@ -57,17 +57,20 @@ def test_run_piranha_with_timeout_success():
             assert rewrite.p_match.matched_string and rewrite.p_match.matches
 
 
-# Test for timeout scenario while executing Piranha
-def test_run_piranha_with_timeout_exception():
+@pytest.mark.skip(reason="This test doesn't work in CI")
+def test_snippet_application():
     language = "java"
-    graph = """
+    graph = '''
     [[rules]]
     name = "rename_variable"
-    query = "(identifier) @var_name"
+    query = """(
+        (identifier) @var_name 
+        (#eq? @var_name "A")
+    )"""
     replace_node = "var_name"
-    replace = "other_name"
-    """
+    replace = "B"
+    '''
     source_code = "class A {}"
 
-    with pytest.raises(CodebaseRefactorerException):
-        CodebaseRefactorer.refactor_snippet(source_code, language, graph)
+    x = CodebaseRefactorer.refactor_snippet(source_code, language, graph)
+    assert x == "class B {}"
diff --git a/src/cleanup_rules/go/rules.toml b/src/cleanup_rules/go/rules.toml
@@ -129,16 +129,7 @@ name = "simplify_something_and_false"
 query = """
 (
     (binary_expression
-        left : ([
-                (identifier)
-                (parenthesized_expression (identifier))
-                (true)
-                (parenthesized_expression (true))
-                (false)
-                (parenthesized_expression (false))
-                (selector_expression)
-                (parenthesized_expression (selector_expression))
-            ]) @lhs
+        left : (_) @lhs
         operator : "&&"
         right: [(false) (parenthesized_expression (false))]
     ) @binary_expression

diff --git a/src/cleanup_rules/java/rules.toml b/src/cleanup_rules/java/rules.toml
@@ -219,15 +219,10 @@ groups = ["boolean_expression_simplify"]
 query = """
 (
     (binary_expression
-        left : [
-            (identifier)
-            (true)
-            (false)
-        ] @lhs
+        left : (_) @lhs
         operator : "&&"
-        right: (false)
-    )
-@binary_expression)
+        right: (false)) @binary_expression
+)
 """
 replace = "false"
 replace_node = "binary_expression"

diff --git a/src/cleanup_rules/kt/rules.toml b/src/cleanup_rules/kt/rules.toml
@@ -288,9 +288,7 @@ groups = ["boolean_expression_simplify"]
 name = "simplify_something_and_false"
 query = """
 (
-(conjunction_expression [(simple_identifier)
-                          (boolean_literal)
-                        ] @lhs 
+(conjunction_expression (_) @lhs 
                         ((boolean_literal) @rhs) ) @conjunction_expression
 (#eq? @rhs "false")  
 )

diff --git a/src/cleanup_rules/swift/rules.toml b/src/cleanup_rules/swift/rules.toml
@@ -194,10 +194,10 @@ is_seed_rule = false
 # In other scenarios (i.e. is a method invocation or some other expression) we would not simplify at all, as:
 # such expressions could have side effects
 [[rules]]
-name = "some_identifier_and_false"
+name = "something_and_false"
 query = """(
 (conjunction_expression
-        lhs: (simple_identifier) 
+        lhs: (_) 
         rhs: [(boolean_literal) @false  
             (tuple_expression 
                 value: (boolean_literal) @false)]

diff --git a/test-resources/go/feature_flag/builtin_rules/boolean_expression_simplify/expected/sample.go b/test-resources/go/feature_flag/builtin_rules/boolean_expression_simplify/expected/sample.go
@@ -53,17 +53,12 @@ func simplify_false_and_something(something bool) {
     fmt.Println("else 4")
     // selector_expression: simplify
     fmt.Println("else 5")
-    // does not simplify binary_expression; left call may contain side-effects
-    if exp.BoolValue("random") && false {
-        fmt.Println("keep 1")
-    } else {
-        fmt.Println("keep 2")
-    }
+    // simplify
+
+    fmt.Println("keep 2")
 
     // function call && false
-    if f1() && false {
-        fmt.Println("keep as it is")
-    }
+
 
     // function call || true
     if f1() || true {

diff --git a/test-resources/go/feature_flag/builtin_rules/boolean_expression_simplify/input/sample.go b/test-resources/go/feature_flag/builtin_rules/boolean_expression_simplify/input/sample.go
@@ -91,15 +91,15 @@ func simplify_false_and_something(something bool) {
     } else {
         fmt.Println("else 5")
     }
-    // does not simplify binary_expression; left call may contain side-effects
+    // simplify
     if exp.BoolValue("random") && exp.BoolValue("false") {
-        fmt.Println("keep 1")
+        fmt.Println("to be removed 1")
     } else {
         fmt.Println("keep 2")
     }
     // function call && false
     if f1() && exp.BoolValue("false") {
-        fmt.Println("keep as it is")
+        fmt.Println("to be removed 2")
     }
 
     // function call || true

diff --git a/test-resources/java/feature_flag_system_2/control/expected/XPMethodChainCases.java b/test-resources/java/feature_flag_system_2/control/expected/XPMethodChainCases.java
@@ -33,10 +33,6 @@ public static void foobar(Parameter cp) {
       System.out.println("!!!");
     }
 
-    if (sp.otherFlag().getCachedValue() && false) {	
-      System.out.println("!!!");	
-    }
-
     if (sp.otherFlag().getCachedValue()) {
       System.out.println("!!!");
     }

diff --git a/test-resources/java/regex_based_matcher/configurations/rules.toml b/test-resources/java/regex_based_matcher/configurations/rules.toml
@@ -32,7 +32,7 @@ replace = "fed"
 
 # The below three rules do a dummy type migration from OurListOfInteger to List<Integer> 
 
-# Updates the import statement from `java.util.List` to `com.uber.NewList`
+# Updates the import statement from `out.list.OurListOfInteger` to `java.util.List`
 [[rules]]
 name = "update_import"
 query = """rgx (?P<n>our\\.list\\.OurListOfInteger)"""
@@ -59,7 +59,8 @@ holes = ["name"]
 is_seed_rule = false
 
 
-# The below three rules do a dummy type migration  like - from OurMapOfStringInteger to HashMap<String, Integer>
+# The below three rules do a dummy type migration OurMapOf{T1}{T2} to HashMap<T1, T2>. For example, from OurMapOfStringInteger to HashMap<String, Integer>. 
+# This is to exercise non-constant regex matches for replace_node.
 
 # Deletes the import statement `our.map.OurMapOf...` 
 [[rules]]
@@ -68,7 +69,7 @@ query = """rgx (?P<n>import our\\.map\\.OurMapOf\\w+;)"""
 replace_node = "n"
 replace = ""
 
-# Adds Import to java.util.hashmap if absent
+# Adds Import to java.util.Hashmap if absent
 [[rules]]
 name = "add_import_For_hashmap"
 query = """(package_declaration) @pkg"""

diff --git a/test-resources/kt/feature_flag_system_2/control/expected/XPMethodChainCases.kt b/test-resources/kt/feature_flag_system_2/control/expected/XPMethodChainCases.kt
@@ -37,9 +37,6 @@ internal class XPMethodChainCases {
                 println("!!!")
             }
 
-            if (sp.isOtherFlag().cachedValue && false) {
-                println("LHS is not a simple identifier!!!")
-            }
             if (sp.isOtherFlag().cachedValue) {
                 println("!!!")
             }

diff --git a/test-resources/swift/cleanup_rules/expected/SampleClass.swift b/test-resources/swift/cleanup_rules/expected/SampleClass.swift
@@ -43,7 +43,7 @@ class SampleClass {
     func checkAndFalse() {
         isEnabled = false
         isEnabled = false
-        isEnabled = f2() && false
+        isEnabled = false
         isEnabled = false
     }