Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix #8470] Do not try to autocorrect Style/StringConcatenation if any of the parts are too complex #8634

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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