Prevent should from circumventing target check

[pirj]

By directly instantiating expectation handler, should/should_not was circumventing enforce_value_expectation check.

subject(:action) { -> { raise } }
it { should raise_error StandardError }

would not print "The implicit block expectation syntax is deprecated", while

subject(:action) { -> { raise } }
it { is_expected.to raise_error StandardError }

will.

See rspec/rspec-expectations#1139

[JonRowe]

So I think the reason why this was the way it was, is because expect can be disabled

[pirj]

There seems to be yet another reason. should/should_not support OperatorMatcher, while is_expected.to does not, due to its prevent_operator_matchers check.

Not sure about the reasoning behind this, probably it does not read right, e.g. it is expected to > 0. Obviously, it is expected to be > 0 sounds better.
However, it should > 0 isn't easy on the ear either.

I intend to assume that should always represents the value expectation syntax, and:

• revert the current change to is_expected.to
• add an explicit call to enforce_value_expectation similar to what we do in ValueExpectationTarget.

This will print a warning for:

subject(:action) { -> { raise } }
it { should raise_error StandardError }

but the following will continue working:

it { should > 3 }

@JonRowe @benoittgt Is this a correct approach?

[pirj]

I would also love to move should/should_not/is_expected to rspec-expectations in a follow-up. Can you think of some implications to that? All of them imply that:

This only works if you are using rspec-expectations

[pirj]

Might be now is a good moment to:

• deprecate should with OperatorMatcher
• retire OperatorMatcher

since BeComparedTo does this job anyway with better readability.

WDYT?

[JonRowe]

With should being removed there is no need to deprecate a specific matcher with it.

[pirj]

Update: I've used a different approach - checking for a value matching support of matchers passed to should/should_not.

[pirj]

I give up on Travis.

[pirj]

This redundant as the whole should is deprecated and will be removed.

[JonRowe]

Reopened because we still need to deprecate this for 3.11, to be merged with rspec/rspec-expectations#1139

[pirj]

@JonRowe ok to merge this?

[JonRowe]

This just looks ugly 😂 , WDYT about extracting the block to a method e.g:

Suggested change

	matcher_supports_value_expectations = begin
	matcher.supports_value_expectations?
	rescue
	true
	end
	return if matcher_supports_value_expectations
	return if matcher_supports_value_expectations?

and elsewhere:

      # @private
      def  matcher_supports_value_expectations?
        matcher.supports_value_expectations?
      rescue
        true
      end

[pirj]

No objections. Especially since enforce_value_expectation will remain only in RSpec 3.x, and will be removed along with should in 4.
Fixed.