Merge pull request #1476 from rspec/error-on-improperly-composing-matchers

Prevent mismatched composed use of value and block matchers

Author: pirj

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:
 

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)

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