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 #111] Fix an incorrect autocorrect for Rails/Presence #115

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 @@ -6,6 +6,7 @@

* [#104](https://github.com/rubocop-hq/rubocop-rails/issues/104): Exclude Rails-independent `bin/bundle` by default. ([@koic][])
* [#107](https://github.com/rubocop-hq/rubocop-rails/issues/107): Fix style guide URLs when specifying `rubocop --display-style-guide` option. ([@koic][])
* [#111](https://github.com/rubocop-hq/rubocop-rails/issues/111): Fix an incorrect autocorrect for `Rails/Presence` when method arguments of `else` branch is not enclosed in parentheses. ([@koic][])

## 2.3.0 (2019-08-13)

Expand Down
24 changes: 23 additions & 1 deletion lib/rubocop/cop/rails/presence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ module Rails
# # good
# a.presence || b
class Presence < Cop
include RangeHelp

MSG = 'Use `%<prefer>s` instead of `%<current>s`.'

def_node_matcher :redundant_receiver_and_other, <<-PATTERN
Expand Down Expand Up @@ -115,9 +117,29 @@ def message(node, receiver, other)
end

def replacement(receiver, other)
or_source = other.nil? || other.nil_type? ? '' : " || #{other.source}"
or_source = if other&.send_type?
build_source_for_or_method(other)
else
''
end

"#{receiver.source}.presence" + or_source
end

def build_source_for_or_method(other)
if other.parenthesized? || !other.arguments?
" || #{other.source}"
else
method = range_between(
other.source_range.begin_pos,
other.first_argument.source_range.begin_pos - 1
).source

arguments = other.arguments.map(&:source).join(', ')

" || #{method}(#{arguments})"
end
end
end
end
end
Expand Down
52 changes: 52 additions & 0 deletions spec/rubocop/cop/rails/presence_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,58 @@
.map { |num| num + 2 }.presence || b
FIXED

context 'when a method argument of `else` branch ' \
'is enclosed in parentheses' do
it_behaves_like 'offense', <<~SOURCE.chomp, <<~CORRECTION.chomp, 1, 5
if value.present?
value
else
do_something(value)
end
SOURCE
value.presence || do_something(value)
CORRECTION
end

context 'when a method argument of `else` branch ' \
'is not enclosed in parentheses' do
it_behaves_like 'offense', <<~SOURCE.chomp, <<~CORRECTION.chomp, 1, 5
if value.present?
value
else
do_something value
end
SOURCE
value.presence || do_something(value)
CORRECTION
end

context 'when multiple method arguments of `else` branch ' \
'is not enclosed in parentheses' do
it_behaves_like 'offense', <<~SOURCE.chomp, <<~CORRECTION.chomp, 1, 5
if value.present?
value
else
do_something arg1, arg2
end
SOURCE
value.presence || do_something(arg1, arg2)
CORRECTION
end

context 'when a method argument with a receiver of `else` branch ' \
'is not enclosed in parentheses' do
it_behaves_like 'offense', <<~SOURCE.chomp, <<~CORRECTION.chomp, 1, 5
if value.present?
value
else
foo.do_something value
end
SOURCE
value.presence || foo.do_something(value)
CORRECTION
end

it 'does not register an offense when using `#presence`' do
expect_no_offenses(<<~RUBY)
a.presence
Expand Down