Support for trailing commas & skip Comments with concrete syntax (#596)

Co-authored-by: Ameya Ketkar <[email protected]>

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
+eventlet

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)
  {

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<Vec<(&str, &str)>>,
+ 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,
  );
 }

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();

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();

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 {

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")