Add SelectorDepth linter

Add a linter which checks for selectors with large depths of
applicability. This will help catch CSS that is tightly coupled to HTML
structure and improve the performance of CSS by encouraging more
efficient selectors.

Closes sds#48

Change-Id: I0c1b8ce745881984647d21e04cdb7bbbf6b2314a
Reviewed-on: https://gerrit.causes.com/31882
Tested-by: jenkins <[email protected]>
Reviewed-by: Shane da Silva <[email protected]>

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 * Fix bug where `ColorKeyword` would incorrectly report a lint for identifiers
   containing color keywords as substrings
+* Teach scss-lint to detect selectors with large depths of applicability
 
 ## 0.13.0
 

README.md

@@ -300,6 +300,40 @@ Any lint can be disabled by using the `--exclude_linter` flag.
 
     See [Mastering Sass extends and placeholders](http://8gramgorilla.com/mastering-sass-extends-and-placeholders/).
 
+* Don't write selectors with a depth of applicability greater than 3
+
+    ```scss
+    // Incorrect - resulting CSS will have a selctor with depth of 4
+    .one .two .three > .four {
+      ...
+    }
+
+    .one .two {
+      .three > .four {
+        ...
+      }
+    }
+
+    // Correct
+    .one .two .three {
+      ...
+    }
+
+    .one .two {
+      .three {
+        ...
+      }
+    }
+    ```
+
+    Selectors with a large [depth of applicability](http://smacss.com/book/applicability)
+    lead to CSS tightly-coupled to your HTML structure, making it brittle to
+    change.
+
+    Deep selectors also come with a performance penalty, which can affect
+    rendering times, especially on mobile devices. While the default limit is
+    3, ideally it is better to use less than 3 whenever possible.
+
 * Prefer the shortest shorthand form possible for properties that support it.
 
     ```scss

config/default.yml

@@ -43,6 +43,10 @@ linters:
   PlaceholderInExtend:
     enabled: true
 
+  SelectorDepth:
+    enabled: true
+    max_depth: 3
+
   Shorthand:
     enabled: true
 

lib/scss_lint/linter/selector_depth.rb

@@ -0,0 +1,61 @@
+module SCSSLint
+  # Checks for selectors with large depths of applicability.
+  class Linter::SelectorDepth < Linter
+    include LinterRegistry
+
+    def visit_root(node)
+      @max_depth = config['max_depth']
+      @depth = 0
+      yield # Continue
+    end
+
+    def visit_rule(node)
+      old_depth = @depth
+      @depth = max_sequence_depth(node.parsed_rules, @depth)
+
+      if @depth > @max_depth
+        add_lint(node.parsed_rules || node,
+                 'Selector should have depth of applicability no greater ' <<
+                 "than #{@max_depth}, but was #{@depth}")
+      end
+
+      yield # Continue linting children
+      @depth = old_depth
+    end
+
+  private
+
+    # Find the maximum depth of all sequences in a comma sequence.
+    def max_sequence_depth(comma_sequence, current_depth)
+      # Sequence contains interpolation; assume a depth of 1
+      return current_depth + 1 unless comma_sequence
+
+      comma_sequence.members.map { |sequence| sequence_depth(sequence, current_depth) }.max
+    end
+
+    def sequence_depth(sequence, current_depth)
+      separators, simple_sequences = sequence.members.partition do |item|
+        item.is_a?(String)
+      end
+
+      parent_selectors = simple_sequences.count { |item| item.to_s == '&' }
+
+      # Take the number of simple sequences and subtract one for each sibling
+      # combinator, as these "combine" simple sequences such that they do not
+      # increase depth.
+      depth = simple_sequences.size -
+        separators.count { |item| item == '~' || item == '+'  }
+
+      if parent_selectors > 0
+        # If parent selectors are present, add the current depth for each
+        # additional parent selector.
+        depth += parent_selectors * (current_depth - 1)
+      else
+        # Otherwise this just descends from the containing selector
+        depth += current_depth
+      end
+
+      depth
+    end
+  end
+end

spec/scss_lint/linter/selector_depth_spec.rb

@@ -0,0 +1,123 @@
+require 'spec_helper'
+
+describe SCSSLint::Linter::SelectorDepth do
+  context 'when sequence has a depth of one' do
+    let(:css) { 'p {}' }
+
+    it { should_not report_lint }
+  end
+
+  context 'when sequence has a depth of three' do
+    let(:css) { 'body article p {}' }
+
+    it { should_not report_lint }
+
+    context 'and contains a nested selector' do
+      let(:css) { <<-CSS }
+        body article p {
+          i {
+            font-style: italic;
+          }
+        }
+      CSS
+
+      it { should report_lint line: 2 }
+    end
+
+    context 'and contains multiple nested selectors' do
+      let(:css) { <<-CSS }
+        body article p {
+          b {
+            font-weight: bold;
+          }
+          i {
+            font-style: italic;
+          }
+        }
+      CSS
+
+      it { should report_lint line: 2 }
+      it { should report_lint line: 5 }
+    end
+  end
+
+  context 'when sequence has a depth of four' do
+    let(:css) { 'body article p i {}' }
+
+    it { should report_lint line: 1 }
+  end
+
+  context 'when sequence is made up of adjacent sibling combinators' do
+    let(:css) { '.one + .two + .three + .four {}' }
+
+    it { should_not report_lint }
+  end
+
+  context 'when sequence is made up of general sibling combinators' do
+    let(:css) { '.one .two ~ .three ~ .four {}' }
+
+    it { should_not report_lint }
+  end
+
+  context 'when sequence contains interpolation' do
+    let(:css) { '.one #{$interpolated-string} .two .three {}' }
+
+    it { should_not report_lint }
+  end
+
+  context 'when comma sequence contains no sequences exceeding depth limit' do
+    let(:css) { <<-CSS }
+      p,
+      .one .two .three,
+      ul > li {
+      }
+    CSS
+
+    it { should_not report_lint }
+
+    context 'and a nested selector causes one of the sequences to exceed the limit' do
+      let(:css) { <<-CSS }
+        p,
+        .one .two .three,
+        ul > li {
+          .four {}
+        }
+      CSS
+
+      it { should report_lint line: 4 }
+    end
+  end
+
+  context 'when comma sequence contains a sequence exceeding the depth limit' do
+    let(:css) { <<-CSS }
+      p,
+      .one .two .three .four,
+      ul > li {
+      }
+    CSS
+
+    it { should report_lint line: 1 }
+  end
+
+  context 'when sequence contains a nested selector with a parent selector' do
+    context 'which does not exceed the depth limit' do
+      let(:css) { <<-CSS }
+        .one .two  {
+          .three & {}
+        }
+      CSS
+
+      it { should_not report_lint }
+    end
+
+    context 'which does exceed the depth limit' do
+      let(:css) { <<-CSS }
+        .one .two {
+          .three & & .four {}
+        }
+      CSS
+
+      it { should report_lint line: 2 }
+    end
+  end
+end