Fix #88 Add exclusions on font-family-not-ending-with-generic-font-fa…

…mily for icon fonts

.travis.yml

@@ -1,3 +1,5 @@
+sudo: required
+
 language: java
 
 jdk:

css-checks/src/main/java/org/sonar/css/checks/common/FontFamilyNotEndingWithGenericFontFamilyCheck.java

@@ -19,21 +19,34 @@
  */
 package org.sonar.css.checks.common;
 
+import com.google.common.annotations.VisibleForTesting;
 import org.sonar.check.Priority;
 import org.sonar.check.Rule;
+import org.sonar.check.RuleProperty;
+import org.sonar.css.checks.CheckList;
+import org.sonar.css.checks.CheckUtils;
 import org.sonar.css.checks.Tags;
 import org.sonar.css.model.atrule.standard.FontFace;
 import org.sonar.css.model.property.StandardProperty;
 import org.sonar.css.model.property.standard.FontFamily;
 import org.sonar.css.tree.impl.SeparatedList;
 import org.sonar.plugins.css.api.tree.Tree;
-import org.sonar.plugins.css.api.tree.css.*;
+import org.sonar.plugins.css.api.tree.css.AtRuleTree;
+import org.sonar.plugins.css.api.tree.css.DelimiterTree;
+import org.sonar.plugins.css.api.tree.css.IdentifierTree;
+import org.sonar.plugins.css.api.tree.css.PropertyDeclarationTree;
+import org.sonar.plugins.css.api.tree.css.StringTree;
+import org.sonar.plugins.css.api.tree.css.ValueCommaSeparatedListTree;
+import org.sonar.plugins.css.api.tree.css.ValueTree;
 import org.sonar.plugins.css.api.visitors.DoubleDispatchVisitorCheck;
 import org.sonar.squidbridge.annotations.ActivatedByDefault;
 import org.sonar.squidbridge.annotations.SqaleConstantRemediation;
 
 import javax.annotation.Nullable;
