Eliminate more @extend output redundancy.

Closes #317

doc-src/SASS_CHANGELOG.md

@@ -108,6 +108,9 @@ that make use of `@media` and other directives dynamically.
 * In certain cases, redundant selectors used to be created as a result of a
   single rule having multiple `@extend`s. That redundancy has been eliminated.
 
+* Redundant selectors were also sometimes created by nested selectors
+  using `@extend`. That redundancy has been eliminated as well.
+
 * There is now much more comprehensive support for using `@extend` alongside
   CSS3 selector combinators (`+`, `~`, and `>`). These combinators will now be
   merged as much as possible.

lib/sass/selector/sequence.rb

@@ -76,14 +76,15 @@ def resolve_parent_refs(super_seq)
       #   These correspond to a {CommaSequence}'s {CommaSequence#members members array}.
       # @see CommaSequence#do_extend
       def do_extend(extends, seen = Set.new)
-        paths = Sass::Util.paths(members.map do |sseq_or_op|
-            next [[sseq_or_op]] unless sseq_or_op.is_a?(SimpleSequence)
-            extended = sseq_or_op.do_extend(extends, seen)
-            choices = extended.map {|seq| seq.members}
-            choices.unshift([sseq_or_op]) unless extended.any? {|seq| seq.superselector?(sseq_or_op)}
-            choices
-          end)
-        Sass::Util.flatten(paths.map {|path| weave(path)}, 1).map {|p| Sequence.new(p)}
+        extended_not_expanded = members.map do |sseq_or_op|
+          next [[sseq_or_op]] unless sseq_or_op.is_a?(SimpleSequence)
+          extended = sseq_or_op.do_extend(extends, seen)
+          choices = extended.map {|seq| seq.members}
+          choices.unshift([sseq_or_op]) unless extended.any? {|seq| seq.superselector?(sseq_or_op)}
+          choices
+        end
+        weaves = Sass::Util.paths(extended_not_expanded).map {|path| weave(path)}
+        Sass::Util.flatten(trim(weaves), 1).map {|p| Sequence.new(p)}
       end
 
       # Returns whether or not this selector matches all elements
@@ -171,13 +172,13 @@ def subweave(seq1, seq2)
         lcs = Sass::Util.lcs(seq2, seq1) do |s1, s2|
           next s1 if s1 == s2
           next unless s1.first.is_a?(SimpleSequence) && s2.first.is_a?(SimpleSequence)
-          next s2 if subweave_superselector?(s1, s2)
-          next s1 if subweave_superselector?(s2, s1)
+          next s2 if parent_superselector?(s1, s2)
+          next s1 if parent_superselector?(s2, s1)
         end
 
         diff = [[init]]
         until lcs.empty?
-          diff << chunks(seq1, seq2) {|s| subweave_superselector?(s.first, lcs.first)} << [lcs.shift]
+          diff << chunks(seq1, seq2) {|s| parent_superselector?(s.first, lcs.first)} << [lcs.shift]
           seq1.shift
           seq2.shift
         end
@@ -254,9 +255,9 @@ def merge_final_ops(seq1, seq2, res = [])
           sel1 = seq1.pop
           sel2 = seq2.pop
           if op1 == '~' && op2 == '~'
-            if subweave_superselector?([sel1], [sel2])
+            if sel1.superselector?(sel2)
               res.unshift sel2, '~'
-            elsif subweave_superselector?([sel2], [sel1])
+            elsif sel2.superselector?(sel1)
               res.unshift sel1, '~'
             else
               merged = sel1.unify(sel2.members)
@@ -273,7 +274,7 @@ def merge_final_ops(seq1, seq2, res = [])
               tilde_sel, plus_sel = sel2, sel1
             end
 
-            if subweave_superselector?([tilde_sel], [plus_sel])
+            if tilde_sel.superselector?(plus_sel)
               res.unshift plus_sel, '+'
             else
               merged = plus_sel.unify(tilde_sel.members)
