Add DuplicateRoot linter

Closes sds#109

Change-Id: I8f7a16aadfff132385cd18fcbe03050f0f97d63f
Reviewed-on: http://gerrit.causes.com/37417
Tested-by: jenkins <[email protected]>
Reviewed-by: Shane da Silva <[email protected]>

CHANGELOG.md

@@ -14,6 +14,8 @@
   reported for braces that aren't part of declarations
 * Teach `Compass::PropertyWithMixin` to prefer `inline-block` mixin over
   `display: inline-block`
+* Add `DuplicateRoot` linter which checks for identical rules used at as root
+  selectors in a document
 
 ## 0.22.0
 

config/default.yml

@@ -21,6 +21,9 @@ linters:
   DuplicateProperty:
     enabled: true
 
+  DuplicateRoot:
+    enabled: true
+
   EmptyLineBetweenBlocks:
     enabled: true
     ignore_single_line_blocks: true

lib/scss_lint/linter/README.md

@@ -10,6 +10,7 @@ Below is a list of linters supported by `scss-lint`, ordered alphabetically.
 * [DebugStatement](#debugstatement)
 * [DeclarationOrder](#declarationorder)
 * [DuplicateProperty](#duplicateproperty)
+* [DuplicateRoot](#duplicateroot)
 * [EmptyLineBetweenBlocks](#emptylinebetweenblocks)
 * [EmptyRule](#emptyrule)
 * [FinalNewline](#finalnewline)
@@ -185,6 +186,40 @@ more helpful since you don't want to clutter your CSS with fallbacks.
 Otherwise, you may want to consider disabling this check in your
 `.scss-lint.yml` configuration.
 
+## DuplicateRoot
+
+Reports when you define the same root selector twice in a single sheet.
+
+**Bad**
+```scss
+h1 {
+  margin: 10px;
+}
+.baz {
+  color: red;
+}
+// Second copy of h1 rule
+h1 {
+  text-transform: uppercase;
+  border: 0;
+}
+```
+
+Combining duplicate selectors can result in an easier to read sheet, but
+occasionally the root rules may be purposely duplicated to set precedence
+after a rule with the same CSS specificity.
+
+```scss
+h1 {
+  margin: 10px;
+  text-transform: uppercase;
+  border: 0;
+}
+.baz {
+  color: red;
+}
+```
+
 ## EmptyLineBetweenBlocks
 
 Separate rule, function, and mixin declarations with empty lines.

lib/scss_lint/linter/duplicate_root.rb

@@ -0,0 +1,23 @@
+module SCSSLint
+  # Checks identical root selectors.
+  class Linter::DuplicateRoot < Linter
+    include LinterRegistry
+
+    def visit_root(node)
+      # Root rules are evaluated per document, so use new hashes for eash file
+      @roots = Hash.new
+      yield # Continue linting children
+    end
+
+    def visit_rule(node)
+      # Check to see if we've seen this rule before
+      if @roots.has_key?(node.rule)
+        add_lint(node.line,
+                 "Root #{node.rule} already seen at #{@roots[node.rule]}")
+      else
+        @roots[node.rule] = node.line
+      end
+      return # Only check one level deep
+    end
+  end
+end

spec/scss_lint/linter/duplicate_root_spec.rb

@@ -0,0 +1,146 @@
+require 'spec_helper'
+
+describe SCSSLint::Linter::DuplicateRoot do
+  context 'when single root' do
+    let(:css) { <<-CSS }
+      p {
+      }
+    CSS
+
+    it { should_not report_lint }
+  end
+
+  context 'when different roots' do
+    let(:css) { <<-CSS }
+      p {
+        background: #000;
+        margin: 5px;
+      }
+      a {
+        background: #000;
+        margin: 5px;
+      }
+    CSS
+
+    it { should_not report_lint }
+  end
+
+  context 'when different roots with matching inner rules' do
+    let(:css) { <<-CSS }
+      p {
+        background: #000;
+        margin: 5px;
+        .foo {
+          color: red;
+        }
+      }
+      a {
+        background: #000;
+        margin: 5px;
+        .foo {
+          color: red;
+        }
+      }
+    CSS
+
+    it { should_not report_lint }
+  end
+
+  context 'when multi-selector roots and parital rule match' do
+    let(:css) { <<-CSS }
+      p, a {
+        background: #000;
+        margin: 5px;
+        .foo {
+          color: red;
+        }
+      }
+      a {
+        background: #000;
+        margin: 5px;
+        .foo {
+          color: red;
+        }
+      }
+    CSS
+
+    it { should_not report_lint }
+  end
+
+  context 'when nested and unnested selectors match' do
+    let(:css) { <<-CSS }
+      a.current {
+        background: #000;
+        margin: 5px;
+        .foo {
+          color: red;
+        }
+      }
+      a {
+        &.current {
+          background: #000;
+          margin: 5px;
+          .foo {
+            color: red;
+          }
+        }
+      }
+    CSS
+
+    it { should_not report_lint }
+  end
+
+  context 'when same class roots' do
+    let(:css) { <<-CSS }
+      .warn {
+        font-weight: bold;
+      }
+      .warn {
+        color: #f00;
+        @extend .warn;
+        a {
+          color: #ccc;
+        }
+      }
+    CSS
+
+    it { should report_lint }
+  end
+
+  context 'when same compond selector roots' do
+    let(:css) { <<-CSS }
+      .warn .alert {
+        font-weight: bold;
+      }
+      .warn .alert {
+        color: #f00;
+        @extend .warn;
+        a {
+          color: #ccc;
+        }
+      }
+    CSS
+
+    it { should report_lint }
+  end
+
+  context 'when same class roots separated by another class' do
+    let(:css) { <<-CSS }
+      .warn {
+        font-weight: bold;
+      }
+      .foo {
+        color: red;
+      }
+      .warn {
+        color: #f00;
+        @extend .warn;
+        a {
+          color: #ccc;
+        }
+      }
+    CSS
+
+    it { should report_lint }
+  end
+end