From d802f3eabb0922387b902d5e37201b16848499b1 Mon Sep 17 00:00:00 2001 From: Daniel Ramos Date: Wed, 6 Sep 2023 20:18:23 +0100 Subject: [PATCH] Support for trailing commas & skip Comments with concrete syntax (#596) Co-authored-by: Ameya Ketkar <94497232+ketkarameya@users.noreply.github.com> --- experimental/requirements.txt | 4 +- src/models/concrete_syntax.rs | 18 +++++- src/models/unit_tests/concrete_syntax_test.rs | 20 +++++- .../control/input/ExperimentInterface.java | 3 +- ...ExperimentInterfaceAnnotatedInterface.java | 3 +- .../input/XPFlagCleanerInterfaceMethod.java | 64 +++++-------------- .../control/input/XPMethodChainCases.java | 29 +++------ 7 files changed, 67 insertions(+), 74 deletions(-) diff --git a/experimental/requirements.txt b/experimental/requirements.txt index 3cf650897..2026ad4fc 100644 --- a/experimental/requirements.txt +++ b/experimental/requirements.txt @@ -1,4 +1,4 @@ -tree-sitter +tree-sitter==0.20.1 tree-sitter-languages attrs openai @@ -8,4 +8,4 @@ pytest flask flask-socketio comby -eventlet \ No newline at end of file +eventlet diff --git a/src/models/concrete_syntax.rs b/src/models/concrete_syntax.rs index 6f9603b97..d85e937a7 100644 --- a/src/models/concrete_syntax.rs +++ b/src/models/concrete_syntax.rs @@ -17,6 +17,7 @@ use regex::Regex; use std::collections::HashMap; use tree_sitter::{Node, TreeCursor}; +use tree_sitter_traversal::Cursor; use crate::models::capture_group_patterns::ConcreteSyntax; use crate::models::matches::Match; @@ -128,7 +129,13 @@ pub(crate) fn get_matches_for_node( ); } - let node = cursor.node(); + let mut node = cursor.node(); + + // Skip comment nodes always + while node.kind().contains("comment") && cursor.goto_next_sibling() { + node = cursor.node(); + } + // In case the template starts with :[var_name], we try match if let Some(caps) = RE_VAR.captures(match_template) { let var_name = &caps["var_name"]; @@ -148,6 +155,15 @@ pub(crate) fn get_matches_for_node( let current_node_code = current_node.utf8_text(source_code).unwrap(); find_next_sibling(&mut tmp_cursor); + // Support for trailing commas + // This skips trailing commas as we are parsing through the match template + // Skips the comma node if the template doesn't contain it. + let next_node = tmp_cursor.node(); + let next_node_text = next_node.utf8_text(source_code).unwrap(); + if next_node_text == "," && !meta_advanced.0.starts_with(',') { + find_next_sibling(&mut tmp_cursor); // Skip comma + } + if let (mut recursive_matches, true) = get_matches_for_node(&mut tmp_cursor, source_code, &meta_advanced) { diff --git a/src/models/unit_tests/concrete_syntax_test.rs b/src/models/unit_tests/concrete_syntax_test.rs index b32209e8e..dd7e39af5 100644 --- a/src/models/unit_tests/concrete_syntax_test.rs +++ b/src/models/unit_tests/concrete_syntax_test.rs @@ -13,12 +13,14 @@ use crate::models::capture_group_patterns::ConcreteSyntax; use crate::models::concrete_syntax::get_all_matches_for_concrete_syntax; +use crate::models::default_configs::GO; use crate::models::{default_configs::JAVA, language::PiranhaLanguage}; fn run_test( code: &str, pattern: &str, expected_matches: usize, expected_vars: Vec>, + language: &str, ) { - let java = PiranhaLanguage::from(JAVA); + let java = PiranhaLanguage::from(language); let mut parser = java.parser(); let tree = parser.parse(code.as_bytes(), None).unwrap(); let meta = ConcreteSyntax(String::from(pattern)); @@ -49,6 +51,7 @@ fn test_single_match() { "public int :[name] = :[value];", 1, vec![vec![("name", "a"), ("value", "10")]], + JAVA, ); } @@ -62,6 +65,7 @@ fn test_multiple_match() { vec![("name", "a"), ("value", "10")], vec![("name", "b"), ("value", "20")], ], + JAVA, ); } @@ -72,5 +76,19 @@ fn test_no_match() { "public String :[name] = :[value];", 0, vec![], + JAVA, + ); +} + +#[test] +fn test_trailing_comma() { + run_test( + "a.foo(x, // something about the first argument + y, // something about the second argumet + );", + ":[var].foo(:[arg1], :[arg2])", + 2, + vec![vec![("var", "a"), ("arg1", "x"), ("arg2", "y")]], + GO, ); } diff --git a/test-resources/java/feature_flag_system_2/control/input/ExperimentInterface.java b/test-resources/java/feature_flag_system_2/control/input/ExperimentInterface.java index 5f19b299a..3ea4fb13d 100644 --- a/test-resources/java/feature_flag_system_2/control/input/ExperimentInterface.java +++ b/test-resources/java/feature_flag_system_2/control/input/ExperimentInterface.java @@ -13,8 +13,7 @@ interface SomeParameter { - @BoolParam(key = "STALE_FLAG", namespace = "some_long_name") - BoolParameter isStaleFeature(); + @BoolParam(key = "other_flag", namespace = "some_long_name") BoolParameter isOtherFlag(); diff --git a/test-resources/java/feature_flag_system_2/control/input/ExperimentInterfaceAnnotatedInterface.java b/test-resources/java/feature_flag_system_2/control/input/ExperimentInterfaceAnnotatedInterface.java index 094d4c7ae..8b3c16ae5 100644 --- a/test-resources/java/feature_flag_system_2/control/input/ExperimentInterfaceAnnotatedInterface.java +++ b/test-resources/java/feature_flag_system_2/control/input/ExperimentInterfaceAnnotatedInterface.java @@ -14,8 +14,7 @@ @ParameterDefinition(namespace = "some_long_name") interface SomeParameter { - @BoolParam(key = "STALE_FLAG") - BoolParameter isStaleFeature(); + @BoolParam(key = "other_flag", namespace = "some_long_name") BoolParameter isOtherFlag(); diff --git a/test-resources/java/feature_flag_system_2/control/input/XPFlagCleanerInterfaceMethod.java b/test-resources/java/feature_flag_system_2/control/input/XPFlagCleanerInterfaceMethod.java index 83d972187..5b5020a94 100644 --- a/test-resources/java/feature_flag_system_2/control/input/XPFlagCleanerInterfaceMethod.java +++ b/test-resources/java/feature_flag_system_2/control/input/XPFlagCleanerInterfaceMethod.java @@ -17,57 +17,39 @@ class XPFlagCleanerPositiveCases { private ExperimentInterface experimentation; - private boolean ftBool = experimentation.isStaleFeature().getCachedValue(); + public void conditional_contains_stale_flag() { - if (experimentation.isStaleFeature().getCachedValue()) { - System.out.println("Hello World"); - } + System.out.println("Hello World"); } public void conditional_with_else_contains_stale_flag() { - if (experimentation.isStaleFeature().getCachedValue()) { - System.out.println("Hello World"); - } else { - System.out.println("Hi world"); - } + System.out.println("Hello World"); } public void conditional_with_else_contains_stale_flag_tbool() { - bool tBool = exp.isStaleFeature().getCachedValue(); - if (tBool && true) { - System.out.println("Hello World"); - } else { - System.out.println("Hi world"); - } + + System.out.println("Hello World"); } public void conditional_with_else_contains_stale_flag_tbool(int a) { - bool tBool = exp.isStaleFeature().getCachedValue(); - if (tBool && true) { - System.out.println("Hello World"); - } else { - System.out.println("Hi world"); - } + + System.out.println("Hello World"); } public void conditional_with_else_contains_stale_flag_tbool(int a, bool abc) { - bool tBool = exp.isStaleFeature().getCachedValue(); - if (!tBool && true) { - System.out.println("Hello World"); - } else { - System.out.println("Hi world"); - } + + System.out.println("Hi world"); } public void conditional_with_else_contains_stale_flag_tbool_reassigned(int a, bool abc, int z) { // Currently if there is another assignment, variable will not be inlined. - bool tBool = exp.isStaleFeature().getCachedValue(); + bool tBool = true; tBool = abc() && tBool; if (!tBool && true) { System.out.println("Hello World"); @@ -79,40 +61,28 @@ public void conditional_with_else_contains_stale_flag_tbool_reassigned(int a, bo public void conditional_with_else_contains_stale_flag_tbool_reassigned_to_same_val( int a, bool abc, int z) { - bool tBool = exp.isStaleFeature().getCachedValue(); - tBool = true; - if (!tBool && true) { - System.out.println("Hello World"); - } else { - System.out.println("Hi world"); - } + + + System.out.println("Hi world"); } public void conditional_with_else_contains_stale_flag_ftbool(int a) { - if (ftBool && true) { - System.out.println("Hello World"); - } else { - System.out.println("Hi world"); - } + System.out.println("Hello World"); } public void conditional_with_else_contains_stale_flag_tbool_reassigned_ftbool( int a, bool abc, int z) { // Currently if there is another assignment, variable will not be inlined. - ftBool = exp.isStaleFeature().getCachedValue(); - if (!ftBool && true) { - System.out.println("Hello World"); - } else { - System.out.println("Hi world"); - } + + System.out.println("Hi world"); } public void conditional_with_else_contains_stale_flag_tbool_reassigned_ftbool_1( int a, bool abc, int z) { // Currently if there is another assignment, variable will not be inlined. bool ftBool = abc(); - ftBool = exp.isStaleFeature().getCachedValue(); + ftBool = true; if (!ftBool && true) { System.out.println("Hello World"); } else { diff --git a/test-resources/java/feature_flag_system_2/control/input/XPMethodChainCases.java b/test-resources/java/feature_flag_system_2/control/input/XPMethodChainCases.java index b8e213616..7ae811669 100644 --- a/test-resources/java/feature_flag_system_2/control/input/XPMethodChainCases.java +++ b/test-resources/java/feature_flag_system_2/control/input/XPMethodChainCases.java @@ -26,29 +26,23 @@ public void testDontMatchNonInstanceNested() { public static void foobar(Parameter cp) { SomeParam sp = SomeParam.create(cp); // Matches API - if (sp.isStaleFeature().getCachedValue()) { - System.out.println("!"); - } + System.out.println("!"); // Matches API - if (!sp.isStaleFeature().getCachedValue()) { - System.out.println("!!!"); - } + // Does not match API if (sp.otherFlag().getCachedValue()) { System.out.println("!!!"); } - if (sp.otherFlag().getCachedValue() && sp.isStaleFeature().getCachedValue()) { - System.out.println("!!!"); - } - if (sp.otherFlag().getCachedValue() || sp.isStaleFeature().getCachedValue()) { + if (sp.otherFlag().getCachedValue()) { System.out.println("!!!"); } + System.out.println("!!!"); // test for identifier && false - if (a && sp.isStaleFeature().getCachedValue()){ + if (a){ System.out.println("!!! Testing identifier conjunction false"); } // test for identifier || true - if (a || !sp.isStaleFeature().getCachedValue()){ + if (a){ System.out.println("!!!! Testing identifier disjunction true"); } SomeParamRev spr = SomeParamRev.create(cp); @@ -72,8 +66,8 @@ public static void foobar(Parameter cp) { System.out.println("done!"); // Matches API - cp.put(sp.isStaleFeature(), true); - cp.put(sp.isStaleFeature(), false); + + // Do not match API cp.put(sp.otherFlag(), true); @@ -82,16 +76,13 @@ public static void foobar(Parameter cp) { class TestMethodChainTest { // Matches annotation - @ParameterValue(ns = "some_long_name", key = "STALE_FLAG", val = "true") + public void testSomethingTreated() { System.out.println(); } // Matches annotation - @ParameterValue(ns = "some_long_name", key = "STALE_FLAG", val = "false") - public void testSomethingControl() { - System.out.println(); - } + // Does not match annotation @ParameterValue(ns = "some_long_name", key = "other_flag", val = "false")