+import java.text.MessageFormat;
 import java.util.List;
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
 
 @Rule(
   key = "font-family-not-ending-with-generic-font-family",
@@ -44,6 +57,16 @@
 @ActivatedByDefault
 public class FontFamilyNotEndingWithGenericFontFamilyCheck extends DoubleDispatchVisitorCheck {
 
+  private static final String DEFAULT_EXCLUSIONS = "(?i)^(FontAwesome|Glyphicons Halflings|Ionicons|Genericons)$";
+  @RuleProperty(
+    key = "Exclusions",
+    description = "Regular expression of font families to exclude. See "
+      + CheckUtils.LINK_TO_JAVA_REGEX_PATTERN_DOC
+      + " for detailed regular expression syntax.",
+    defaultValue = DEFAULT_EXCLUSIONS)
+  private String exclusions = DEFAULT_EXCLUSIONS;
+
+
   @Override
   public void visitPropertyDeclaration(PropertyDeclarationTree tree) {
     if (propertyToBeChecked(tree)) {
@@ -85,18 +108,24 @@ private boolean isPotentialGenericFamily(@Nullable Tree tree) {
       return false;
     }
 
-    if (!tree.is(Tree.Kind.IDENTIFIER, Tree.Kind.LESS_VARIABLE, Tree.Kind.SCSS_VARIABLE, Tree.Kind.VARIABLE, Tree.Kind.FUNCTION)) {
+    if (!tree.is(Tree.Kind.IDENTIFIER, Tree.Kind.STRING, Tree.Kind.LESS_VARIABLE, Tree.Kind.SCSS_VARIABLE, Tree.Kind.VARIABLE, Tree.Kind.FUNCTION)) {
       return false;
     }
 
     if (tree.is(Tree.Kind.IDENTIFIER)) {
       IdentifierTree identifier = (IdentifierTree) tree;
       if (!identifier.isInterpolated()
         && !FontFamily.GENERIC_FAMILY_NAMES.contains(identifier.text().toLowerCase())
-        && !StandardProperty.COMMON_VALUES.contains(identifier.text().toLowerCase())) {
+        && !StandardProperty.COMMON_VALUES.contains(identifier.text().toLowerCase())
+        && !identifier.text().matches(exclusions)) {
         return false;
       }
     }
+
+    if (tree.is(Tree.Kind.STRING) && !((StringTree) tree).actualText().matches(exclusions)) {
+      return false;
+    }
+
     return true;
   }
 
@@ -115,4 +144,25 @@ private boolean propertyToBeChecked(PropertyDeclarationTree tree) {
     return true;
   }
 
+  @VisibleForTesting
+  void setExclusions(String exclusions) {
+    this.exclusions = exclusions;
+  }
+
+  @Override
+  public void validateParameters() {
+    try {
+      Pattern.compile(exclusions);
+    } catch (PatternSyntaxException exception) {
+      throw new IllegalStateException(paramsErrorMessage(), exception);
+    }
+  }
+
+  private String paramsErrorMessage() {
+    return CheckUtils.paramsErrorMessage(
+      this.getClass(),
+      CheckList.CSS_REPOSITORY_KEY,
+      MessageFormat.format("exclusions parameter \"{0}\" is not a valid regular expression.", exclusions));
+  }
+
 }

css-checks/src/test/java/org/sonar/css/checks/common/FontFamilyNotEndingWithGenericFontFamilyCheckTest.java

@@ -25,25 +25,55 @@
 
 import java.io.File;
 
-public class FontFamilyNotEndingWithGenericFontFamilyCheckTest {
+import static org.fest.assertions.Assertions.assertThat;
 
-  private FontFamilyNotEndingWithGenericFontFamilyCheck check = new FontFamilyNotEndingWithGenericFontFamilyCheck();
+public class FontFamilyNotEndingWithGenericFontFamilyCheckTest {
 
   @Test
   public void test_css() {
+    FontFamilyNotEndingWithGenericFontFamilyCheck check = new FontFamilyNotEndingWithGenericFontFamilyCheck();
     CssCheckVerifier.verifyCssFile(check, getTestFile("fontFamilyNotEndingWithGenericFontFamily.css"));
   }
 
   @Test
   public void test_less() {
+    FontFamilyNotEndingWithGenericFontFamilyCheck check = new FontFamilyNotEndingWithGenericFontFamilyCheck();
     CssCheckVerifier.verifyLessFile(check, getTestFile("fontFamilyNotEndingWithGenericFontFamily.less"));
   }
 
   @Test
   public void test_scss() {
+    FontFamilyNotEndingWithGenericFontFamilyCheck check = new FontFamilyNotEndingWithGenericFontFamilyCheck();
     CssCheckVerifier.verifyScssFile(check, getTestFile("fontFamilyNotEndingWithGenericFontFamily.scss"));
   }
 
+  @Test
+  public void test_with_default_exclusions() {
+    FontFamilyNotEndingWithGenericFontFamilyCheck check = new FontFamilyNotEndingWithGenericFontFamilyCheck();
+    CssCheckVerifier.verifyCssFile(check, getTestFile("fontFamilyNotEndingWithGenericFontFamilyDefaultExclusions.css"));
+  }
+
+  @Test
+  public void test_with_custom_format() {
+    FontFamilyNotEndingWithGenericFontFamilyCheck check = new FontFamilyNotEndingWithGenericFontFamilyCheck();
+    check.setExclusions("^My-.+$");
+    CssCheckVerifier.verifyCssFile(check, getTestFile("fontFamilyNotEndingWithGenericFontFamilyCustomExclusions.css"));
+  }
+
+  @Test
+  public void should_throw_an_illegal_state_exception_as_the_exclusions_parameter_is_not_valid() {
+    try {
+      FontFamilyNotEndingWithGenericFontFamilyCheck check = new FontFamilyNotEndingWithGenericFontFamilyCheck();
+      check.setExclusions("(");
+
+      CssCheckVerifier.issuesOnCssFile(check, getTestFile("fontFamilyNotEndingWithGenericFontFamily.css")).noMore();
+
+    } catch (IllegalStateException e) {
+      assertThat(e.getMessage()).isEqualTo("Check css:font-family-not-ending-with-generic-font-family (font-family properties should end with a generic font family): "
+        + "exclusions parameter \"(\" is not a valid regular expression.");
+    }
+  }
+
   private File getTestFile(String fileName) {
     return CheckTestUtils.getCommonTestFile("font-family-not-ending-with-generic-font-family/" + fileName);
   }

css-checks/src/test/resources/checks/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyCustomExclusions.css

@@ -0,0 +1,5 @@
+.mybox1 {
+  font-family: FontAwesome; /* Noncompliant ![sc=16;ec=27;el=+0]! !{Add a generic font family at the end of the declaration.}! */
+  font-family: My-Icons;
+  font-family: my-icons; /* Noncompliant ![sc=16;ec=24;el=+0]! !{Add a generic font family at the end of the declaration.}! */
+}

css-checks/src/test/resources/checks/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyDefaultExclusions.css

@@ -0,0 +1,13 @@
+.mybox1 {
+  font-family: FontAwesome;
+  font-family: "FontAwesome";
+  font-family: "Glyphicons Halflings";
+  font-family: Ionicons;
+  font-family: "Ionicons";
+  font-family: Genericons;
+  font-family: "Genericons";
+  font-family: genericons;
+  font-family: "genericons";
+  font-family: MyIcons; /* Noncompliant ![sc=16;ec=23;el=+0]! !{Add a generic font family at the end of the declaration.}! */
+  font-family: "MyIcons"; /* Noncompliant ![sc=16;ec=25;el=+0]! !{Add a generic font family at the end of the declaration.}! */
+}

its/ruling/projects/custom/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyCustomExclusions.css

@@ -0,0 +1,5 @@
+.mybox1 {
+  font-family: FontAwesome; /* Noncompliant ![sc=16;ec=27;el=+0]! !{Add a generic font family at the end of the declaration.}! */
+  font-family: My-Icons;
+  font-family: my-icons; /* Noncompliant ![sc=16;ec=24;el=+0]! !{Add a generic font family at the end of the declaration.}! */
+}

its/ruling/projects/custom/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyDefaultExclusions.css

@@ -0,0 +1,13 @@
+.mybox1 {
+  font-family: FontAwesome;
+  font-family: "FontAwesome";
+  font-family: "Glyphicons Halflings";
+  font-family: Ionicons;
+  font-family: "Ionicons";
+  font-family: Genericons;
+  font-family: "Genericons";
+  font-family: genericons;
+  font-family: "genericons";
+  font-family: MyIcons; /* Noncompliant ![sc=16;ec=23;el=+0]! !{Add a generic font family at the end of the declaration.}! */
+  font-family: "MyIcons"; /* Noncompliant ![sc=16;ec=25;el=+0]! !{Add a generic font family at the end of the declaration.}! */
+}

its/ruling/tests/src/test/expected/css-font-family-not-ending-with-generic-font-family.json

@@ -14,6 +14,14 @@
 11,
 12,
 ],
+'project:custom/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyCustomExclusions.css':[
+3,
+4,
+],
+'project:custom/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyDefaultExclusions.css':[
+11,
+12,
+],
 'project:custom/common/formatting/delimiterSeparatedList.css':[
 2,
 ],

its/ruling/tests/src/test/expected/css-line-length.json

@@ -84,6 +84,14 @@
 11,
 12,
 ],
+'project:custom/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyCustomExclusions.css':[
+2,
+4,
+],
+'project:custom/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyDefaultExclusions.css':[
+11,
+12,
+],
 'project:custom/common/fontface/basic.css':[
 58,
 63,

its/ruling/tests/src/test/expected/css-single-quotes.json

@@ -30,6 +30,14 @@
 'project:custom/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamily.css':[
 6,
 ],
+'project:custom/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyDefaultExclusions.css':[
+3,
+4,
+6,
+8,
+10,
+12,
+],
 'project:custom/common/known-properties/knownProperties.css':[
 38,
 68,

its/ruling/tests/src/test/expected/css-unquoted-font-family-names.json

@@ -35,6 +35,18 @@
 11,
 12,
 ],
+'project:custom/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyCustomExclusions.css':[
+2,
+3,
+4,
+],
+'project:custom/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyDefaultExclusions.css':[
+2,
+5,
+7,
+9,
+11,
+],
 'project:custom/common/inliningFontFiles.css':[
 12,
 19,