Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a false positive for RSpec/PredicateMatcher when multiple arguments and not replaceable method #1554

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- Fix a false positive for `RSpec/ContextMethod` when multi-line context with `#` at the beginning. ([@ydah])
- Extract Capybara cops to a separate repository. ([@pirj])
- Fix an incorrect autocorrect for `RSpec/PredicateMatcher` when multiline expect and predicate method with heredoc. ([@ydah])
- Fix a false positive for `RSpec/PredicateMatcher` when `include` with multiple argument. ([@ydah])

## 2.17.0 (2023-01-13)

Expand Down
11 changes: 11 additions & 0 deletions lib/rubocop/cop/rspec/predicate_matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ 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|
next if uncorrectable_matcher?(node, matcher)

Expand All @@ -157,6 +159,15 @@ def check_explicit(node) # rubocop:disable Metrics/MethodLength
end
end

def replaceable_matcher?(matcher)
case matcher.method_name.to_s
when 'include'
matcher.arguments.one?
else
true
end
end

def uncorrectable_matcher?(node, matcher)
heredoc_argument?(matcher) && !same_line?(node, matcher)
end
Expand Down
44 changes: 29 additions & 15 deletions spec/rubocop/cop/rspec/predicate_matcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,14 @@

it 'registers an offense for a predicate method with heredoc' do
expect_offense(<<~RUBY)
expect(foo.include?(<<~TEXT)).to be_truthy
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `include` matcher over `include?`.
expect(foo.something?(<<~TEXT)).to be_truthy
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `be_something` matcher over `something?`.
bar
TEXT
RUBY

expect_correction(<<~RUBY)
expect(foo).to include(<<~TEXT)
expect(foo).to be_something(<<~TEXT)
bar
TEXT
RUBY
Expand Down Expand Up @@ -346,14 +346,14 @@
'heredoc and multiline expect' do
expect_offense(<<~RUBY)
expect(foo)
^^^^^^^^^^^ Prefer using `include?` over `include` matcher.
.to include(<<~TEXT)
^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher.
.to be_something(<<~TEXT)
bar
TEXT

expect(foo)
^^^^^^^^^^^ Prefer using `include?` over `include` matcher.
.to include(bar, <<~TEXT, 'baz')
^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher.
.to be_something(bar, <<~TEXT, 'baz')
bar
TEXT
RUBY
Expand All @@ -365,14 +365,14 @@
'heredoc include #{} and multiline expect' do
expect_offense(<<~'RUBY')
expect(foo)
^^^^^^^^^^^ Prefer using `include?` over `include` matcher.
.to include(<<~TEXT)
^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher.
.to be_something(<<~TEXT)
#{bar}
TEXT

expect(foo)
^^^^^^^^^^^ Prefer using `include?` over `include` matcher.
.to include(bar, <<~TEXT, 'baz')
^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher.
.to be_something(bar, <<~TEXT, 'baz')
#{bar}
TEXT
RUBY
Expand All @@ -384,21 +384,35 @@
'heredoc surrounded by back ticks and multiline expect' do
expect_offense(<<~'RUBY')
expect(foo)
^^^^^^^^^^^ Prefer using `include?` over `include` matcher.
.to include(<<~`COMMAND`)
^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher.
.to be_something(<<~`COMMAND`)
pwd
COMMAND

expect(foo)
^^^^^^^^^^^ Prefer using `include?` over `include` matcher.
.to include(bar, <<~COMMAND, 'baz')
^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher.
.to be_something(bar, <<~COMMAND, 'baz')
pwd
COMMAND
RUBY

expect_no_corrections
end

it 'does not register an offense for a `include` ' \
'with no argument' do
expect_no_offenses(<<~RUBY)
expect(foo).to include
RUBY
end

it 'does not register an offense for a `include` ' \
'with multiple arguments' do
expect_no_offenses(<<~RUBY)
expect(foo).to include(foo, bar)
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