Skip to content

Commit

Permalink
Allow extends that match but don't unify.
Browse files Browse the repository at this point in the history
Closes #2250
  • Loading branch information
nex3 committed Jun 16, 2017
1 parent 3b0de7a commit 7e4aca7
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 107 deletions.
3 changes: 3 additions & 0 deletions doc-src/SASS_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion lib/sass/selector/comma_sequence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions lib/sass/selector/simple_sequence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
8 changes: 3 additions & 5 deletions lib/sass/tree/visitors/cssize.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,9 @@ def visit_root(node)
# @attr node [Sass::Tree::ExtendNode] The node that produced this extend.
# @attr directives [Array<Sass::Tree::DirectiveNode>]
# 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)
Expand Down
10 changes: 2 additions & 8 deletions lib/sass/tree/visitors/extend.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(<<MESSAGE, :filename => 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
Expand Down
116 changes: 25 additions & 91 deletions test/sass/extend_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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, *'
Expand All @@ -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'
Expand All @@ -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
Expand All @@ -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'
Expand All @@ -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'
Expand All @@ -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
Expand All @@ -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'
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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(<<SCSS)
assert_equal <<CSS, render(<<SCSS)
a.baz.bar {
color: blue; }
CSS
a%foo.baz {color: blue}
.bar {@extend %foo}
div {@extend %foo}
SCSS
end
end

def test_placeholder_interpolation
Expand Down Expand Up @@ -1343,17 +1288,6 @@ def test_extend_warns_when_extendee_doesnt_exist
SCSS
end

def test_extend_warns_when_extension_fails
assert_raise_message(Sass::SyntaxError, <<ERR) {render(<<SCSS)}
"b.foo" failed to @extend ".bar".
No selectors matching ".bar" could be unified with "b.foo".
Use "@extend .bar !optional" if the extend should be able to fail.
ERR
a.bar {a: b}
b.foo {@extend .bar}
SCSS
end

def test_extend_succeeds_when_one_extension_fails_but_others_dont
assert_equal(<<CSS, render(<<SCSS))
a.bar {
Expand Down

0 comments on commit 7e4aca7

Please sign in to comment.