From fead43484bcb448a23e4e704fd65e3f53e3aeb0b Mon Sep 17 00:00:00 2001 From: David RACODON Date: Sat, 17 Mar 2018 13:14:09 +0100 Subject: [PATCH] Fix #88 Add exclusions on font-family-not-ending-with-generic-font-family for icon fonts --- .travis.yml | 2 + ...lyNotEndingWithGenericFontFamilyCheck.java | 56 ++++++++++++++++++- ...tEndingWithGenericFontFamilyCheckTest.java | 34 ++++++++++- ...gWithGenericFontFamilyCustomExclusions.css | 5 ++ ...WithGenericFontFamilyDefaultExclusions.css | 13 +++++ ...gWithGenericFontFamilyCustomExclusions.css | 5 ++ ...WithGenericFontFamilyDefaultExclusions.css | 13 +++++ ...y-not-ending-with-generic-font-family.json | 8 +++ .../src/test/expected/css-line-length.json | 8 +++ .../src/test/expected/css-single-quotes.json | 8 +++ .../css-unquoted-font-family-names.json | 12 ++++ 11 files changed, 159 insertions(+), 5 deletions(-) create mode 100644 css-checks/src/test/resources/checks/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyCustomExclusions.css create mode 100644 css-checks/src/test/resources/checks/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyDefaultExclusions.css create mode 100644 its/ruling/projects/custom/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyCustomExclusions.css create mode 100644 its/ruling/projects/custom/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyDefaultExclusions.css diff --git a/.travis.yml b/.travis.yml index 486e0c07..61583a18 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,3 +1,5 @@ +sudo: required + language: java jdk: diff --git a/css-checks/src/main/java/org/sonar/css/checks/common/FontFamilyNotEndingWithGenericFontFamilyCheck.java b/css-checks/src/main/java/org/sonar/css/checks/common/FontFamilyNotEndingWithGenericFontFamilyCheck.java index 70a22e4f..a81cce05 100644 --- a/css-checks/src/main/java/org/sonar/css/checks/common/FontFamilyNotEndingWithGenericFontFamilyCheck.java +++ b/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,7 +108,7 @@ 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; } @@ -93,10 +116,16 @@ private boolean isPotentialGenericFamily(@Nullable Tree tree) { 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)); + } + } diff --git a/css-checks/src/test/java/org/sonar/css/checks/common/FontFamilyNotEndingWithGenericFontFamilyCheckTest.java b/css-checks/src/test/java/org/sonar/css/checks/common/FontFamilyNotEndingWithGenericFontFamilyCheckTest.java index 433ceddf..423bc213 100644 --- a/css-checks/src/test/java/org/sonar/css/checks/common/FontFamilyNotEndingWithGenericFontFamilyCheckTest.java +++ b/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); } diff --git a/css-checks/src/test/resources/checks/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyCustomExclusions.css b/css-checks/src/test/resources/checks/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyCustomExclusions.css new file mode 100644 index 00000000..9b6bfd1e --- /dev/null +++ b/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.}! */ +} diff --git a/css-checks/src/test/resources/checks/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyDefaultExclusions.css b/css-checks/src/test/resources/checks/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyDefaultExclusions.css new file mode 100644 index 00000000..8f0014dc --- /dev/null +++ b/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.}! */ +} diff --git a/its/ruling/projects/custom/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyCustomExclusions.css b/its/ruling/projects/custom/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyCustomExclusions.css new file mode 100644 index 00000000..9b6bfd1e --- /dev/null +++ b/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.}! */ +} diff --git a/its/ruling/projects/custom/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyDefaultExclusions.css b/its/ruling/projects/custom/common/font-family-not-ending-with-generic-font-family/fontFamilyNotEndingWithGenericFontFamilyDefaultExclusions.css new file mode 100644 index 00000000..8f0014dc --- /dev/null +++ b/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.}! */ +} diff --git a/its/ruling/tests/src/test/expected/css-font-family-not-ending-with-generic-font-family.json b/its/ruling/tests/src/test/expected/css-font-family-not-ending-with-generic-font-family.json index 47b5b5a5..6d4327a7 100644 --- a/its/ruling/tests/src/test/expected/css-font-family-not-ending-with-generic-font-family.json +++ b/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, ], diff --git a/its/ruling/tests/src/test/expected/css-line-length.json b/its/ruling/tests/src/test/expected/css-line-length.json index 4979446e..8b351b2b 100644 --- a/its/ruling/tests/src/test/expected/css-line-length.json +++ b/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, diff --git a/its/ruling/tests/src/test/expected/css-single-quotes.json b/its/ruling/tests/src/test/expected/css-single-quotes.json index 433b7cc0..20a2a231 100644 --- a/its/ruling/tests/src/test/expected/css-single-quotes.json +++ b/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, diff --git a/its/ruling/tests/src/test/expected/css-unquoted-font-family-names.json b/its/ruling/tests/src/test/expected/css-unquoted-font-family-names.json index cade2fb0..f08cb114 100644 --- a/its/ruling/tests/src/test/expected/css-unquoted-font-family-names.json +++ b/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,