Add setting for strict_predicate_matchers. Make failure message more …

…precise

Changelog.md

@@ -29,6 +29,9 @@ Enhancements:
   (Jon Rowe, #1169)
 * Dynamic `have_<n>` matchers now have output consistent with other dynamic matchers.
   (Marc-André Lafortune, #1195)
+* New config option `strict_predicate_matchers` allows predicate matcher to be strict
+  (i.e. match for `true` or `false`) instead of the default (match truthy vs `false` or `nil`).
+  (Marc-André Lafortune, #1196)
 
 ### 3.9.2 / 2020-05-08
 [Full Changelog](http://github.com/rspec/rspec-expectations/compare/v3.9.1...v3.9.2)

features/built_in_matchers/predicates.feature

@@ -39,7 +39,7 @@ Feature: Predicate matchers
 
   In either case, RSpec provides nice, clear error messages, such as:
 
-    `expected zero? to return true, got false`
+    `expected zero? to be truthy, got false`
 
   Calling private methods will also fail:
 
@@ -60,7 +60,7 @@ Feature: Predicate matchers
       """
     When I run `rspec should_be_zero_spec.rb`
     Then the output should contain "2 examples, 1 failure"
-     And the output should contain "expected `7.zero?` to return true, got false"
+     And the output should contain "expected `7.zero?` to be truthy, got false"
 
   Scenario: should_not be_empty (based on Array#empty?)
     Given a file named "should_not_be_empty_spec.rb" with:
@@ -75,7 +75,7 @@ Feature: Predicate matchers
       """
     When I run `rspec should_not_be_empty_spec.rb`
     Then the output should contain "2 examples, 1 failure"
-     And the output should contain "expected `[].empty?` to return false, got true"
+     And the output should contain "expected `[].empty?` to be falsey, got true"
 
    Scenario: should have_key (based on Hash#has_key?)
     Given a file named "should_have_key_spec.rb" with:
@@ -88,7 +88,7 @@ Feature: Predicate matchers
       """
     When I run `rspec should_have_key_spec.rb`
     Then the output should contain "2 examples, 1 failure"
-     And the output should contain "expected `{:foo=>7}.has_key?(:bar)` to return true, got false"
+     And the output should contain "expected `{:foo=>7}.has_key?(:bar)` to be truthy, got false"
 
    Scenario: should_not have_all_string_keys (based on custom #has_all_string_keys? method)
      Given a file named "should_not_have_all_string_keys_spec.rb" with:
@@ -114,7 +114,7 @@ Feature: Predicate matchers
        """
      When I run `rspec should_not_have_all_string_keys_spec.rb`
      Then the output should contain "2 examples, 1 failure"
-      And the output should contain "expected `42.0.has_decimals?` to return true, got false"
+      And the output should contain "expected `42.0.has_decimals?` to be truthy, got false"
 
    Scenario: matcher arguments are passed on to the predicate method
      Given a file named "predicate_matcher_argument_spec.rb" with:
@@ -136,8 +136,44 @@ Feature: Predicate matchers
        """
      When I run `rspec predicate_matcher_argument_spec.rb`
      Then the output should contain "4 examples, 2 failures"
-      And the output should contain "expected `12.multiple_of?(4)` to return false, got true"
-      And the output should contain "expected `12.multiple_of?(5)` to return true, got false"
+      And the output should contain "expected `12.multiple_of?(4)` to be falsey, got true"
+      And the output should contain "expected `12.multiple_of?(5)` to be truthy, got false"
+
+    Scenario: the config `strict_predicate_matchers` impacts matching of results other than `true` and `false`
+      Given a file named "strict_or_not.rb" with:
+        """ruby
+        class StrangeResult
+          def has_strange_result?
+            42
+          end
+        end
+
+        RSpec.describe StrangeResult do
+          subject { StrangeResult.new }
+
+          before do
+            RSpec.configure do |config|
+              config.expect_with :rspec do |expectations|
+                expectations.strict_predicate_matchers = strict
+              end
+            end
+          end
+
+          context 'with non-strict matchers (default)' do
+            let(:strict) { false }
+            it { is_expected.to have_strange_result }
+          end
+
+          context 'with strict matchers' do
+            let(:strict) { true }
+            # deliberate failure
+            it { is_expected.to have_strange_result }
+          end
+        end
+        """
+      When I run `rspec strict_or_not.rb`
+      Then the output should contain "2 examples, 1 failure"
+      And the output should contain "has_strange_result?` to return true, got 42"
 
     Scenario: calling private method with be_predicate causes error
       Given a file named "attempting_to_match_private_method_spec.rb" with:

features/test_frameworks/minitest.feature

@@ -47,7 +47,7 @@ Feature: Minitest integration
       """
      When I run `ruby rspec_expectations_test.rb`
      Then the output should contain "4 runs, 5 assertions, 2 failures, 0 errors"
-      And the output should contain "expected `[1, 2].empty?` to return true, got false"
+      And the output should contain "expected `[1, 2].empty?` to be truthy, got false"
       And the output should contain "be_an_int is deprecated"
       And the output should contain "Got 2 failures from failure aggregation block"
 
@@ -105,7 +105,7 @@ Feature: Minitest integration
       """
      When I run `ruby rspec_expectations_spec.rb`
      Then the output should contain "9 runs, 10 assertions, 5 failures, 0 errors"
-      And the output should contain "expected `[1, 2].empty?` to return true, got false"
+      And the output should contain "expected `[1, 2].empty?` to be truthy, got false"
       And the output should contain "expected ZeroDivisionError but nothing was raised"
       And the output should contain "Got 2 failures from failure aggregation block"
       And the output should contain "Expected [1, 2] to be empty?"

lib/rspec/expectations/configuration.rb

@@ -28,6 +28,7 @@ class Configuration
 
       def initialize
         @on_potential_false_positives = :warn
+        @strict_predicate_matchers = false
       end
 
       # Configures the supported syntax.
@@ -185,6 +186,21 @@ def on_potential_false_positives=(behavior)
         @on_potential_false_positives = behavior
       end
 
+
+      # Configures RSpec to check predicate matchers to `be(true)` / `be(false)` (strict),
+      # or `be_truthy` / `be_falsey` (not strict).
+      # Historically, the default was `false`, but `true` is recommended.
+      def strict_predicate_matchers=(flag)
+        raise ArgumentError, "Pass `true` or `false`" unless flag == true || flag == false
+        @strict_predicate_matchers = flag
+      end
+
+      attr_reader :strict_predicate_matchers
+
+      def strict_predicate_matchers?
+        @strict_predicate_matchers
+      end
+
       # Indicates what RSpec will do about matcher use which will
       # potentially cause false positives in tests, generally you want to
       # avoid such scenarios so this defaults to `true`.

lib/rspec/matchers/built_in/has.rb

@@ -30,7 +30,7 @@ def matches?(actual, &block)
         def does_not_match?(actual, &block)
           @actual = actual
           @block ||= block
-          predicate_accessible? && !predicate_matches?
+          predicate_accessible? && predicate_matches?(false)
         end
 
         # @api private
@@ -90,8 +90,12 @@ def predicate_method_name
           predicate
         end
 
-        def predicate_matches?
-          !!predicate_result
+        def predicate_matches?(value=true)
+          if RSpec::Expectations.configuration.strict_predicate_matchers?
+            value == predicate_result
+          else
+            value == !!predicate_result
+          end
         end
 
         def root
@@ -108,7 +112,17 @@ def method_description
 
         def failure_message_expecting(value)
           validity_message ||
-            "expected `#{actual_formatted}.#{predicate}#{args_to_s}` to return #{value}, got #{description_of @predicate_result}"
+            "expected `#{actual_formatted}.#{predicate}#{args_to_s}` to #{expectation_of value}, got #{description_of @predicate_result}"
+        end
+
+        def expectation_of(value)
+          if RSpec::Expectations.configuration.strict_predicate_matchers?
+            "return #{value}"
+          elsif value
+            "be truthy"
+          else
+            "be falsey"
+          end
         end
 
         def validity_message

spec/rspec/matchers/built_in/be_spec.rb

@@ -58,6 +58,35 @@
     }.to fail_with("expected `#{actual.inspect}.happy?` to return true, got nil")
   end
 
+  context "when strict_predicate_matchers is set to true" do
+    it "fails when actual returns 42 for :predicate?" do
+      actual = double("actual", :happy? => 42)
+      expect {
+        expect(actual).to be_happy
+      }.to fail_with("expected `#{actual.inspect}.happy?` to return true, got 42")
+    end
+  end
+
+  context "when strict_predicate_matchers is set to false" do
+    around do |example|
+      RSpec::Expectations.configuration.strict_predicate_matchers = false
+      example.run
+      RSpec::Expectations.configuration.strict_predicate_matchers = true
+    end
+
+    it "passes when actual returns truthy value for :predicate?" do
+      actual = double("actual", :happy? => 42)
+      expect(actual).to be_happy
+    end
+
+    it "states actual predicate used when it fails" do
+      actual = double("actual", :happy? => false)
+      expect {
+        expect(actual).to be_happy
+      }.to fail_with("expected `#{actual.inspect}.happy?` to be truthy, got false")
+    end
+  end
+
   it "fails when actual does not respond to :predicate?" do
     expect {
       expect(Object.new).to be_happy
@@ -184,14 +213,47 @@ def foo?
 end
 
 RSpec.describe "expect(...).not_to be_predicate" do
+  let(:strict_predicate_matchers) { true }
+
+  around do |example|
+    default = RSpec::Expectations.configuration.strict_predicate_matchers?
+    RSpec::Expectations.configuration.strict_predicate_matchers = strict_predicate_matchers
+    example.run
+    RSpec::Expectations.configuration.strict_predicate_matchers = default
+  end
+
   it "passes when actual returns false for :sym?" do
     actual = double("actual", :happy? => false)
     expect(actual).not_to be_happy
   end
 
-  it "passes when actual returns nil for :sym?" do
-    actual = double("actual", :happy? => nil)
-    expect(actual).not_to be_happy
+  context "when strict_predicate_matchers is set to true" do
+    it "fails when actual returns nil for :sym?" do
+      actual = double("actual", :happy? => nil)
+      expect {
+        expect(actual).not_to be_happy
+      }.to fail_with("expected `#{actual.inspect}.happy?` to return false, got nil")
+    end
+  end
+
+  context "when strict_predicate_matchers is set to false" do
+    around do |example|
+      RSpec::Expectations.configuration.strict_predicate_matchers = false
+      example.run
+      RSpec::Expectations.configuration.strict_predicate_matchers = true
+    end
+
+    it "passes when actual returns nil for :sym?" do
+      actual = double("actual", :happy? => nil)
+      expect(actual).not_to be_happy
+    end
+
+    it "shows actual comparision made when it fails" do
+      actual = double("actual", :happy? => 42)
+      expect {
+        expect(actual).not_to be_happy
+      }.to fail_with("expected `#{actual.inspect}.happy?` to be falsey, got 42")
+    end
   end
 
   it "fails when actual returns true for :sym?" do

spec/rspec/matchers/built_in/has_spec.rb

@@ -139,12 +139,26 @@ def o.has_sym?(sym); sym == :foo; end
     expect({ :a => "A" }).not_to have_key(:b)
   end
 
-  it "passes if #has_sym?(*args) returns nil" do
-    klass = Class.new do
-      def has_foo?
-      end
+  context "when strict_predicate_matchers is set to true" do
+    it "fails when #has_sym? returns nil" do
+      actual = double("actual", :has_foo? => nil)
+      expect {
+        expect(actual).not_to have_foo
+      }.to fail_with("expected `#{actual.inspect}.has_foo?` to return false, got nil")
+    end
+  end
+
+  context "when strict_predicate_matchers is set to false" do
+    around do |example|
+      RSpec::Expectations.configuration.strict_predicate_matchers = false
+      example.run
+      RSpec::Expectations.configuration.strict_predicate_matchers = true
+    end
+
+    it "passes if #has_sym?(*args) returns nil" do
+      actual = double("actual", :has_foo? => nil)
+      expect(actual).not_to have_foo
     end
-    expect(klass.new).not_to have_foo
   end
 
   it "fails if #has_sym?(*args) returns true" do

spec/spec_helper.rb

@@ -52,6 +52,7 @@ def hash_inspect(hash)
     $default_expectation_syntax = expectations.syntax # rubocop:disable Style/GlobalVars
     expectations.syntax = :expect
     expectations.include_chain_clauses_in_custom_matcher_descriptions = true
+    expectations.strict_predicate_matchers = true
   end
 
   config.mock_with :rspec do |mocks|