@@ -297,11 +298,11 @@ def merge_final_ops(seq1, seq2, res = [])
           end
           return merge_final_ops(seq1, seq2, res)
         elsif op1
-          seq2.pop if op1 == '>' && seq2.last && subweave_superselector?([seq2.last], [seq1.last])
+          seq2.pop if op1 == '>' && seq2.last && seq2.last.superselector?(seq1.last)
           res.unshift seq1.pop, op1
           return merge_final_ops(seq1, seq2, res)
         else # op2
-          seq1.pop if op2 == '>' && seq1.last && subweave_superselector?([seq1.last], [seq2.last])
+          seq1.pop if op2 == '>' && seq1.last && seq1.last.superselector?(seq2.last)
           res.unshift seq2.pop, op2
           return merge_final_ops(seq1, seq2, res)
         end
@@ -360,36 +361,80 @@ def group_selectors(seq)
         return newseq
       end
 
-      # Given two sequences of simple selectors, returns whether `sseq1` is a
-      # superselector of `sseq2`; that is, whether `sseq1` matches every element
-      # `sseq2` matches.
-      #
-      # Both `sseq1` and `sseq2` are of the form
-      # `SimpleSelector (String SimpleSelector)* String*`, although selectors
-      # with a trailing operator are considered to be neither superselectors nor
-      # subselectors.
+      # Given two selector sequences, returns whether `seq1` is a
+      # superselector of `seq2`; that is, whether `seq1` matches every
+      # element `seq2` matches.
       #
-      # @param sseq1 [Array<SimpleSequence or String>]
-      # @param sseq2 [Array<SimpleSequence or String>]
+      # @param seq1 [Array<SimpleSequence or String>]
+      # @param seq2 [Array<SimpleSequence or String>]
       # @return [Boolean]
-      def subweave_superselector?(sseq1, sseq2)
-        sseq1 = sseq1.reject {|e| e == "\n"}
-        sseq2 = sseq2.reject {|e| e == "\n"}
-        # Selectors with trailing operators are neither superselectors nor
-        # subselectors.
-        return if sseq1.last.is_a?(String) || sseq2.last.is_a?(String)
-        if sseq1.size > 1
-          # More complex selectors are never superselectors of less complex ones
-          return unless sseq2.size > 1
+      def _superselector?(seq1, seq2)
+        seq1 = seq1.reject {|e| e == "\n"}
+        seq2 = seq2.reject {|e| e == "\n"}
+        # Selectors with leading or trailing operators are neither
+        # superselectors nor subselectors.
+        return if seq1.last.is_a?(String) || seq2.last.is_a?(String) ||
+          seq1.first.is_a?(String) || seq2.first.is_a?(String)
+        # More complex selectors are never superselectors of less complex ones
+        return if seq1.size > seq2.size
+        return seq1.first.superselector?(seq2.last) if seq1.size == 1
+
+        _, si = Sass::Util.enum_with_index(seq2).find do |e, i|
+          return if i == seq2.size - 1
+          next if e.is_a?(String)
+          seq1.first.superselector?(e)
+        end
+        return unless si
+
+        if seq1[1].is_a?(String)
+          return unless seq2[si+1].is_a?(String)
           # .foo ~ .bar is a superselector of .foo + .bar
-          return unless sseq1[1] == "~" ? sseq2[1] != ">" : sseq2[1] == sseq1[1]
-          return unless sseq1.first.superselector?(sseq2.first)
-          return subweave_superselector?(sseq1[2..-1], sseq2[2..-1])
-        elsif sseq2.size > 1
-          return true if sseq2[1] == ">" && sseq1.first.superselector?(sseq2.first)
-          return subweave_superselector?(sseq1, sseq2[2..-1])
+          return unless seq1[1] == "~" ? seq2[si+1] != ">" : seq1[1] == seq2[si+1]
+          return _superselector?(seq1[2..-1], seq2[si+2..-1])
+        elsif seq2[si+1].is_a?(String)
+          return unless seq2[si+1] == ">"
+          return _superselector?(seq1[1..-1], seq2[si+2..-1])
         else
