From 4734c9a462aeecedf5b439db310e598efd8538e2 Mon Sep 17 00:00:00 2001
From: Koichi ITO <koic.ito@gmail.com>
Date: Wed, 21 Aug 2019 19:31:54 +0900
Subject: [PATCH] [Fix #111] Fix incorrect autocorrect for `Rails/Presence`

Fixes #111.

This PR fixes 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
```
---
 CHANGELOG.md                            |  1 +
 lib/rubocop/cop/rails/presence.rb       | 24 +++++++++++-
 spec/rubocop/cop/rails/presence_spec.rb | 52 +++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 1 deletion(-)

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 `%<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
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