Merge pull request #115 from koic/fix_incorrect_autocorrect_for_rails…

…_presence

[Fix #111] Fix an incorrect autocorrect for `Rails/Presence`

CHANGELOG.md

@@ -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)
 

lib/rubocop/cop/rails/presence.rb

@@ -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
@@ -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

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