Skip to content

Commit

Permalink
[Fix #111] Fix an incorrect autocorrect for Rails/Presence
Browse files Browse the repository at this point in the history
Fixes #111.

This PR fixes an incorrect autocorrect for `Rails/Presence` when method
arguments of `else` branch is not enclosed in parentheses.

The following is a reproduction procedure.

```console
% cat example.rb
if value.present?
  value
else
  do_something value
end
```

```console
% bundle exec rubocop -a --only Rails/Presence
Inspecting 1 file
E

Offenses:

example.rb:1:1: C: [Corrected] Rails/Presence: Use value.presence ||
do_something value instead of if value.present?
  value
else
  do_something value
end.
if value.present? ...
^^^^^^^^^^^^^^^^^
example.rb:1:32: E: Lint/Syntax: unexpected token tIDENTIFIER
(Using Ruby 2.7 parser; configure using TargetRubyVersion parameter,
under AllCops)
value.presence || do_something value
                               ^^^^^

1 file inspected, 2 offenses detected, 1 offense corrected
```

```console
% cat example.rb
value.presence || do_something value
```

The auto-corrected code is a syntax error.

```console
% ruby example.rb
example.rb:3: syntax error, unexpected local variable or method,
expecting `do' or '{' or '('
....presence || do_something value
```
  • Loading branch information
koic committed Aug 26, 2019
1 parent e0bfc5e commit ce2937d
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 1 deletion.
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

0 comments on commit ce2937d

Please sign in to comment.