Fix #57 Change 'allowed-tags' rule parameter from a comma-separated l…

…ist to a regular expression
racodond · Dec 11, 2017 · 2b1d57a · 2b1d57a
1 parent 064615d
commit 2b1d57a
Show file tree

Hide file tree

Showing 10 changed files with 101 additions and 26 deletions.
diff --git a/gherkin-checks/src/main/java/org/sonar/gherkin/checks/AllowedTagsCheck.java b/gherkin-checks/src/main/java/org/sonar/gherkin/checks/AllowedTagsCheck.java
@@ -20,47 +20,53 @@
 package org.sonar.gherkin.checks;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Splitter;
-import com.google.common.collect.Lists;
 import org.sonar.check.Priority;
 import org.sonar.check.Rule;
 import org.sonar.check.RuleProperty;
-import org.sonar.plugins.gherkin.api.tree.GherkinDocumentTree;
 import org.sonar.plugins.gherkin.api.tree.TagTree;
 import org.sonar.plugins.gherkin.api.visitors.DoubleDispatchVisitorCheck;
 import org.sonar.squidbridge.annotations.SqaleConstantRemediation;
 
-import java.util.List;
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
 
 @Rule(
   key = "allowed-tags",
-  name = "Only tags from the whitelist should be used",
+  name = "Only tags matching a regular expression should be used",
   priority = Priority.MINOR,
   tags = {Tags.TAG, Tags.CONVENTION})
 @SqaleConstantRemediation("5min")
 public class AllowedTagsCheck extends DoubleDispatchVisitorCheck {
 
-  private static final String DEFAULT = "smoke,nrt";
-  private List<String> listOfAllowedTags;
+  private static final String DEFAULT = "smoke|nrt";
 
   @RuleProperty(
     key = "allowedTags",
     defaultValue = DEFAULT,
-    description = "Comma-separated list of allowed tags.")
+    description = "The regular expression that tags should match. See " + CheckUtils.LINK_TO_JAVA_REGEX_PATTERN_DOC + " for detailed regular expression syntax.")
   private String allowedTags = DEFAULT;
 
   @Override
