Skip to content

Commit

Permalink
[Fix #8470] Do not try to autocorrect Style/StringConcatenation if an…
Browse files Browse the repository at this point in the history
…y of the parts are too complex.

Complex includes multiline statements, heredocs, nested interpolations and expressions with blocks.
  • Loading branch information
dvandersluis authored and mergify[bot] committed Sep 3, 2020
1 parent fc8a933 commit 5b93f78
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -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]
Expand Down
16 changes: 15 additions & 1 deletion lib/rubocop/cop/style/string_concatenation.rb
Expand Up @@ -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 + '>'
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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|
Expand Down
81 changes: 81 additions & 0 deletions spec/rubocop/cop/style/string_concatenation_spec.rb
Expand Up @@ -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

0 comments on commit 5b93f78

Please sign in to comment.