diff --git a/Changelog.md b/Changelog.md index d7bc40671..6196f978c 100644 --- a/Changelog.md +++ b/Changelog.md @@ -6,6 +6,7 @@ Bug Fixes: * When using null object doubles, prevent typos triggering dynamic matchers. (Eric Mueller, #1455) * Use `RSpec.warning` for an expectation warning rather than `Kernel.warn`. (Jon Rowe, #1472) +* Prevent mismatched use of block and value matchers in compound expectations. (Phil Pirozhkov, #1476) Enhancements: diff --git a/lib/rspec/matchers/built_in/compound.rb b/lib/rspec/matchers/built_in/compound.rb index 0d8090c68..3a7fb1e67 100644 --- a/lib/rspec/matchers/built_in/compound.rb +++ b/lib/rspec/matchers/built_in/compound.rb @@ -77,8 +77,11 @@ def initialize_copy(other) def match(_expected, actual) evaluator_klass = if supports_block_expectations? && Proc === actual NestedEvaluator - else + elsif supports_value_expectations? SequentialEvaluator + else + # Can't raise an ArgumentError in this context, as it's rescued + raise "Block and value matchers can't be combined in a compound expectation (#{matcher_1.description}, #{matcher_2.description})" end @evaluator = evaluator_klass.new(actual, matcher_1, matcher_2) diff --git a/spec/rspec/matchers/built_in/compound_spec.rb b/spec/rspec/matchers/built_in/compound_spec.rb index 12c416148..fde567dc1 100644 --- a/spec/rspec/matchers/built_in/compound_spec.rb +++ b/spec/rspec/matchers/built_in/compound_spec.rb @@ -261,10 +261,17 @@ def expect_block end describe "expect(...).to custom_matcher.and(other_matcher)" do - it "treats a matcher that doesn't support value expectations correctly" do - expect { - expect([1, 2]).to include(1).and raise_error(/test/) - }.to fail_with(/but was not given a block/) + if RUBY_VERSION.to_f > 1.8 # The example can be adjusted to be compatible with Ruby 1.8, but it is then not indicative of the problem + binding.eval(<<-CODE, __FILE__, __LINE__) + it "does not allow composing incompatible matchers" do + arr = [] + expect { + expect { arr << :foo } + .to change { arr }.to be_one + .and change { arr }.to include(:foo) # There is a barely noticeable difference: the `.and` runs on the wrong matcher, `be_one` instead of `change` + }.to raise_error("Block and value matchers can't be combined in a compound expectation (be one, change result to include :foo)") + end + CODE end it "treats a matcher that does support value expectations correctly" do