-          sseq1.first.superselector?(sseq2.first)
+          return _superselector?(seq1[1..-1], seq2[si+1..-1])
+        end
+      end
+
+      # Like \{#_superselector?}, but compares the selectors in the
+      # context of parent selectors, as though they shared an implicit
+      # base simple selector. For example, `B` is not normally a
+      # superselector of `B A`, since it doesn't match `A` elements.
+      # However, it is a parent superselector, since `B X` is a
+      # superselector of `B A X`.
+      #
+      # @param seq1 [Array<SimpleSequence or String>]
+      # @param seq2 [Array<SimpleSequence or String>]
+      # @return [Boolean]
+      def parent_superselector?(seq1, seq2)
+        base = Sass::Selector::SimpleSequence.new([Sass::Selector::Placeholder.new('<temp>')])
+        _superselector?(seq1 + [base], seq2 + [base])
+      end
+
+      # Removes redundant selectors from between multiple lists of
+      # selectors. This takes a list of lists of selector sequences;
+      # each individual list is assumed to have no redundancy within
+      # itself. A selector is only removed if it's redundant with a
+      # selector in another list.
+      #
+      # "Redundant" here means that one selector is a superselector of
+      # the other. The more specific selector is removed.
+      #
+      # @param seqses [Array<Array<Array<SimpleSequence or String>>>]
+      # @return [Array<Array<Array<SimpleSequence or String>>>]
+      def trim(seqses)
+        # This is n^2 on the sequences, but only comparing between
+        # separate sequences should limit the quadratic behavior.
+        seqses.map do |seqs1|
+          seqs1.reject do |seq1|
+            seqses.any? do |seqs2|
+              next if seqs1.object_id == seqs2.object_id
+              seqs2.any? {|seq2| _superselector?(seq2, seq1)}
+            end
+          end
         end
       end
 
@@ -400,6 +445,19 @@ def _hash
       def _eql?(other)
         other.members.reject {|m| m == "\n"}.eql?(self.members.reject {|m| m == "\n"})
       end
+
+      private
+
+      def extended_not_expanded_to_s(extended_not_expanded)
+        extended_not_expanded.map do |choices|
+          choices = choices.map do |sel|
+            next sel.first.to_s if sel.size == 1
+            "#{sel.join ' '}"
+          end
+          next choices.first if choices.size == 1 && !choices.include?(' ')
+          "(#{choices.join ', '})"
+        end.join ' '
+      end
     end
   end
 end

test/sass/extend_test.rb

@@ -1753,7 +1753,7 @@ def test_nested_extend_loop
     assert_equal <<CSS, render(<<SCSS)
 .bar, .bar .foo {
   a: b; }
-  .bar .foo, .bar .foo .foo {
+  .bar .foo {
     c: d; }
 CSS
 .bar {
@@ -2010,6 +2010,40 @@ def test_nested_selector_with_child_selector_hack_extender_and_extendee_and_newl
 SCSS
   end
 
+  def test_extended_parent_and_child_redundancy_elimination
+    assert_equal <<CSS, render(<<SCSS)
+a b, d b, a c, d c {
+  a: b; }
+CSS
+a {
+  b {a: b}
+  c {@extend b}
+}
+d {@extend a}
+SCSS
+  end
+
+  def test_extend_cross_branch_redundancy_elimination
+    assert_equal <<CSS, render(<<SCSS)
+a c d, b c a d {
+  a: b; }
+CSS
+%x c %y {a: b}
+a, b {@extend %x}
+a d {@extend %y}
+SCSS
+
+    assert_equal <<CSS, render(<<SCSS)
+e a c d, a c e d, e b c a d, b c a e d {
+  a: b; }
+CSS
+e %z {a: b}
+%x c %y {@extend %z}
+a, b {@extend %x}
+a d {@extend %y}
+SCSS
+  end
+
   private
 
   def render(sass, options = {})