Restrict block matchers use with value expectations

https://blog.rubystyle.guide/rspec/2019/07/17/rspec-implicit-block-syntax.html
https://rspec.rubystyle.guide/#implicit-block-expectations

Spec changes are due to:

    matcher.matches?(invalid_value)

doesn't work with block-only matchers, as no block is passed in, and it
fails with a message:

    1) RSpec::Matchers::BuiltIn::Change behaves like an RSpec block-only matcher uses the `ObjectFormatter` for `failure_message`
       Failure/Error: expect(message).to include("detailed inspect")
         expected "expected `@k` to have changed, but was not given a block" to include "detailed inspect"

The redundant (due to existing check in ExpectationTarget) `Proc ===
@event_proc` checks could not be removed safely as well, since
@actual_after is not initialized yet when we haven't executed the block:

     RuntimeError:
       Warnings were generated: rspec-dev/repos/rspec-expectations/lib/rspec/matchers/built_in/change.rb:407: warning: instance variable @actual_after not initialized

Also see:
  #1139
  #1125

Changelog.md

@@ -11,6 +11,8 @@ Breaking Changes:
 * Remove `include_chain_clauses_in_custom_matcher_descriptions` option
   and make it the default. (Phil Pirozhkov, #1279)
 * Remove support for present-tense dynamic predicate. (Phil Pirozhkov, #1286)
+* Prevent implicit blocks (e.g blocks as values) from being used with block matchers.
+  (Phil Pirozhkov, #1285)
 
 Enhancements:
 

lib/rspec/expectations/expectation_target.rb

@@ -111,16 +111,13 @@ def not_to(matcher=nil, message=nil, &block)
       def enforce_value_expectation(matcher)
         return if supports_value_expectations?(matcher)
 
-        RSpec.deprecate(
-          "expect(value).to #{RSpec::Support::ObjectFormatter.format(matcher)}",
-          :message =>
-            "The implicit block expectation syntax is deprecated, you should pass " \
-            "a block rather than an argument to `expect` to use the provided " \
-            "block expectation matcher or the matcher must implement " \
-            "`supports_value_expectations?`. e.g  `expect { value }.to " \
-            "#{RSpec::Support::ObjectFormatter.format(matcher)}` not " \
-            "`expect(value).to #{RSpec::Support::ObjectFormatter.format(matcher)}`"
-        )
+        raise ArgumentError,
+          "The implicit block expectation syntax is not supported, you should pass " \
+          "a block rather than an argument to `expect` to use the provided " \
+          "block expectation matcher or the matcher must implement " \
+          "`supports_value_expectations?`. e.g  `expect { value }.to " \
+          "#{RSpec::Support::ObjectFormatter.format(matcher)}` not " \
+          "`expect(value).to #{RSpec::Support::ObjectFormatter.format(matcher)}`"
       end
 
       def supports_value_expectations?(matcher)

lib/rspec/matchers/built_in/change.rb

@@ -112,12 +112,10 @@ def raise_block_syntax_error
         end
 
         def positive_failure_reason
-          return "was not given a block" unless Proc === @event_proc
           "is still #{@actual_before_description}"
         end
 
         def negative_failure_reason
-          return "was not given a block" unless Proc === @event_proc
           "did change from #{@actual_before_description} " \
           "to #{description_of change_details.actual_after}"
         end
@@ -171,7 +169,6 @@ def supports_value_expectations?
       private
 
         def failure_reason
-          return "was not given a block" unless Proc === @event_proc
           "was changed by #{description_of @change_details.actual_delta}"
         end
       end

lib/rspec/matchers/built_in/output.rb

@@ -108,13 +108,11 @@ def captured?
         end
 
         def positive_failure_reason
-          return "was not a block" unless Proc === @block
           return "output #{actual_output_description}" if @expected
           "did not"
         end
 
         def negative_failure_reason
-          return "was not a block" unless Proc === @block
           "output #{actual_output_description}"
         end
 

lib/rspec/matchers/built_in/raise_error.rb

@@ -234,7 +234,6 @@ def format_backtrace(backtrace)
         end
 
         def given_error
-          return " but was not given a block" unless Proc === @given_proc
           return " but nothing was raised" unless @actual_error
 
           backtrace = format_backtrace(@actual_error.backtrace)

lib/rspec/matchers/built_in/throw_symbol.rb

@@ -102,7 +102,6 @@ def expects_call_stack_jump?
       private
 
         def actual_result
-          return "but was not a block" unless Proc === @block
           "got #{caught}"
         end
 

lib/rspec/matchers/built_in/yield.rb

@@ -12,7 +12,6 @@ module BuiltIn
       class YieldProbe
         def self.probe(block, &callback)
           probe = new(block, &callback)
-          return probe unless probe.has_block?
           probe.probe
         end
 
@@ -26,10 +25,6 @@ def initialize(block, &callback)
           self.yielded_args = []
         end
 
-        def has_block?
-          Proc === @block
-        end
-
         def probe
           assert_valid_expect_block!
           @block.call(self)
@@ -90,13 +85,12 @@ class YieldControl < BaseMatcher
         # @private
         def matches?(block)
           @probe = YieldProbe.probe(block)
-          return false unless @probe.has_block?
           expected_count_matches?(@probe.num_yields)
         end
 
         # @private
         def does_not_match?(block)
-          !matches?(block) && @probe.has_block?
+          !matches?(block)
         end
 
         # @api private
@@ -124,7 +118,6 @@ def supports_value_expectations?
       private
 
         def failure_reason
-          return ' but was not a block' unless @probe.has_block?
           return "#{count_expectation_description} but did not yield" if @probe.num_yields == 0
           count_failure_reason('yielded')
         end
@@ -137,13 +130,12 @@ class YieldWithNoArgs < BaseMatcher
         # @private
         def matches?(block)
           @probe = YieldProbe.probe(block)
-          return false unless @probe.has_block?
           @probe.yielded_once?(:yield_with_no_args) && @probe.single_yield_args.empty?
         end
 
         # @private
         def does_not_match?(block)
-          !matches?(block) && @probe.has_block?
+          !matches?(block)
         end
 
         # @private
@@ -169,13 +161,11 @@ def supports_value_expectations?
       private
 
         def positive_failure_reason
-          return 'was not a block' unless @probe.has_block?
           return 'did not yield' if @probe.num_yields.zero?
           "yielded with arguments: #{description_of @probe.single_yield_args}"
         end
 
         def negative_failure_reason
-          return 'was not a block' unless @probe.has_block?
           'did'
         end
       end
@@ -196,14 +186,13 @@ def matches?(block)
             @actual_formatted = actual_formatted
             @args_matched_when_yielded &&= args_currently_match?
           end
-          return false unless @probe.has_block?
           @probe.probe
           @probe.yielded_once?(:yield_with_args) && @args_matched_when_yielded
         end
 
         # @private
         def does_not_match?(block)
-          !matches?(block) && @probe.has_block?
+          !matches?(block)
         end
 
         # @private
@@ -236,7 +225,6 @@ def supports_value_expectations?
       private
 
         def positive_failure_reason
-          return 'was not a block' unless @probe.has_block?
           return 'did not yield' if @probe.num_yields.zero?
           @positive_args_failure
         end
@@ -246,9 +234,7 @@ def expected_arg_description
         end
 
         def negative_failure_reason
-          if !@probe.has_block?
-            'was not a block'
-          elsif @args_matched_when_yielded && !@expected.empty?
+          if @args_matched_when_yielded && !@expected.empty?
             'yielded with expected arguments' \
               "\nexpected not: #{surface_descriptions_in(@expected).inspect}" \
               "\n         got: #{@actual_formatted}"
@@ -300,12 +286,11 @@ def matches?(block)
             yield_count += 1
           end
 
-          return false unless @probe.has_block?
           args_matched_when_yielded && yield_count == @expected.length
         end
 
         def does_not_match?(block)
-          !matches?(block) && @probe.has_block?
+          !matches?(block)
         end
 
         # @private
@@ -342,16 +327,12 @@ def expected_arg_description
         end
 
         def positive_failure_reason
-          return 'was not a block' unless @probe.has_block?
-
           'yielded with unexpected arguments' \
           "\nexpected: #{surface_descriptions_in(@expected).inspect}" \
           "\n     got: [#{@actual_formatted.join(", ")}]"
         end
 
         def negative_failure_reason
-          return 'was not a block' unless @probe.has_block?
-
           'yielded with expected arguments' \
           "\nexpected not: #{surface_descriptions_in(@expected).inspect}" \
           "\n         got: [#{@actual_formatted.join(", ")}]"

spec/rspec/matchers/built_in/change_spec.rb

@@ -1038,7 +1038,7 @@ def invalid_block
 end
 
 RSpec.describe RSpec::Matchers::BuiltIn::ChangeRelatively do
-  it_behaves_like "an RSpec block-only matcher", :disallows_negation => true, :skip_deprecation_check => true do
+  it_behaves_like "an RSpec block-only matcher", :disallows_negation => true do
     let(:matcher) { change { @k }.by(1) }
     before { @k = 0 }
     def valid_block

spec/rspec/matchers/built_in/yield_spec.rb

@@ -36,8 +36,8 @@ def each_arg(*args, &block)
 # this example group overrides the default definition of expectations
 # and lambdas that take the expectation target in a way that they accept
 # a probe.
-RSpec.shared_examples "an RSpec probe-yielding block-only matcher" do |*options|
-  include_examples "an RSpec block-only matcher", { :expects_lambda => true }.merge(options.first || {}) do
+RSpec.shared_examples "an RSpec probe-yielding block-only matcher" do |**options|
+  include_examples "an RSpec block-only matcher", **options do
     let(:valid_expectation) { expect { |block| valid_block(&block) } }
     let(:invalid_expectation) { expect { |block| invalid_block(&block) } }
 

spec/support/shared_examples/block_matcher.rb

@@ -69,27 +69,25 @@
   it 'fails gracefully when given a value' do
     expect {
       expect(3).to matcher
-    }.to fail_with(/was not( given)? a block/)
-
+    }.to raise_error(/implicit block expectation syntax is not supported/)
     unless options[:disallows_negation]
       expect {
         expect(3).not_to matcher
-      }.to fail_with(/was not( given)? a block/)
+      }.to raise_error(/implicit block expectation syntax is not supported/)
     end
   end
 
   it 'prints a deprecation warning when given a value' do
-    expect_warn_deprecation(/The implicit block expectation syntax is deprecated, you should pass/)
-    expect { expect(3).to matcher }.to fail
-  end unless options[:skip_deprecation_check] || options[:expects_lambda]
+    expect { expect(3).to matcher }
+      .to raise_error(/implicit block expectation syntax is not supported/)
+  end
 
   it 'prints a deprecation warning when given a value and negated' do
-    expect_warn_deprecation(/The implicit block expectation syntax is deprecated, you should pass/)
-    expect { expect(3).not_to matcher }.to fail
-  end unless options[:disallows_negation] || options[:expects_lambda]
+    expect { expect(3).not_to matcher }
+      .to raise_error(/implicit block expectation syntax is not supported/)
+  end unless options[:disallows_negation]
 
-  it 'allows lambda expectation target' do
-    allow_deprecation
-    expect(valid_block_lambda).to matcher
+  it 'allows a Proc for an expectation target' do
+    expect(&valid_block_lambda).to matcher
   end
 end