Fix #197 Log only one issue for rule vendor-prefix per rule set and p…

…er property
SonarQubeCommunity · Jun 30, 2016 · de6243d · de6243d
1 parent 6c36049
commit de6243d
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 16 deletions.
diff --git a/css-checks/src/main/java/org/sonar/css/checks/VendorPrefixWithStandard.java b/css-checks/src/main/java/org/sonar/css/checks/VendorPrefixWithStandard.java
@@ -20,9 +20,13 @@
 package org.sonar.css.checks;
 
 import com.sonar.sslr.api.AstNode;
+
+import java.util.*;
+
 import org.sonar.check.Priority;
 import org.sonar.check.Rule;
 import org.sonar.css.CssCheck;
+import org.sonar.css.issue.PreciseIssue;
 import org.sonar.css.model.Property;
 import org.sonar.css.model.property.UnknownProperty;
 import org.sonar.css.parser.CssGrammar;
@@ -41,21 +45,51 @@
 @ActivatedByDefault
 public class VendorPrefixWithStandard extends CssCheck {
 
+  private final Map<String, List<AstNode>> missingStandardProperties = new HashMap<>();
+
   @Override
   public void init() {
-    subscribeTo(CssGrammar.DECLARATION);
+    subscribeTo(CssGrammar.DECLARATION, CssGrammar.SUP_DECLARATION);
+  }
+
+  @Override
+  public void visitNode(AstNode node) {
+    if (node.is(CssGrammar.SUP_DECLARATION)) {
+      missingStandardProperties.clear();
+    } else {
+      AstNode propertyNode = node.getFirstChild(CssGrammar.PROPERTY);
+      Property property = new Property(propertyNode.getTokenValue());
+      if (!(property.getStandardProperty() instanceof UnknownProperty)
+        && property.isVendorPrefixed()
+        && !isNonPrefixedPropertyDefined(node, property)) {
+        if (missingStandardProperties.get(property.getStandardProperty().getName()) != null) {
+          missingStandardProperties.get(property.getStandardProperty().getName()).add(propertyNode);
+        } else {
+          missingStandardProperties.put(
+            property.getStandardProperty().getName(),
+            new ArrayList<>(Arrays.asList(propertyNode)));
+
+        }
+      }
+    }
   }
 
   @Override
-  public void leaveNode(AstNode declarationNode) {
-    Property property = new Property(declarationNode.getFirstChild(CssGrammar.PROPERTY).getTokenValue());
-    if (!(property.getStandardProperty() instanceof UnknownProperty)
-      && property.isVendorPrefixed()
-      && !isNonPrefixedPropertyDefined(declarationNode, property)) {
-      addIssue(
-        this,
-        "Define the standard property after this vendor-prefixed property.",
-        declarationNode.getFirstChild(CssGrammar.PROPERTY));
+  public void leaveNode(AstNode node) {
+    if (node.is(CssGrammar.SUP_DECLARATION)) {
+      for (Map.Entry<String, List<AstNode>> missingStandardProperty : missingStandardProperties.entrySet()) {
+        createIssue(missingStandardProperty.getKey(), missingStandardProperty.getValue());
+      }
+    }
+  }
+
+  private void createIssue(String missingStandardProperty, List<AstNode> vendorPrefixProperties) {
+    PreciseIssue issue = addIssue(
+      this,
+      "Define the standard property after this vendor-prefixed property.",
+      vendorPrefixProperties.get(0));
+    for (int i = 1; i < vendorPrefixProperties.size(); i++) {
+      issue.addSecondaryLocation("Missing standard " + missingStandardProperty + " property", vendorPrefixProperties.get(i));
     }
   }
 

diff --git a/css-checks/src/main/resources/org/sonar/l10n/css/rules/css/vendor-prefix.html b/css-checks/src/main/resources/org/sonar/l10n/css/rules/css/vendor-prefix.html
@@ -18,6 +18,11 @@
 
 <h2>Noncompliant Code Example</h2>
 <pre>
+.mybox {
+  -ms-hyphens: none;
+  -webkit-hyphens: none;
+}
+
 .mybox {
   hyphens: none;
   -ms-hyphens: none;

diff --git a/css-checks/src/test/resources/checks/vendorprefixwithstandard.css b/css-checks/src/test/resources/checks/vendorprefixwithstandard.css
@@ -1,16 +1,42 @@
-/* Compliant: Both vendor-prefix and standard property */
+/* Compliant: Both vendor-prefix and standard properties */
 .mybox {
-  -moz-border-radius: 5px;
-  border-radius: 5px;
+  -moz-user-select: 5px;
+  user-select: 5px;
+}
+
+/* Compliant: Both vendor-prefix and standard properties */
+.mybox {
+  -moz-user-select: 5px;
+  -ms-user-select: 5px;
+  -webkit-user-select: 5px;
+  user-select: 5px;
 }
 
 /* Missing standard property */
 .mybox {
-  -moz-border-radius: 5px; /* Noncompliant ![sc=3;ec=21;el=+0]! !{Define the standard property after this vendor-prefixed property.}! */
+  -moz-user-select: 5px; /* Noncompliant ![sc=3;ec=19;el=+0]! !{Define the standard property after this vendor-prefixed property.}! */
 }
 
 /* Standard property should come after vendor-prefixed property */
 .mybox {
-  border-radius: 5px;
-  -webkit-border-radius: 5px; /* Noncompliant ![sc=3;ec=24;el=+0]! !{Define the standard property after this vendor-prefixed property.}! */
+  user-select: 5px;
+  -webkit-user-select: 5px; /* Noncompliant ![sc=3;ec=22;el=+0]! !{Define the standard property after this vendor-prefixed property.}! */
+}
+
+/* Standard property should come after all vendor-prefixed properties */
+.mybox {
+  user-select: 5px;
+  -webkit-user-select: 5px; /* Noncompliant ![sc=3;ec=22;el=+0;secondary=+1,+3]! !{Define the standard property after this vendor-prefixed property.}! */
+  -ms-user-select: 5px;
+  color: green;
+  -moz-user-select: 5px;
+}
+
+/* Standard property should come after all vendor-prefixed properties */
+.mybox {
+  -ms-user-select: 5px;
+  user-select: 5px;
+  -webkit-user-select: 5px; /* Noncompliant ![sc=3;ec=22;el=+0;secondary=+2]! !{Define the standard property after this vendor-prefixed property.}! */
+  color: green;
+  -moz-user-select: 5px;
 }