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

@@ -1,5 +1,5 @@
 ### Development
-[Full Changelog](http://github.com/rspec/rspec-expectations/compare/v3.10.0...main)
+[Full Changelog](http://github.com/rspec/rspec-expectations/compare/v3.10.1...main)
 
 Breaking Changes:
 
@@ -10,6 +10,10 @@ Breaking Changes:
 * Remove support for legacy RSpec matchers (pre 3). (Phil Pirozhkov, #1253)
 * Remove `include_chain_clauses_in_custom_matcher_descriptions` option
   and make it the default. (Phil Pirozhkov, #1279)
+* Restrict block matchers use with value expectations. (Phil Pirozhkov, #1285)
+
+### 3.10.1 / 2020-12-27
+[Full Changelog](http://github.com/rspec/rspec-expectations/compare/v3.10.0...v3.10.1)
 
 Enhancements:
 
@@ -122,7 +126,7 @@ Bug Fixes:
 * Prevent composed `all` matchers from leaking into their siblings leading to duplicate
   failures. (Jamie English, #1086)
 * Prevent objects which change their hash on comparison from failing change checks.
-  (Phil Pirozhkov, #1110)
+  (Phil Pirozhkov, #1100)
 * Issue an `ArgumentError` rather than a `NoMethodError` when `be_an_instance_of` and
   `be_kind_of` matchers encounter objects not supporting those methods.
   (Taichi Ishitani, #1107)

lib/rspec/expectations/expectation_target.rb

@@ -42,7 +42,7 @@ def self.for(value, block)
         elsif block
           raise ArgumentError, "You cannot pass both an argument and a block to `expect`."
         else
-          new(value)
+          ValueExpectationTarget.new(value)
         end
       end
 
@@ -90,6 +90,41 @@ def prevent_operator_matchers(verb)
       include InstanceMethods
     end
 
+    # @private
+    # Validates the provided matcher to ensure it supports block
+    # expectations, in order to avoid user confusion when they
+    # use a block thinking the expectation will be on the return
+    # value of the block rather than the block itself.
+    class ValueExpectationTarget < ExpectationTarget
+      def to(matcher=nil, message=nil, &block)
+        enforce_value_expectation(matcher)
+        super
+      end
+
+      def not_to(matcher=nil, message=nil, &block)
+        enforce_value_expectation(matcher)
+        super
+      end
+
+    private
+
+      def enforce_value_expectation(matcher)
+        return if supports_value_expectations?(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 (#{RSpec::Support::ObjectFormatter.format(matcher)}), " \
+          "or the matcher must implement `supports_value_expectations?`."
+      end
+
+      def supports_value_expectations?(matcher)
+        matcher.supports_value_expectations?
+      rescue NoMethodError
+        true
+      end
+    end
+
     # @private
     # Validates the provided matcher to ensure it supports block
     # expectations, in order to avoid user confusion when they

lib/rspec/matchers/built_in/base_matcher.rb

@@ -78,6 +78,11 @@ def supports_block_expectations?
           false
         end
 
+        # @private
+        def supports_value_expectations?
+          true
+        end
+
         # @api private
         def expects_call_stack_jump?
           false

lib/rspec/matchers/built_in/change.rb

@@ -77,6 +77,11 @@ def supports_block_expectations?
           true
         end
 
+        # @private
+        def supports_value_expectations?
+          false
+        end
+
       private
 
         def initialize(receiver=nil, message=nil, &block)
@@ -107,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
@@ -158,10 +161,14 @@ def supports_block_expectations?
           true
         end
 
+        # @private
+        def supports_value_expectations?
+          false
+        end
+
       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
@@ -201,6 +208,11 @@ def supports_block_expectations?
           true
         end
 
+        # @private
+        def supports_value_expectations?
+          false
+        end
+
       private
 
         def perform_change(event_proc)
@@ -337,6 +349,8 @@ def change_description
       class ChangeDetails
         attr_reader :actual_after
 
+        UNDEFINED = Module.new.freeze
+
         def initialize(matcher_name, receiver=nil, message=nil, &block)
           if receiver && !message
             raise(
@@ -351,6 +365,11 @@ def initialize(matcher_name, receiver=nil, message=nil, &block)
           @receiver = receiver
           @message = message
           @value_proc = block
+          # TODO: temporary measure to mute warning of access to an initialized
+          # instance variable when a deprecated implicit block expectation
+          # syntax is used. This may be removed once `fail` is used, and the
+          # matcher never issues this warning.
+          @actual_after = UNDEFINED
         end
 
         def value_representation

lib/rspec/matchers/built_in/compound.rb

@@ -26,11 +26,19 @@ def description
           "#{matcher_1.description} #{conjunction} #{matcher_2.description}"
         end
 
+        # @api private
         def supports_block_expectations?
           matcher_supports_block_expectations?(matcher_1) &&
           matcher_supports_block_expectations?(matcher_2)
         end
 
+        # @api private
+        def supports_value_expectations?
+          matcher_supports_value_expectations?(matcher_1) &&
+          matcher_supports_value_expectations?(matcher_2)
+        end
+
+        # @api private
         def expects_call_stack_jump?
           NestedEvaluator.matcher_expects_call_stack_jump?(matcher_1) ||
           NestedEvaluator.matcher_expects_call_stack_jump?(matcher_2)
@@ -102,6 +110,12 @@ def matcher_supports_block_expectations?(matcher)
           false
         end
 
+        def matcher_supports_value_expectations?(matcher)
+          matcher.supports_value_expectations?
+        rescue NoMethodError
+          true
+        end
+
         def matcher_is_diffable?(matcher)
           matcher.diffable?
         rescue NoMethodError

lib/rspec/matchers/built_in/output.rb

@@ -94,20 +94,25 @@ def supports_block_expectations?
           true
         end
 
+        # @api private
+        # Indicates this matcher matches against a block only.
+        # @return [False]
+        def supports_value_expectations?
+          false
+        end
+
       private
 
         def captured?
           @actual.length > 0
         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

@@ -86,6 +86,12 @@ def supports_block_expectations?
           true
         end
 
+        # @private
+        def supports_value_expectations?
+          false
+        end
+
+        # @private
         def expects_call_stack_jump?
           true
         end
@@ -228,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

@@ -85,20 +85,23 @@ def description
         end
 
         # @api private
-        # Indicates this matcher matches against a block.
-        # @return [True]
         def supports_block_expectations?
           true
         end
 
+        # @api private
+        def supports_value_expectations?
+          false
+        end
+
+        # @api private
         def expects_call_stack_jump?
           true
         end
 
       private
 
         def actual_result
-          return "but was not a block" unless Proc === @block
           "got #{caught}"
         end