Skip to content

Commit

Permalink
Merge pull request #1555 from ydah/fix-incorrect-autocorrect-rspec-Pr…
Browse files Browse the repository at this point in the history
…edicateMatcher

Fix an incorrect autocorrect for `RSpec/PredicateMatcher` when multiline expect and predicate method with heredoc
  • Loading branch information
bquorning committed Jan 16, 2023
1 parent 5c4b6a4 commit 9e8c975
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,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/ContextMethod` when multi-line context with `#` at the beginning. ([@ydah])
- Fix an incorrect autocorrect for `RSpec/PredicateMatcher` when multiline expect and predicate method with heredoc. ([@ydah])

## 2.17.0 (2023-01-13)

Expand Down
11 changes: 11 additions & 0 deletions docs/modules/ROOT/pages/cops_rspec.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -3968,6 +3968,17 @@ expect(foo).to be_something
# good - the above code is rewritten to it by this cop
expect(foo.something?).to be(true)
# bad - no autocorrect
expect(foo)
.to be_something(<<~TEXT)
bar
TEXT
# good
expect(foo.something?(<<~TEXT)).to be(true)
bar
TEXT
----

==== Strict: false, EnforcedStyle: explicit
Expand Down
25 changes: 24 additions & 1 deletion lib/rubocop/cop/rspec/predicate_matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,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 @@ -150,11 +150,23 @@ def check_explicit(node) # rubocop:disable Metrics/MethodLength

predicate_matcher?(node) do |actual, matcher|
add_offense(node, message: message_explicit(matcher)) do |corrector|
next if uncorrectable_matcher?(node, matcher)

corrector_explicit(corrector, node, actual, matcher, matcher)
end
end
end

def uncorrectable_matcher?(node, matcher)
heredoc_argument?(matcher) && !same_line?(node, matcher)
end

def heredoc_argument?(matcher)
matcher.arguments.select do |arg|
%i[str dstr xstr].include?(arg.type)
end.any?(&:heredoc?)
end

# @!method predicate_matcher?(node)
def_node_matcher :predicate_matcher?, <<-PATTERN
(send
Expand Down Expand Up @@ -271,6 +283,17 @@ def replacement_matcher(node)
# # good - the above code is rewritten to it by this cop
# expect(foo.something?).to be(true)
#
# # bad - no autocorrect
# expect(foo)
# .to be_something(<<~TEXT)
# bar
# TEXT
#
# # good
# expect(foo.something?(<<~TEXT)).to be(true)
# bar
# TEXT
#
# @example Strict: false, EnforcedStyle: explicit
# # bad
# expect(foo).to be_something
Expand Down
87 changes: 87 additions & 0 deletions spec/rubocop/cop/rspec/predicate_matcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,21 @@
RUBY
end

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?`.
bar
TEXT
RUBY

expect_correction(<<~RUBY)
expect(foo).to include(<<~TEXT)
bar
TEXT
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 @@ -312,6 +327,78 @@
RUBY
end

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

expect_correction(<<~RUBY)
expect(foo.include?(<<~TEXT)).to #{matcher_true}
bar
TEXT
RUBY
end

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

expect_no_corrections
end

it 'registers an offense for a predicate method with ' \
'heredoc include #{} and multiline expect' do
expect_offense(<<~'RUBY')
expect(foo)
^^^^^^^^^^^ Prefer using `include?` over `include` matcher.
.to include(<<~TEXT)
#{bar}
TEXT
expect(foo)
^^^^^^^^^^^ Prefer using `include?` over `include` matcher.
.to include(bar, <<~TEXT, 'baz')
#{bar}
TEXT
RUBY

expect_no_corrections
end

it 'registers an offense for a predicate method with ' \
'heredoc surrounded by back ticks and multiline expect' do
expect_offense(<<~'RUBY')
expect(foo)
^^^^^^^^^^^ Prefer using `include?` over `include` matcher.
.to include(<<~`COMMAND`)
pwd
COMMAND
expect(foo)
^^^^^^^^^^^ Prefer using `include?` over `include` matcher.
.to include(bar, <<~COMMAND, 'baz')
pwd
COMMAND
RUBY

expect_no_corrections
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 9e8c975

Please sign in to comment.