From 7e4aca7defad5003e1930b18129fd664ac34418b Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Thu, 15 Jun 2017 17:11:59 -0700 Subject: [PATCH] Allow extends that match but don't unify. Closes #2250 --- doc-src/SASS_CHANGELOG.md | 3 + lib/sass/selector/comma_sequence.rb | 2 +- lib/sass/selector/simple_sequence.rb | 3 +- lib/sass/tree/visitors/cssize.rb | 8 +- lib/sass/tree/visitors/extend.rb | 10 +-- test/sass/extend_test.rb | 116 ++++++--------------------- 6 files changed, 35 insertions(+), 107 deletions(-) diff --git a/doc-src/SASS_CHANGELOG.md b/doc-src/SASS_CHANGELOG.md index 03069588fd..5966bfed03 100644 --- a/doc-src/SASS_CHANGELOG.md +++ b/doc-src/SASS_CHANGELOG.md @@ -10,6 +10,9 @@ * Combine ids and `:root` when unifying selectors with `@extend` and selector functions. +* It's no longer an error to `@extend` a selector that exists in the stylesheet, + but for which unification fails. + * Add a `$weight` parameter to `invert()`. * The last argument in an argument list can now have a trailing comma. diff --git a/lib/sass/selector/comma_sequence.rb b/lib/sass/selector/comma_sequence.rb index 9a388e23df..8b6b373309 100644 --- a/lib/sass/selector/comma_sequence.rb +++ b/lib/sass/selector/comma_sequence.rb @@ -129,7 +129,7 @@ def populate_extends(extends, extendee, extend_node = nil, parent_directives = [ end extends[sel] = Sass::Tree::Visitors::Cssize::Extend.new( - member, sel, extend_node, parent_directives, :not_found) + member, sel, extend_node, parent_directives, false) end end end diff --git a/lib/sass/selector/simple_sequence.rb b/lib/sass/selector/simple_sequence.rb index 0b28533e1c..d7c3d73f2a 100644 --- a/lib/sass/selector/simple_sequence.rb +++ b/lib/sass/selector/simple_sequence.rb @@ -191,10 +191,9 @@ def do_extend(extends, parent_directives, replace, seen) # seq is A, sels is B, and self is C self_without_sel = Sass::Util.array_minus(members, sels) - group.each {|e| e.result = :failed_to_unify unless e.result == :succeeded} + group.each {|e| e.success = true} unified = seq.members.last.unify(SimpleSequence.new(self_without_sel, subject?)) next unless unified - group.each {|e| e.result = :succeeded} group.each {|e| check_directives_match!(e, parent_directives)} new_seq = Sequence.new(seq.members[0...-1] + [unified]) new_seq.add_sources!(sources + [seq]) diff --git a/lib/sass/tree/visitors/cssize.rb b/lib/sass/tree/visitors/cssize.rb index d5c8ee2d5c..6851286ea0 100644 --- a/lib/sass/tree/visitors/cssize.rb +++ b/lib/sass/tree/visitors/cssize.rb @@ -106,11 +106,9 @@ def visit_root(node) # @attr node [Sass::Tree::ExtendNode] The node that produced this extend. # @attr directives [Array] # The directives containing the `@extend`. - # @attr result [Symbol] - # The result of this extend. One of `:not_found` (the target doesn't exist - # in the document), `:failed_to_unify` (the target exists but cannot be - # unified with the extender), or `:succeeded`. - Extend = Struct.new(:extender, :target, :node, :directives, :result) + # @attr success [Boolean] + # Whether this extend successfully matched a selector. + Extend = Struct.new(:extender, :target, :node, :directives, :success) # Registers an extension in the `@extends` subset map. def visit_extend(node) diff --git a/lib/sass/tree/visitors/extend.rb b/lib/sass/tree/visitors/extend.rb index 073adc98c0..ffc3e216d8 100644 --- a/lib/sass/tree/visitors/extend.rb +++ b/lib/sass/tree/visitors/extend.rb @@ -49,19 +49,13 @@ class << self def check_extends_fired!(extends) extends.each_value do |ex| - next if ex.result == :succeeded || ex.node.optional? + next if ex.success || ex.node.optional? message = "\"#{ex.extender}\" failed to @extend \"#{ex.target.join}\"." - reason = - if ex.result == :not_found - "The selector \"#{ex.target.join}\" was not found." - else - "No selectors matching \"#{ex.target.join}\" could be unified with \"#{ex.extender}\"." - end # TODO(nweiz): this should use the Sass stack trace of the extend node. raise Sass::SyntaxError.new(< ex.node.filename, :line => ex.node.line) #{message} -#{reason} +The selector "#{ex.target.join}" was not found. Use "@extend #{ex.target.join} !optional" if the extend should be able to fail. MESSAGE end diff --git a/test/sass/extend_test.rb b/test/sass/extend_test.rb index 1dc90821ce..ad5245599e 100755 --- a/test/sass/extend_test.rb +++ b/test/sass/extend_test.rb @@ -152,10 +152,7 @@ def test_class_unification def test_id_unification assert_unification '.foo.bar', '#baz {@extend .foo}', '.foo.bar, .bar#baz' assert_unification '.foo#baz', '#baz {@extend .foo}', '#baz' - - assert_extend_doesnt_match('#bar', '.foo', :failed_to_unify, 2) do - render_unification '.foo#baz', '#bar {@extend .foo}' - end + assert_unification '.foo#baz', '#bar {@extend .foo}', '.foo#baz' end def test_universal_unification_with_simple_target @@ -167,10 +164,7 @@ def test_universal_unification_with_simple_target end def test_universal_unification_with_namespaceless_universal_target - assert_extend_doesnt_match('ns|*', '.foo', :failed_to_unify, 2) do - render_unification '*.foo', 'ns|* {@extend .foo}' - end - + assert_unification '*.foo', 'ns|* {@extend .foo}', '*.foo' assert_unification '*.foo', '* {@extend .foo}', '*' assert_unification '*.foo', '*|* {@extend .foo}', '*' assert_unification '*|*.foo', '* {@extend .foo}', '*|*.foo, *' @@ -179,23 +173,14 @@ def test_universal_unification_with_namespaceless_universal_target end def test_universal_unification_with_namespaced_universal_target - assert_extend_doesnt_match('*', '.foo', :failed_to_unify, 2) do - render_unification 'ns|*.foo', '* {@extend .foo}' - end - - assert_extend_doesnt_match('ns2|*', '.foo', :failed_to_unify, 2) do - render_unification 'ns1|*.foo', 'ns2|* {@extend .foo}' - end - + assert_unification 'ns|*.foo', '* {@extend .foo}', 'ns|*.foo' + assert_unification 'ns1|*.foo', 'ns2|* {@extend .foo}', 'ns1|*.foo' assert_unification 'ns|*.foo', '*|* {@extend .foo}', 'ns|*' assert_unification 'ns|*.foo', 'ns|* {@extend .foo}', 'ns|*' end def test_universal_unification_with_namespaceless_element_target - assert_extend_doesnt_match('ns|*', '.foo', :failed_to_unify, 2) do - render_unification 'a.foo', 'ns|* {@extend .foo}' - end - + assert_unification 'a.foo', 'ns|* {@extend .foo}', 'a.foo' assert_unification 'a.foo', '* {@extend .foo}', 'a' assert_unification 'a.foo', '*|* {@extend .foo}', 'a' assert_unification '*|a.foo', '* {@extend .foo}', '*|a.foo, a' @@ -204,14 +189,8 @@ def test_universal_unification_with_namespaceless_element_target end def test_universal_unification_with_namespaced_element_target - assert_extend_doesnt_match('*', '.foo', :failed_to_unify, 2) do - render_unification 'ns|a.foo', '* {@extend .foo}' - end - - assert_extend_doesnt_match('ns2|*', '.foo', :failed_to_unify, 2) do - render_unification 'ns1|a.foo', 'ns2|* {@extend .foo}' - end - + assert_unification 'ns|a.foo', '* {@extend .foo}', 'ns|a.foo' + assert_unification 'ns1|a.foo', 'ns2|* {@extend .foo}', 'ns1|a.foo' assert_unification 'ns|a.foo', '*|* {@extend .foo}', 'ns|a' assert_unification 'ns|a.foo', 'ns|* {@extend .foo}', 'ns|a' end @@ -224,10 +203,7 @@ def test_element_unification_with_simple_target end def test_element_unification_with_namespaceless_universal_target - assert_extend_doesnt_match('ns|a', '.foo', :failed_to_unify, 2) do - render_unification '*.foo', 'ns|a {@extend .foo}' - end - + assert_unification '*.foo', 'ns|a {@extend .foo}', '*.foo' assert_unification '*.foo', 'a {@extend .foo}', '*.foo, a' assert_unification '*.foo', '*|a {@extend .foo}', '*.foo, a' assert_unification '*|*.foo', 'a {@extend .foo}', '*|*.foo, a' @@ -236,27 +212,15 @@ def test_element_unification_with_namespaceless_universal_target end def test_element_unification_with_namespaced_universal_target - assert_extend_doesnt_match('a', '.foo', :failed_to_unify, 2) do - render_unification 'ns|*.foo', 'a {@extend .foo}' - end - - assert_extend_doesnt_match('ns2|a', '.foo', :failed_to_unify, 2) do - render_unification 'ns1|*.foo', 'ns2|a {@extend .foo}' - end - + assert_unification 'ns|*.foo', 'a {@extend .foo}', 'ns|*.foo' + assert_unification 'ns1|*.foo', 'ns2|a {@extend .foo}', 'ns1|*.foo' assert_unification 'ns|*.foo', '*|a {@extend .foo}', 'ns|*.foo, ns|a' assert_unification 'ns|*.foo', 'ns|a {@extend .foo}', 'ns|*.foo, ns|a' end def test_element_unification_with_namespaceless_element_target - assert_extend_doesnt_match('ns|a', '.foo', :failed_to_unify, 2) do - render_unification 'a.foo', 'ns|a {@extend .foo}' - end - - assert_extend_doesnt_match('h1', '.foo', :failed_to_unify, 2) do - render_unification 'a.foo', 'h1 {@extend .foo}' - end - + assert_unification 'a.foo', 'ns|a {@extend .foo}', 'a.foo' + assert_unification 'a.foo', 'h1 {@extend .foo}', 'a.foo' assert_unification 'a.foo', 'a {@extend .foo}', 'a' assert_unification 'a.foo', '*|a {@extend .foo}', 'a' assert_unification '*|a.foo', 'a {@extend .foo}', '*|a.foo, a' @@ -265,14 +229,8 @@ def test_element_unification_with_namespaceless_element_target end def test_element_unification_with_namespaced_element_target - assert_extend_doesnt_match('a', '.foo', :failed_to_unify, 2) do - render_unification 'ns|a.foo', 'a {@extend .foo}' - end - - assert_extend_doesnt_match('ns2|a', '.foo', :failed_to_unify, 2) do - render_unification 'ns1|a.foo', 'ns2|a {@extend .foo}' - end - + assert_unification 'ns|a.foo', 'a {@extend .foo}', 'ns|a.foo' + assert_unification 'ns1|a.foo', 'ns2|a {@extend .foo}', 'ns1|a.foo' assert_unification 'ns|a.foo', '*|a {@extend .foo}', 'ns|a' assert_unification 'ns|a.foo', 'ns|a {@extend .foo}', 'ns|a' end @@ -288,15 +246,8 @@ def test_attribute_unification def test_pseudo_unification assert_unification ':foo.baz', ':foo(2n+1) {@extend .baz}', ':foo.baz, :foo:foo(2n+1)' assert_unification ':foo.baz', '::foo {@extend .baz}', ':foo.baz, :foo::foo' - - assert_extend_doesnt_match('::bar', '.baz', :failed_to_unify, 2) do - render_unification '::foo.baz', '::bar {@extend .baz}' - end - - assert_extend_doesnt_match('::foo(2n+1)', '.baz', :failed_to_unify, 2) do - render_unification '::foo.baz', '::foo(2n+1) {@extend .baz}' - end - + assert_unification '::foo.baz', '::bar {@extend .baz}', '::foo.baz' + assert_unification '::foo.baz', '::foo(2n+1) {@extend .baz}', '::foo.baz' assert_unification '::foo.baz', '::foo {@extend .baz}', '::foo' assert_unification '::foo(2n+1).baz', '::foo(2n+1) {@extend .baz}', '::foo(2n+1)' assert_unification ':foo.baz', ':bar {@extend .baz}', ':foo.baz, :foo:bar' @@ -618,14 +569,9 @@ def test_long_extender_runs_unification assert_extends 'ns|*.foo.bar', '*|a.baz {@extend .foo}', 'ns|*.foo.bar, ns|a.bar.baz' end - def test_long_extender_aborts_unification - assert_extend_doesnt_match('h1.baz', '.foo', :failed_to_unify, 2) do - render_extends 'a.foo#bar', 'h1.baz {@extend .foo}' - end - - assert_extend_doesnt_match('.bang#baz', '.foo', :failed_to_unify, 2) do - render_extends 'a.foo#bar', '.bang#baz {@extend .foo}' - end + def test_long_extender_doesnt_unify + assert_extends 'a.foo#bar', 'h1.baz {@extend .foo}', 'a.foo#bar' + assert_extends 'a.foo#bar', '.bang#baz {@extend .foo}', 'a.foo#bar' end ## Nested Extenders @@ -638,10 +584,8 @@ def test_nested_extender_runs_unification assert_extends '.foo.bar', 'foo bar {@extend .foo}', '.foo.bar, foo bar.bar' end - def test_nested_extender_aborts_unification - assert_extend_doesnt_match('foo bar', '.foo', :failed_to_unify, 2) do - render_extends 'baz.foo', 'foo bar {@extend .foo}' - end + def test_nested_extender_doesnt_unify + assert_extends 'baz.foo', 'foo bar {@extend .foo}', 'baz.foo' end def test_nested_extender_alternates_parents @@ -1069,13 +1013,14 @@ def test_placeholder_selector_with_multiple_extenders end def test_placeholder_selector_as_modifier - assert_extend_doesnt_match('div', '%foo', :failed_to_unify, 3) do - render(<