From 5b93f78085d27a51106070ca7a7d9057cf845506 Mon Sep 17 00:00:00 2001
From: Daniel Vandersluis <daniel.vandersluis@gmail.com>
Date: Wed, 2 Sep 2020 14:19:28 -0400
Subject: [PATCH] [Fix #8470] Do not try to autocorrect
 Style/StringConcatenation if any of the parts are too complex.

Complex includes multiline statements, heredocs, nested interpolations and expressions with blocks.
---
 CHANGELOG.md                                  |  4 +
 docs/modules/ROOT/pages/cops_style.adoc       |  5 ++
 lib/rubocop/cop/style/string_concatenation.rb | 16 +++-
 .../cop/style/string_concatenation_spec.rb    | 81 +++++++++++++++++++
 4 files changed, 105 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 936135cde47..07a5b4ad50e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,10 @@
 
 * [#8627](https://github.com/rubocop-hq/rubocop/issues/8627): Fix a false positive for `Lint/DuplicateRequire` when same feature argument but different require method. ([@koic][])
 
+### Changes
+
+* [#8470](https://github.com/rubocop-hq/rubocop/issues/8470): Do not autocorrect `Style/StringConcatenation` when parts of the expression are too complex. ([@dvandersluis][])
+
 ## 0.90.0 (2020-09-01)
 
 ### New features
diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc
index 44ff2ab2b4c..d1a84bdd840 100644
--- a/docs/modules/ROOT/pages/cops_style.adoc
+++ b/docs/modules/ROOT/pages/cops_style.adoc
@@ -9414,6 +9414,11 @@ warn('hello')
 This cop checks for places where string concatenation
 can be replaced with string interpolation.
 
+The cop can autocorrect simple cases but will skip autocorrecting
+more complex cases where the resulting code would be harder to read.
+In those cases, it might be useful to extract statements to local
+variables or methods which you can then interpolate in a string.
+
 === Examples
 
 [source,ruby]
diff --git a/lib/rubocop/cop/style/string_concatenation.rb b/lib/rubocop/cop/style/string_concatenation.rb
index 074e9979d7c..8936f5bad0f 100644
--- a/lib/rubocop/cop/style/string_concatenation.rb
+++ b/lib/rubocop/cop/style/string_concatenation.rb
@@ -6,6 +6,11 @@ module Style
       # This cop checks for places where string concatenation
       # can be replaced with string interpolation.
       #
+      # The cop can autocorrect simple cases but will skip autocorrecting
+      # more complex cases where the resulting code would be harder to read.
+      # In those cases, it might be useful to extract statements to local
+      # variables or methods which you can then interpolate in a string.
+      #
       # @example
       #   # bad
       #   email_with_name = user.name + ' <' + user.email + '>'
@@ -37,7 +42,9 @@ def on_send(node)
           collect_parts(topmost_plus_node, parts)
 
           add_offense(topmost_plus_node) do |corrector|
-            corrector.replace(topmost_plus_node, replacement(parts))
+            if parts.none? { |part| uncorrectable?(part) }
+              corrector.replace(topmost_plus_node, replacement(parts))
+            end
           end
         end
 
@@ -66,6 +73,13 @@ def plus_node?(node)
           node.send_type? && node.method?(:+)
         end
 
+        def uncorrectable?(part)
+          part.multiline? ||
+            part.dstr_type? ||
+            (part.str_type? && part.heredoc?) ||
+            part.each_descendant(:block).any?
+        end
+
         def replacement(parts)
           interpolated_parts =
             parts.map do |part|
diff --git a/spec/rubocop/cop/style/string_concatenation_spec.rb b/spec/rubocop/cop/style/string_concatenation_spec.rb
index f270b3d5bb0..d0b12cf46b7 100644
--- a/spec/rubocop/cop/style/string_concatenation_spec.rb
+++ b/spec/rubocop/cop/style/string_concatenation_spec.rb
@@ -41,4 +41,85 @@
       user.name + user.email
     RUBY
   end
+
+  context 'multiline' do
+    context 'simple expressions' do
+      it 'registers an offense and corrects' do
+        expect_offense(<<-RUBY)
+          email_with_name = user.name +
+                            ^^^^^^^^^^^ Prefer string interpolation to string concatenation.
+            ' ' +
+            user.email +
+            '\\n'
+        RUBY
+
+        expect_correction(<<-RUBY)
+          email_with_name = "\#{user.name} \#{user.email}\\\\n"
+        RUBY
+      end
+    end
+
+    context 'if condition' do
+      it 'registers an offense but does not correct' do
+        expect_offense(<<~RUBY)
+          "result:" + if condition
+          ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer string interpolation to string concatenation.
+            "true"
+          else
+            "false"
+          end
+        RUBY
+
+        expect_no_corrections
+      end
+    end
+
+    context 'multiline block' do
+      it 'registers an offense but does not correct' do
+        expect_offense(<<~RUBY)
+          '(' + values.map do |v|
+          ^^^^^^^^^^^^^^^^^^^^^^^ Prefer string interpolation to string concatenation.
+              v.titleize
+          end.join(', ') + ')'
+        RUBY
+
+        expect_no_corrections
+      end
+    end
+  end
+
+  context 'nested interpolation' do
+    it 'registers an offense but does not correct' do
+      expect_offense(<<~RUBY)
+        "foo" + "bar: \#{baz}"
+        ^^^^^^^^^^^^^^^^^^^^^ Prefer string interpolation to string concatenation.
+      RUBY
+
+      expect_no_corrections
+    end
+  end
+
+  context 'inline block' do
+    it 'registers an offense but does not correct' do
+      expect_offense(<<~RUBY)
+        '(' + values.map { |v| v.titleize }.join(', ') + ')'
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer string interpolation to string concatenation.
+      RUBY
+
+      expect_no_corrections
+    end
+  end
+
+  context 'heredoc' do
+    it 'registers an offense but does not correct' do
+      expect_offense(<<~RUBY)
+        "foo" + <<~STR
+        ^^^^^^^^^^^^^^ Prefer string interpolation to string concatenation.
+          text
+        STR
+      RUBY
+
+      expect_no_corrections
+    end
+  end
 end