Skip to content

Commit

Permalink
Fix a false positive for RSpec/PredicateMatcher when multiple argum…
Browse files Browse the repository at this point in the history
…ents and not replaceable method

This PR is fix a false positive for `RSpec/PredicateMatcher` when multiple arguments and not replaceable method.

following code:
```ruby
expect(foo).to include(foo, bar)
```

`RSpec/PredicateMatcher` autocorrects it as follows:
```ruby
expect(foo.include?(foo, bar)).to be(true)
```

However, this is a SyntaxError.
```
ArgumentError:
       wrong number of arguments (given 2, expected 1)
```
  • Loading branch information
ydah committed Jan 14, 2023
1 parent 9280a68 commit 0149e4c
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

- Fix a false negative for `RSpec/Pending` when using skipped in metadata is multiline string. ([@ydah])
- Fix a false positive for `RSpec/NoExpectationExample` when using skipped in metadata is multiline string. ([@ydah])
- Fix a false positive for `RSpec/PredicateMatcher` when multiple arguments and not replaceable method. ([@ydah])

## 2.17.0 (2023-01-13)

Expand Down
27 changes: 26 additions & 1 deletion lib/rubocop/cop/rspec/predicate_matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ module InflectedHelper

def check_inflected(node)
predicate_in_actual?(node) do |predicate, to, matcher|
next unless replaceable_predicate?(predicate)

msg = message_inflected(predicate)
add_offense(node, message: msg) do |corrector|
remove_predicate(corrector, predicate)
Expand All @@ -25,6 +27,16 @@ def check_inflected(node)
end
end

def replaceable_predicate?(predicate)
case predicate.method_name.to_s
when 'is_a?', 'instance_of?', 'include?',
'respond_to?', 'exist?', 'exists?', /\Ahas_/
predicate.arguments.count <= 1
else
true
end
end

# @!method predicate_in_actual?(node)
def_node_matcher :predicate_in_actual?, <<-PATTERN
(send
Expand Down Expand Up @@ -118,7 +130,7 @@ def true?(to_symbol, matcher)
end

# A helper for `explicit` style
module ExplicitHelper
module ExplicitHelper # rubocop:disable Metrics/ModuleLength
include RuboCop::RSpec::Language
extend NodePattern::Macros

Expand Down Expand Up @@ -149,12 +161,25 @@ def check_explicit(node) # rubocop:disable Metrics/MethodLength
return if part_of_ignored_node?(node)

predicate_matcher?(node) do |actual, matcher|
next unless replaceable_matcher?(matcher)

add_offense(node, message: message_explicit(matcher)) do |corrector|
corrector_explicit(corrector, node, actual, matcher, matcher)
end
end
end

def replaceable_matcher?(matcher)
case matcher.method_name.to_s
when 'be_a', 'be_an', 'be_a_kind_of', 'a_kind_of', 'be_kind_of',
'be_an_instance_of', 'be_instance_of', 'an_instance_of',
'include', 'respond_to', /\Ahave_/
matcher.arguments.count <= 1
else
true
end
end

# @!method predicate_matcher?(node)
def_node_matcher :predicate_matcher?, <<-PATTERN
(send
Expand Down
33 changes: 32 additions & 1 deletion spec/rubocop/cop/rspec/predicate_matcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,20 @@
RUBY
end

it 'does not register an offense for a predicate method with ' \
'multiple arguments and not replaceable method' do
expect_no_offenses(<<~RUBY)
expect(foo.is_a?(foo, bar)).to be_truthy
expect(foo.instance_of? foo, bar).to be_truthy
expect(foo.include?(foo, bar, baz)).to be_truthy
expect(foo.has_key? foo, bar, baz).to be_truthy
expect(foo.respond_to?(foo, bar)).to be_truthy
expect(foo.exist? foo, bar).to be_truthy
expect(foo.exists?(foo, bar, baz)).to be_truthy
expect(foo.has_key? foo, bar, baz).to be_truthy
RUBY
end

it 'registers an offense for a predicate method with a block' do
expect_offense(<<~RUBY)
expect(foo.all?(&:present?)).to be_truthy
Expand Down Expand Up @@ -295,7 +309,7 @@
RUBY
end

it 'registers an offense for a predicate method with argument' do
it 'registers an offense for a `be_something` with argument' do
expect_offense(<<~RUBY)
expect(foo).to be_something(1)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher.
Expand All @@ -312,6 +326,23 @@
RUBY
end

it 'does not register an offense for a predicate method ' \
'with multiple arguments and not replaceable method' do
expect_no_offenses(<<~RUBY)
expect(foo).to be_a(foo, bar)
expect(foo).to be_an foo, bar
expect(foo).to be_a_kind_of(foo, bar, baz)
expect(foo).to a_kind_of foo, bar, baz
expect(foo).to be_kind_of(foo, bar)
expect(foo).to be_an_instance_of foo, bar
expect(foo).to be_instance_of(foo, bar, baz)
expect(foo).to an_instance_of foo, bar, baz
expect(foo).to include(foo, bar)
expect(foo).to respond_to foo, bar
expect(foo).to have_key(foo, bar, baz)
RUBY
end

it 'registers an offense for a predicate method with a block' do
expect_offense(<<~RUBY)
expect(foo).to be_all { |x| x.present? }
Expand Down

0 comments on commit 0149e4c

Please sign in to comment.