Fix #68 Add a rule that forbids using types in selectors

Author: Nico Heller

README.md

@@ -155,6 +155,7 @@ It computes the complexity/rule, meaning the average number of selectors per rul
 * The number of web fonts should be reduced
 * There should be one single declaration per line
 * Trailing zeros for numeric values should be removed
+* Types in selectors should be removed
 * Underscore hack should not be used
 * Units for zero length values should be removed
 * Universal selector should not be used as key part

css-checks/src/main/java/org/sonar/css/checks/CheckList.java

@@ -187,6 +187,7 @@ private static List<Class> getCommonChecks() {
       UnknownPropertyCheck.class,
       UnknownPseudoCheck.class,
       UnknownTypeSelectorCheck.class,
+      TypeSelectorCheck.class,
       UnquotedFontFamilyNamesCheck.class,
       UseShorthandPropertyCheck.class,
       ValidatePropertyValueCheck.class,
@@ -266,6 +267,7 @@ public static List<Class> getEmbeddedCssChecks() {
       UnknownPropertyCheck.class,
       UnknownPseudoCheck.class,
       UnknownTypeSelectorCheck.class,
+      TypeSelectorCheck.class,
       UnquotedFontFamilyNamesCheck.class,
       UseShorthandPropertyCheck.class,
       ValidatePropertyValueCheck.class,

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

@@ -0,0 +1,71 @@
+/*
+ * SonarQube CSS / SCSS / Less Analyzer
+ * Copyright (C) 2013-2017 David RACODON
+ * mailto: david.racodon@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+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.Tags;
+import org.sonar.plugins.css.api.tree.css.TypeSelectorTree;
+import org.sonar.plugins.css.api.visitors.DoubleDispatchVisitorCheck;
+import org.sonar.squidbridge.annotations.SqaleConstantRemediation;
+
+@Rule(
+  key = "type-selector",
+  name = "Types in selectors should be removed",
+  priority = Priority.MAJOR,
+  tags = {Tags.DESIGN})
+@SqaleConstantRemediation("30min")
+public class TypeSelectorCheck extends DoubleDispatchVisitorCheck {
+
+  private static final String DEFAULT_ALLOWED_TYPES = "";
+  @RuleProperty(
+    key = "Exclusions",
+    description = "A list of types that are allowed in selectors (separated by \",\")",
+    defaultValue = DEFAULT_ALLOWED_TYPES)
+  private String allowedTypes = DEFAULT_ALLOWED_TYPES;
+
+  @Override
+  public void visitTypeSelector(TypeSelectorTree tree) {
+    if (!tree.identifier().isInterpolated()
+      && !isAllowedType(tree.identifier().text())) {
+      addPreciseIssue(
+        tree.identifier(),
+        "Remove the \"" + tree.identifier().text() + "\" type from this selector.");
+    }
+    super.visitTypeSelector(tree);
+  }
+
+  private boolean isAllowedType(String type) {
+    for (String allowedType : allowedTypes.split(",")) {
+      if (allowedType.trim().equalsIgnoreCase(type)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  @VisibleForTesting
+  void setAllowedTypes(String allowedTypes) {
+    this.allowedTypes = allowedTypes;
+  }
+
+}

css-checks/src/main/resources/org/sonar/css/checks/l10n/common/template/type-selector.html

@@ -0,0 +1,37 @@
+<p>
+    Using types in selectors reduces the flexibility and narrows the applicability of CSS styles. Classes should be
+    preferred as they improve the understandability by giving elements a context-specific name.
+    For example:
+</p>
+<pre>
+.myBox span {
+  color: black;
+}
+</pre>
+<p>
+    This styling can only be applied to a limited subset of all containers with the .someThing class.
+    Furthermore the reader cannot identify what the span actually represents.
+    An alternative using a class instead of a type does not have this issue:
+</p>
+<pre>
+.myBox .headline {
+  color: black;
+}
+</pre>
+<p>The intent is clear and the contextual understandability improved dramatically.</p>
+
+
+<h2>Noncompliant Code Example</h2>
+<pre>
+span {
+  display: block;
+}
+
+.mybox div {
+  color: red;
+}
+</pre>
+
+<p>
+    You can exclude certain types via the configuration of this rule as e.g. the anchor type is a valid exception from this rule as it should always denote a link of some sort.
+</p>

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

@@ -0,0 +1,57 @@
+/*
+ * SonarQube CSS / SCSS / Less Analyzer
+ * Copyright (C) 2013-2017 David RACODON
+ * mailto: david.racodon@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.css.checks.common;
+
+import org.junit.Test;
+import org.sonar.css.checks.CheckTestUtils;
+import org.sonar.css.checks.verifier.CssCheckVerifier;
+
+public class TypeSelectorCheckTest {
+
+  @Test
+  public void test_css_without_excludes() {
+    CssCheckVerifier.verifyCssFile(new TypeSelectorCheck(), CheckTestUtils.getCommonTestFile("type-selector/typeSelector.css"));
+  }
+
+  @Test
+  public void test_css_with_excludes() {
+    TypeSelectorCheck check = new TypeSelectorCheck();
+    check.setAllowedTypes("a, span");
+    CssCheckVerifier.verifyCssFile(check, CheckTestUtils.getCommonTestFile("type-selector/typeSelectorExcluded.css"));
+  }
+
+  @Test
+  public void test_css_with_excludes_case_insensitive() {
+    TypeSelectorCheck check = new TypeSelectorCheck();
+    check.setAllowedTypes("A, SpAn");
+    CssCheckVerifier.verifyCssFile(check, CheckTestUtils.getCommonTestFile("type-selector/typeSelectorExcluded.css"));
+  }
+
+  @Test
+  public void test_less_without_excludes() {
+    CssCheckVerifier.verifyLessFile(new TypeSelectorCheck(), CheckTestUtils.getCommonTestFile("type-selector/typeSelector.less"));
+  }
+
+  @Test
+  public void test_scss_without_excludes() {
+    CssCheckVerifier.verifyScssFile(new TypeSelectorCheck(), CheckTestUtils.getCommonTestFile("type-selector/typeSelector.scss"));
+  }
+
+}

css-checks/src/test/resources/checks/common/type-selector/typeSelector.css

@@ -0,0 +1,19 @@
+.mybox {
+  display: block;
+}
+
+div { /* Noncompliant ![sc=1;ec=4;el=+0]! !{Remove the "div" type from this selector.}! */
+  display: block;
+}
+
+#mybox {
+  display: block;
+}
+
+.mybox span { /* Noncompliant ![sc=8;ec=12;el=+0]! !{Remove the "span" type from this selector.}! */
+  color: red;
+}
+
+.mybox #go > a {/* Noncompliant ![sc=14;ec=15;el=+0]! !{Remove the "a" type from this selector.}! */
+  color: red;
+}

