Skip to content

Commit ce2937d

Browse files
committed
[Fix #111] Fix an incorrect autocorrect for Rails/Presence
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 ```
1 parent e0bfc5e commit ce2937d

File tree

3 files changed

+76
-1
lines changed

3 files changed

+76
-1
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
* [#104](https://github.com/rubocop-hq/rubocop-rails/issues/104): Exclude Rails-independent `bin/bundle` by default. ([@koic][])
88
* [#107](https://github.com/rubocop-hq/rubocop-rails/issues/107): Fix style guide URLs when specifying `rubocop --display-style-guide` option. ([@koic][])
9+
* [#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][])
910

1011
## 2.3.0 (2019-08-13)
1112

lib/rubocop/cop/rails/presence.rb

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ module Rails
3838
# # good
3939
# a.presence || b
4040
class Presence < Cop
41+
include RangeHelp
42+
4143
MSG = 'Use `%<prefer>s` instead of `%<current>s`.'
4244

4345
def_node_matcher :redundant_receiver_and_other, <<-PATTERN
@@ -115,9 +117,29 @@ def message(node, receiver, other)
115117
end
116118

117119
def replacement(receiver, other)
118-
or_source = other.nil? || other.nil_type? ? '' : " || #{other.source}"
120+
or_source = if other&.send_type?
121+
build_source_for_or_method(other)
122+
else
123+
''
124+
end
125+
119126
"#{receiver.source}.presence" + or_source
120127
end
128+
129+
def build_source_for_or_method(other)
130+
if other.parenthesized? || !other.arguments?
131+
" || #{other.source}"
132+
else
133+
method = range_between(
134+
other.source_range.begin_pos,
135+
other.first_argument.source_range.begin_pos - 1
136+
).source
137+
138+
arguments = other.arguments.map(&:source).join(', ')
139+
140+
" || #{method}(#{arguments})"
141+
end
142+
end
121143
end
122144
end
123145
end

spec/rubocop/cop/rails/presence_spec.rb

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,58 @@
7373
.map { |num| num + 2 }.presence || b
7474
FIXED
7575

76+
context 'when a method argument of `else` branch ' \
77+
'is enclosed in parentheses' do
78+
it_behaves_like 'offense', <<~SOURCE.chomp, <<~CORRECTION.chomp, 1, 5
79+
if value.present?
80+
value
81+
else
82+
do_something(value)
83+
end
84+
SOURCE
85+
value.presence || do_something(value)
86+
CORRECTION
87+
end
88+
89+
context 'when a method argument of `else` branch ' \
90+
'is not enclosed in parentheses' do
91+
it_behaves_like 'offense', <<~SOURCE.chomp, <<~CORRECTION.chomp, 1, 5
92+
if value.present?
93+
value
94+
else
95+
do_something value
96+
end
97+
SOURCE
98+
value.presence || do_something(value)
99+
CORRECTION
100+
end
101+
102+
context 'when multiple method arguments of `else` branch ' \
103+
'is not enclosed in parentheses' do
104+
it_behaves_like 'offense', <<~SOURCE.chomp, <<~CORRECTION.chomp, 1, 5
105+
if value.present?
106+
value
107+
else
108+
do_something arg1, arg2
109+
end
110+
SOURCE
111+
value.presence || do_something(arg1, arg2)
112+
CORRECTION
113+
end
114+
115+
context 'when a method argument with a receiver of `else` branch ' \
116+
'is not enclosed in parentheses' do
117+
it_behaves_like 'offense', <<~SOURCE.chomp, <<~CORRECTION.chomp, 1, 5
118+
if value.present?
119+
value
120+
else
121+
foo.do_something value
122+
end
123+
SOURCE
124+
value.presence || foo.do_something(value)
125+
CORRECTION
126+
end
127+
76128
it 'does not register an offense when using `#presence`' do
77129
expect_no_offenses(<<~RUBY)
78130
a.presence

0 commit comments

Comments
 (0)