-  public void visitGherkinDocument(GherkinDocumentTree tree) {
-    listOfAllowedTags = Lists.newArrayList(Splitter.on(",").split(allowedTags));
-    super.visitGherkinDocument(tree);
+  public void visitTag(TagTree tree) {
+    if (!tree.text().matches(allowedTags)) {
+      addPreciseIssue(tree, "Remove this tag that does not match the regular expression: \"" + allowedTags + "\"");
+    }
+    super.visitTag(tree);
   }
 
   @Override
-  public void visitTag(TagTree tree) {
-    if (!listOfAllowedTags.contains(tree.text())) {
-      addPreciseIssue(tree, "Remove this tag that is not in the whitelist.");
+  public void validateParameters() {
+    try {
+      Pattern.compile(allowedTags);
+    } catch (PatternSyntaxException exception) {
+      throw new IllegalStateException(paramErrorMessage(), exception);
     }
-    super.visitTag(tree);
+  }
+
+  private String paramErrorMessage() {
+    return CheckUtils.paramErrorMessage(
+      this.getClass(),
+      "allowedTags parameter \"" + allowedTags + "\" is not a valid regular expression.");
   }
 
   @VisibleForTesting

diff --git a/gherkin-checks/src/test/java/org/sonar/gherkin/checks/AllowedTagsCheckTest.java b/gherkin-checks/src/test/java/org/sonar/gherkin/checks/AllowedTagsCheckTest.java
@@ -22,18 +22,41 @@
 import org.junit.Test;
 import org.sonar.gherkin.checks.verifier.GherkinCheckVerifier;
 
+import static org.fest.assertions.Assertions.assertThat;
+
 public class AllowedTagsCheckTest {
 
   @Test
-  public void test_default_white_list() {
+  public void test_default_regular_expression() {
     GherkinCheckVerifier.verify(new AllowedTagsCheck(), CheckTestUtils.getTestFile("allowed-tags/allowed-tags-default.feature"));
   }
 
   @Test
-  public void test_default_custom_white_list() {
+  public void test_custom_regular_expression() {
     AllowedTagsCheck check = new AllowedTagsCheck();
-    check.setAllowedTags("mytag,yourtag");
+    check.setAllowedTags("mytag|yourtag");
     GherkinCheckVerifier.verify(check, CheckTestUtils.getTestFile("allowed-tags/allowed-tags-custom.feature"));
   }
 
+  @Test
+  public void test_composite_tags() {
+    AllowedTagsCheck check = new AllowedTagsCheck();
+    check.setAllowedTags("us:\\d+|uid:[a-zA-Z''-'\\s]{1,40}");
+    GherkinCheckVerifier.verify(check, CheckTestUtils.getTestFile("allowed-tags/allowed-tags-composite.feature"));
+  }
+
+  @Test
+  public void should_throw_an_illegal_state_exception_as_the_regular_expression_parameter_is_not_valid() {
+    try {
+      AllowedTagsCheck check = new AllowedTagsCheck();
+      check.setAllowedTags("(");
+
+      GherkinCheckVerifier.issues(check, CheckTestUtils.getTestFile("allowed-tags/allowed-tags-custom.feature")).noMore();
+
+    } catch (IllegalStateException e) {
+      assertThat(e.getMessage()).isEqualTo("Check gherkin:allowed-tags (Only tags matching a regular expression should be used): "
+        + "allowedTags parameter \"(\" is not a valid regular expression.");
+    }
+  }
+
 }
diff --git a/gherkin-checks/src/test/resources/checks/allowed-tags/allowed-tags-composite.feature b/gherkin-checks/src/test/resources/checks/allowed-tags/allowed-tags-composite.feature
@@ -0,0 +1,17 @@
+# Noncompliant [[sc=1;ec=8]] {{Remove this tag that does not match the regular expression: "us:\d+|uid:[a-zA-Z''-'\s]{1,40}"}}
+@us:abc
+Feature: My feature allowed tags composite...
+  Blabla...
+
+  # Noncompliant [[sc=3;ec=11]] {{Remove this tag that does not match the regular expression: "us:\d+|uid:[a-zA-Z''-'\s]{1,40}"}}
+  @uid:A_B
+  Scenario: Scenario 1 allowed tags composite
+    Given Blabla given...
+    When Blabla when...
+    Then Blabla then...
+
+  @us:123 @uid:UniqueID
+  Scenario: Scenario 2 allowed tags composite
+    Given Blabla given...
+    When Blabla when...
+    Then Blabla then...
diff --git a/gherkin-checks/src/test/resources/checks/allowed-tags/allowed-tags-custom.feature b/gherkin-checks/src/test/resources/checks/allowed-tags/allowed-tags-custom.feature
@@ -1,9 +1,9 @@
-# Noncompliant [[sc=1;ec=5]] {{Remove this tag that is not in the whitelist.}}
+# Noncompliant [[sc=1;ec=5]] {{Remove this tag that does not match the regular expression: "mytag|yourtag"}}
 @nrt
 Feature: My feature allowed tags custom...
   Blabla...
 
-  # Noncompliant [[sc=3;ec=23]] {{Remove this tag that is not in the whitelist.}}
+  # Noncompliant [[sc=3;ec=23]] {{Remove this tag that does not match the regular expression: "mytag|yourtag"}}
   @non-regression-test
   Scenario: Scenario 1 allowed tags custom
     Given Blabla given...

diff --git a/gherkin-checks/src/test/resources/checks/allowed-tags/allowed-tags-default.feature b/gherkin-checks/src/test/resources/checks/allowed-tags/allowed-tags-default.feature
@@ -1,9 +1,9 @@
-# Noncompliant [[sc=1;ec=7]] {{Remove this tag that is not in the whitelist.}}
+# Noncompliant [[sc=1;ec=7]] {{Remove this tag that does not match the regular expression: "smoke|nrt"}}
 @mytag
 Feature: My feature allowed tags default...
   Blabla...
 
-  # Noncompliant [[sc=3;ec=23]] {{Remove this tag that is not in the whitelist.}}
+  # Noncompliant [[sc=3;ec=23]] {{Remove this tag that does not match the regular expression: "smoke|nrt"}}
   @non-regression-test
   Scenario: Scenario 1 allowed tags default
     Given Blabla given...

diff --git a/its/ruling/projects/custom/allowed-tags/allowed-tags-composite.feature b/its/ruling/projects/custom/allowed-tags/allowed-tags-composite.feature
@@ -0,0 +1,17 @@
+# Noncompliant [[sc=1;ec=8]] {{Remove this tag that does not match the regular expression: "us:\d+|uid:[a-zA-Z''-'\s]{1,40}"}}
+@us:abc
+Feature: My feature allowed tags composite...
+  Blabla...
+
+  # Noncompliant [[sc=3;ec=11]] {{Remove this tag that does not match the regular expression: "us:\d+|uid:[a-zA-Z''-'\s]{1,40}"}}
+  @uid:A_B
+  Scenario: Scenario 1 allowed tags composite
+    Given Blabla given...
+    When Blabla when...
+    Then Blabla then...
+
+  @us:123 @uid:UniqueID
+  Scenario: Scenario 2 allowed tags composite
+    Given Blabla given...
+    When Blabla when...
+    Then Blabla then...
diff --git a/its/ruling/projects/custom/allowed-tags/allowed-tags-custom.feature b/its/ruling/projects/custom/allowed-tags/allowed-tags-custom.feature
@@ -1,9 +1,9 @@
-# Noncompliant [[sc=1;ec=5]] {{Remove this tag that is not in the whitelist.}}
+# Noncompliant [[sc=1;ec=5]] {{Remove this tag that does not match the regular expression: "mytag|yourtag"}}
 @nrt
 Feature: My feature allowed tags custom...
   Blabla...
 
-  # Noncompliant [[sc=3;ec=23]] {{Remove this tag that is not in the whitelist.}}
+  # Noncompliant [[sc=3;ec=23]] {{Remove this tag that does not match the regular expression: "mytag|yourtag"}}
   @non-regression-test
   Scenario: Scenario 1 allowed tags custom
     Given Blabla given...

diff --git a/its/ruling/projects/custom/allowed-tags/allowed-tags-default.feature b/its/ruling/projects/custom/allowed-tags/allowed-tags-default.feature
@@ -1,9 +1,9 @@
-# Noncompliant [[sc=1;ec=7]] {{Remove this tag that is not in the whitelist.}}
+# Noncompliant [[sc=1;ec=7]] {{Remove this tag that does not match the regular expression: "smoke|nrt"}}
 @mytag
 Feature: My feature allowed tags default...
   Blabla...
 
-  # Noncompliant [[sc=3;ec=23]] {{Remove this tag that is not in the whitelist.}}
+  # Noncompliant [[sc=3;ec=23]] {{Remove this tag that does not match the regular expression: "smoke|nrt"}}
   @non-regression-test
   Scenario: Scenario 1 allowed tags default
     Given Blabla given...

diff --git a/its/ruling/tests/src/test/expected/gherkin-allowed-tags.json b/its/ruling/tests/src/test/expected/gherkin-allowed-tags.json
@@ -1,4 +1,10 @@
 {
+'project:custom/allowed-tags/allowed-tags-composite.feature':[
+2,
+7,
+13,
+13,
+],
 'project:custom/allowed-tags/allowed-tags-custom.feature':[
 7,
 13,

diff --git a/its/ruling/tests/src/test/expected/gherkin-tag-naming-convention.json b/its/ruling/tests/src/test/expected/gherkin-tag-naming-convention.json
@@ -1,4 +1,10 @@
 {
+'project:custom/allowed-tags/allowed-tags-composite.feature':[
+2,
+7,
+13,
+13,
+],
 'project:custom/tag-name/tag-name-custom-ko.feature':[
 2,
 ],