css-checks/src/test/resources/checks/common/type-selector/typeSelector.less

@@ -0,0 +1,19 @@
+.mybox {
+  display: block;
+}
+
+div { /* Noncompliant ![sc=1;ec=4;el=+0]! !{Remove the "div" type from this selector.}! */
+  display: block;
+}
+
+#mybox {
+  display: block;
+}
+
+.mybox span { /* Noncompliant ![sc=8;ec=12;el=+0]! !{Remove the "span" type from this selector.}! */
+  color: red;
+}
+
+.mybox #go > a {/* Noncompliant ![sc=14;ec=15;el=+0]! !{Remove the "a" type from this selector.}! */
+  color: red;
+}

css-checks/src/test/resources/checks/common/type-selector/typeSelector.scss

@@ -0,0 +1,19 @@
+.mybox {
+  display: block;
+}
+
+div { /* Noncompliant ![sc=1;ec=4;el=+0]! !{Remove the "div" type from this selector.}! */
+  display: block;
+}
+
+#mybox {
+  display: block;
+}
+
+.mybox span { /* Noncompliant ![sc=8;ec=12;el=+0]! !{Remove the "span" type from this selector.}! */
+  color: red;
+}
+
+.mybox #go > a {/* Noncompliant ![sc=14;ec=15;el=+0]! !{Remove the "a" type from this selector.}! */
+  color: red;
+}

css-checks/src/test/resources/checks/common/type-selector/typeSelectorExcluded.css

@@ -0,0 +1,19 @@
+.mybox {
+  display: block;
+}
+
+div { /* Noncompliant ![sc=1;ec=4;el=+0]! !{Remove the "div" type from this selector.}! */
+  display: block;
+}
+
+#mybox {
+  display: block;
+}
+
+.mybox span {
+  color: red;
+}
+
+.mybox #go > a {
+  color: red;
+}

its/ruling/projects/custom/common/type-selector/typeSelector.css

@@ -0,0 +1,19 @@
+.mybox {
+  display: block;
+}
+
+div { /* Noncompliant ![sc=1;ec=4;el=+0]! !{Remove the "div" type from this selector.}! */
+  display: block;
+}
+
+#mybox {
+  display: block;
+}
+
+.mybox span { /* Noncompliant ![sc=8;ec=12;el=+0]! !{Remove the "span" type from this selector.}! */
+  color: red;
+}
+
+.mybox #go > a {/* Noncompliant ![sc=14;ec=15;el=+0]! !{Remove the "a" type from this selector.}! */
+  color: red;
+}