Skip to content

Commit

Permalink
Merge pull request #1930 from Earlopain/false-negative-predicate-matcher
Browse files Browse the repository at this point in the history
Fix false negative for `RSpec/PredicateMatcher` when expectation contains custom failure message
  • Loading branch information
pirj committed Jun 19, 2024
2 parents 781a866 + 7586bc3 commit 11f5cb8
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Master (Unreleased)

- Fix wrong autocorrect for `RSpec/ScatteredSetup` when hook contains heredoc. ([@earlopain])
- Fix false negative for `RSpec/PredicateMatcher` when expectation contains custom failure message. ([@earlopain])

## 3.0.1 (2024-06-11)

Expand Down
10 changes: 7 additions & 3 deletions lib/rubocop/cop/rspec/predicate_matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def check_inflected(node)
(block $(send !nil? #predicate? ...) ...)
$(send !nil? #predicate? ...)})
$#Runners.all
$#boolean_matcher?)
$#boolean_matcher? ...)
PATTERN

# @!method be_bool?(node)
Expand Down Expand Up @@ -183,8 +183,12 @@ def heredoc_argument?(matcher)
(send
(send nil? :expect $!nil?)
#Runners.all
{$(send nil? #predicate_matcher_name? ...)
(block $(send nil? #predicate_matcher_name? ...) ...)})
{
$(send nil? #predicate_matcher_name? ...)
(block $(send nil? #predicate_matcher_name? ...) ...)
}
...
)
PATTERN

# @!method predicate_matcher_block?(node)
Expand Down
26 changes: 26 additions & 0 deletions spec/rubocop/cop/rspec/predicate_matcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
expect_offense(<<~RUBY)
expect(foo.empty?).to be_truthy
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `be_empty` matcher over `empty?`.
expect(foo.empty?).to be_truthy, 'fail'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `be_empty` matcher over `empty?`.
expect(foo.empty?).not_to be_truthy
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `be_empty` matcher over `empty?`.
expect(foo.empty?).to_not be_truthy
Expand All @@ -30,6 +32,7 @@

expect_correction(<<~RUBY)
expect(foo).to be_empty
expect(foo).to be_empty, 'fail'
expect(foo).not_to be_empty
expect(foo).not_to be_empty
expect(foo).not_to be_empty
Expand All @@ -42,6 +45,8 @@
expect_offense(<<~RUBY)
expect(foo.exist?).to be_truthy
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `exist` matcher over `exist?`.
expect(foo.exist?).to be_truthy, 'fail'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `exist` matcher over `exist?`.
expect(foo.exists?).to be_truthy
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `exist` matcher over `exists?`.
expect(foo.has_something?).to be_truthy
Expand All @@ -58,6 +63,7 @@

expect_correction(<<~RUBY)
expect(foo).to exist
expect(foo).to exist, 'fail'
expect(foo).to exist
expect(foo).to have_something
expect(foo).not_to have_something
Expand All @@ -71,6 +77,8 @@
expect_offense(<<~RUBY)
expect(foo.something?('foo')).to be_truthy
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `be_something` matcher over `something?`.
expect(foo.something?('foo')).to be_truthy, 'fail'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `be_something` matcher over `something?`.
expect(foo.something?('foo', 'bar')).to be_truthy
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `be_something` matcher over `something?`.
expect(foo.something? 1, 2).to be_truthy
Expand All @@ -85,6 +93,7 @@

expect_correction(<<~RUBY)
expect(foo).to be_something('foo')
expect(foo).to be_something('foo'), 'fail'
expect(foo).to be_something('foo', 'bar')
expect(foo).to be_something 1, 2
expect(foo).to have_key('foo')
Expand Down Expand Up @@ -112,6 +121,8 @@
expect_offense(<<~RUBY)
expect(foo.all?(&:present?)).to be_truthy
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `be_all` matcher over `all?`.
expect(foo.all?(&:present?)).to be_truthy, 'fail'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `be_all` matcher over `all?`.
expect(foo.all? { |x| x.present? }).to be_truthy
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `be_all` matcher over `all?`.
expect(foo.all?(n) { |x| x.present? }).to be_truthy
Expand All @@ -134,6 +145,7 @@

expect_correction(<<~RUBY)
expect(foo).to be_all(&:present?)
expect(foo).to be_all(&:present?), 'fail'
expect(foo).to be_all { |x| x.present? }
expect(foo).to be_all(n) { |x| x.present? }
expect(foo).to be_all { present }
Expand All @@ -151,13 +163,15 @@
it 'accepts a predicate method that is not checked true/false' do
expect_no_offenses(<<~RUBY)
expect(foo.something?).to eq "something"
expect(foo.something?).to eq "something", "fail"
expect(foo.has_something?).to eq "something"
RUBY
end

it 'accepts non-predicate method' do
expect_no_offenses(<<~RUBY)
expect(foo.something).to be(true)
expect(foo.something).to be(true), 'fail'
expect(foo.has_something).to be(true)
RUBY
end
Expand All @@ -171,6 +185,7 @@
it 'accepts strict checking boolean matcher' do
expect_no_offenses(<<~RUBY)
expect(foo.empty?).to eq(true)
expect(foo.empty?).to eq(true), 'fail'
expect(foo.empty?).to be(true)
expect(foo.empty?).to be(false)
expect(foo.empty?).not_to be true
Expand All @@ -188,6 +203,8 @@
expect_offense(<<~RUBY)
expect(foo.empty?).to eq(true)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `be_empty` matcher over `empty?`.
expect(foo.empty?).to eq(true), 'fail'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `be_empty` matcher over `empty?`.
expect(foo.empty?).not_to be(true)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `be_empty` matcher over `empty?`.
expect(foo.empty?).to be(true)
Expand All @@ -202,6 +219,7 @@

expect_correction(<<~RUBY)
expect(foo).to be_empty
expect(foo).to be_empty, 'fail'
expect(foo).not_to be_empty
expect(foo).to be_empty
expect(foo).not_to be_empty
Expand All @@ -220,6 +238,8 @@
expect_offense(<<~RUBY)
expect(foo).to be_empty
^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `empty?` over `be_empty` matcher.
expect(foo).to be_empty, 'fail'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `empty?` over `be_empty` matcher.
expect(foo).not_to be_empty
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `empty?` over `be_empty` matcher.
expect(foo).to have_something
Expand Down Expand Up @@ -252,6 +272,7 @@
it 'accepts built in matchers' do
expect_no_offenses(<<~RUBY)
expect(foo).to be_truthy
expect(foo).to be_truthy, 'fail'
expect(foo).to be_falsey
expect(foo).to be_falsy
expect(foo).to have_attributes(name: 'foo')
Expand All @@ -275,13 +296,16 @@
it 'accepts non-predicate matcher' do
expect_no_offenses(<<~RUBY)
expect(foo).to be(true)
expect(foo).to be(true), 'fail'
RUBY
end

it 'registers an offense for a predicate method with built-in equiv' do
expect_offense(<<~RUBY)
expect(foo).to be_something
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher.
expect(foo).to be_something, 'fail'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher.
expect(foo).not_to be_something
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher.
expect(foo).to have_something
Expand All @@ -300,6 +324,7 @@

expect_correction(<<~RUBY)
expect(foo.something?).to #{matcher_true}
expect(foo.something?).to #{matcher_true}, 'fail'
expect(foo.something?).to #{matcher_false}
expect(foo.has_something?).to #{matcher_true}
expect(foo.is_a?(Array)).to #{matcher_true}
Expand Down Expand Up @@ -403,6 +428,7 @@
'with no argument' do
expect_no_offenses(<<~RUBY)
expect(foo).to include
expect(foo).to include, 'fail'
RUBY
end

Expand Down

0 comments on commit 11f5cb8

Please sign in to comment.