diff --git a/CHANGELOG.md b/CHANGELOG.md index 65902ed464..d2d2b87087 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Bug fixes * [#104](https://github.com/rubocop-hq/rubocop-rails/issues/104): Exclude Rails-independent `bin/bundle` by default. ([@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) diff --git a/lib/rubocop/cop/rails/presence.rb b/lib/rubocop/cop/rails/presence.rb index d5a74ef379..73b9587269 100644 --- a/lib/rubocop/cop/rails/presence.rb +++ b/lib/rubocop/cop/rails/presence.rb @@ -38,6 +38,8 @@ module Rails # # good # a.presence || b class Presence < Cop + include RangeHelp + MSG = 'Use `%s` instead of `%s`.' def_node_matcher :redundant_receiver_and_other, <<-PATTERN @@ -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 diff --git a/spec/rubocop/cop/rails/presence_spec.rb b/spec/rubocop/cop/rails/presence_spec.rb index 642ac51e1d..304d83183b 100644 --- a/spec/rubocop/cop/rails/presence_spec.rb +++ b/spec/rubocop/cop/rails/presence_spec.rb @@ -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