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

Add new Style/StringConcatenation cop #8378

Merged
merged 1 commit into from Jul 23, 2020
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -6,6 +6,7 @@

* [#8322](https://github.com/rubocop-hq/rubocop/pull/8322): Support autocorrect for `Style/CaseEquality` cop. ([@fatkodima][])
* [#8339](https://github.com/rubocop-hq/rubocop/pull/8339): Add `Config#for_badge` as an efficient way to get a cop's config merged with its department's. ([@marcandre][])
* [#5067](https://github.com/rubocop-hq/rubocop/issues/5067): Add new `Style/StringConcatenation` cop. ([@fatkodima][])

### Bug fixes

Expand Down
7 changes: 7 additions & 0 deletions config/default.yml
Expand Up @@ -3939,6 +3939,13 @@ Style/StderrPuts:
Enabled: true
VersionAdded: '0.51'

Style/StringConcatenation:
Description: 'Checks for places where string concatenation can be replaced with string interpolation.'
StyleGuide: '#string-interpolation'
Enabled: pending
Safe: false
VersionAdded: '0.89'

Style/StringHashKeys:
Description: 'Prefer symbols instead of strings as hash keys.'
StyleGuide: '#symbols-as-keys'
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -471,6 +471,7 @@ In the following section you find all available cops:
* xref:cops_style.adoc#stylespecialglobalvars[Style/SpecialGlobalVars]
* xref:cops_style.adoc#stylestabbylambdaparentheses[Style/StabbyLambdaParentheses]
* xref:cops_style.adoc#stylestderrputs[Style/StderrPuts]
* xref:cops_style.adoc#stylestringconcatenation[Style/StringConcatenation]
* xref:cops_style.adoc#stylestringhashkeys[Style/StringHashKeys]
* xref:cops_style.adoc#stylestringliterals[Style/StringLiterals]
* xref:cops_style.adoc#stylestringliteralsininterpolation[Style/StringLiteralsInInterpolation]
Expand Down
31 changes: 31 additions & 0 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -8944,6 +8944,37 @@ warn('hello')

* https://rubystyle.guide#warn

== Style/StringConcatenation

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| No
| Yes (Unsafe)
| 0.89
| -
|===

This cop checks for places where string concatenation
can be replaced with string interpolation.

=== Examples

[source,ruby]
----
# bad
email_with_name = user.name + ' <' + user.email + '>'

# good
email_with_name = "#{user.name} <#{user.email}>"
email_with_name = format('%s <%s>', user.name, user.email)
----

=== References

* https://rubystyle.guide#string-interpolation

== Style/StringHashKeys

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -516,6 +516,7 @@
require_relative 'rubocop/cop/style/special_global_vars'
require_relative 'rubocop/cop/style/stabby_lambda_parentheses'
require_relative 'rubocop/cop/style/stderr_puts'
require_relative 'rubocop/cop/style/string_concatenation'
require_relative 'rubocop/cop/style/string_hash_keys'
require_relative 'rubocop/cop/style/string_literals'
require_relative 'rubocop/cop/style/string_literals_in_interpolation'
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cli/command/auto_genenerate_config.rb
Expand Up @@ -113,7 +113,7 @@ def add_inheritance_from_auto_generated_file(config_file)
return if files.include?(AUTO_GENERATED_FILE)

files.unshift(AUTO_GENERATED_FILE)
file_string = "\n - " + files.join("\n - ") if files.size > 1
file_string = "\n - #{files.join("\n - ")}" if files.size > 1
rubocop_yml_contents = existing_configuration(config_file)
end

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cli/command/show_cops.rb
Expand Up @@ -68,7 +68,7 @@ def cops_of_department(cops, department)

def config_lines(cop)
cnf = @config.for_cop(cop)
cnf.to_yaml.lines.to_a.drop(1).map { |line| ' ' + line }
cnf.to_yaml.lines.to_a.drop(1).map { |line| " #{line}" }
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/comment_config.rb
Expand Up @@ -11,7 +11,7 @@ class CommentConfig
COPS_PATTERN = "(all|#{COP_NAMES_PATTERN})"

COMMENT_DIRECTIVE_REGEXP = Regexp.new(
('# rubocop : ((?:disable|enable|todo))\b ' + COPS_PATTERN)
"# rubocop : ((?:disable|enable|todo))\\b #{COPS_PATTERN}"
.gsub(' ', '\s*')
)

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/config_loader_resolver.rb
Expand Up @@ -201,7 +201,7 @@ def handle_disabled_by_default(config, new_default_configuration)
next unless dept_params['Enabled']

new_default_configuration.each do |cop, params|
next unless cop.start_with?(dept + '/')
next unless cop.start_with?("#{dept}/")

# Retain original default configuration for cops in the department.
params['Enabled'] = ConfigLoader.default_configuration[cop]['Enabled']
Expand Down
6 changes: 3 additions & 3 deletions lib/rubocop/cop/correctors/line_break_corrector.rb
Expand Up @@ -29,8 +29,8 @@ def break_line_before(range:, node:, corrector:, indent_steps: 1,
configured_width:)
corrector.insert_before(
range,
"\n" + ' ' * (node.loc.keyword.column +
indent_steps * configured_width)
"\n#{' ' * (node.loc.keyword.column +
indent_steps * configured_width)}"
)
end

Expand All @@ -39,7 +39,7 @@ def move_comment(eol_comment:, node:, corrector:)

text = eol_comment.loc.expression.source
corrector.insert_before(node,
text + "\n" + (' ' * node.loc.keyword.column))
"#{text}\n#{' ' * node.loc.keyword.column}")
corrector.remove(eol_comment)
end

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/correctors/percent_literal_corrector.rb
Expand Up @@ -110,7 +110,7 @@ def substitute_escaped_delimiters(content, delimiters)

def end_content(source)
result = /\A(\s*)\]/.match(source.split("\n").last)
("\n" + result[1]) if result
"\n#{result[1]}" if result
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/correctors/punctuation_corrector.rb
Expand Up @@ -10,7 +10,7 @@ def remove_space(space_before)
end

def add_space(token)
->(corrector) { corrector.replace(token.pos, token.pos.source + ' ') }
->(corrector) { corrector.replace(token.pos, "#{token.pos.source} ") }
end

def swap_comma(range)
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/generator/configuration_injector.rb
Expand Up @@ -25,9 +25,9 @@ def inject
target_line = find_target_line
if target_line
configuration_entries.insert(target_line,
new_configuration_entry + "\n")
"#{new_configuration_entry}\n")
else
configuration_entries.push("\n" + new_configuration_entry)
configuration_entries.push("\n#{new_configuration_entry}")
end

File.write(configuration_file_path, configuration_entries.join)
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/layout/block_alignment.rb
Expand Up @@ -213,7 +213,7 @@ def alt_start_msg(start_loc, source_line_column)
start_loc.column == source_line_column[:column]
''
else
' or ' + format_source_line_column(source_line_column)
" or #{format_source_line_column(source_line_column)}"
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/lint/heredoc_method_call_position.rb
Expand Up @@ -148,7 +148,7 @@ def call_range_to_safely_reposition(node, heredoc)
end

def trailing_comma?(call_source, call_line_source)
call_source + ',' == call_line_source
"#{call_source}," == call_line_source
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/hash_syntax.rb
Expand Up @@ -174,7 +174,7 @@ def autocorrect_ruby19(corrector, pair_node)

corrector.replace(
range,
range.source.sub(/^:(.*\S)\s*=>\s*$/, space.to_s + '\1: ')
range.source.sub(/^:(.*\S)\s*=>\s*$/, "#{space}\\1: ")
)

hash_node = pair_node.parent
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/infinite_loop.rb
Expand Up @@ -99,7 +99,7 @@ def replace_source(range, replacement)
def modifier_replacement(node)
body = node.body
if node.single_line?
'loop { ' + body.source + ' }'
"loop { #{body.source} }"
else
indentation = body.source_range.source_line[LEADING_SPACE]

Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/style/multiline_memoization.rb
Expand Up @@ -77,13 +77,13 @@ def keyword_begin_str(node, node_buf)
if node_buf.source[node.loc.begin.end_pos] == "\n"
'begin'
else
"begin\n" + (' ' * (node.loc.column + indent))
"begin\n#{' ' * (node.loc.column + indent)}"
end
end

def keyword_end_str(node, node_buf)
if /[^\s)]/.match?(node_buf.source_line(node.loc.end.line))
"\n" + (' ' * node.loc.column) + 'end'
"\n#{' ' * node.loc.column}end"
else
'end'
end
Expand Down
92 changes: 92 additions & 0 deletions lib/rubocop/cop/style/string_concatenation.rb
@@ -0,0 +1,92 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# This cop checks for places where string concatenation
# can be replaced with string interpolation.
#
# @example
# # bad
# email_with_name = user.name + ' <' + user.email + '>'
#
# # good
# email_with_name = "#{user.name} <#{user.email}>"
# email_with_name = format('%s <%s>', user.name, user.email)
#
class StringConcatenation < Base
include Util
extend AutoCorrector

MSG = 'Prefer string interpolation instead of string concatenation.'

def_node_matcher :string_concatenation?, <<~PATTERN
{
(send str_type? :+ _)
(send _ :+ str_type?)
}
PATTERN

def on_send(node)
return unless node.method?(:+)
return unless string_concatenation?(node)

topmost_plus_node = find_topmost_plus_node(node)

parts = []
collect_parts(topmost_plus_node, parts)

add_offense(topmost_plus_node) do |corrector|
corrector.replace(topmost_plus_node, replacement(parts))
end
end

private

def find_topmost_plus_node(node)
current = node
while (parent = current.parent) && plus_node?(parent)
current = parent
end
current
end

def collect_parts(node, parts)
return unless node

if plus_node?(node)
collect_parts(node.receiver, parts)
collect_parts(node.first_argument, parts)
else
parts << node
end
end

def plus_node?(node)
node.send_type? && node.method?(:+)
end

def replacement(parts)
interpolated_parts =
parts.map do |part|
if part.str_type?
if single_quoted?(part)
part.value.gsub('\\') { '\\\\' }
else
escape_string(part.value)
end
else
"\#{#{part.source}}"
end
end

"\"#{interpolated_parts.join}\""
end

def single_quoted?(str_node)
str_node.source.start_with?("'")
end
end
end
end
end
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/symbol_array.rb
Expand Up @@ -74,7 +74,7 @@ def correct_bracketed(node)
if c.dsym_type?
string_literal = to_string_literal(c.source)

':' + trim_string_interporation_escape_character(string_literal)
":#{trim_string_interporation_escape_character(string_literal)}"
else
to_symbol_literal(c.value.to_s)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/symbol_proc.rb
Expand Up @@ -84,7 +84,7 @@ def autocorrect_with_args(corrector, node, args, method_name)
arg_range = args.last.source_range
arg_range = range_with_surrounding_comma(arg_range, :right)
replacement = " &:#{method_name}"
replacement = ',' + replacement unless arg_range.source.end_with?(',')
replacement = ",#{replacement}" unless arg_range.source.end_with?(',')
corrector.insert_after(arg_range, replacement)
corrector.remove(block_range_with_space(node))
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/trailing_method_end_statement.rb
Expand Up @@ -45,7 +45,7 @@ def on_def(node)
add_offense(node.loc.end) do |corrector|
corrector.insert_before(
node.loc.end,
"\n" + ' ' * node.loc.keyword.column
"\n#{' ' * node.loc.keyword.column}"
)
end
end
Expand Down
6 changes: 3 additions & 3 deletions spec/rubocop/cli/cli_options_spec.rb
Expand Up @@ -144,7 +144,7 @@
it 'exits cleanly' do
expect(cli.run(['-v'])).to eq(0)
expect(cli.run(['--version'])).to eq(0)
expect($stdout.string).to eq((RuboCop::Version::STRING + "\n") * 2)
expect($stdout.string).to eq("#{RuboCop::Version::STRING}\n" * 2)
end
end

Expand Down Expand Up @@ -537,7 +537,7 @@ class SomeCop < Cop
context 'when a namespace is given' do
it 'runs all enabled cops in that namespace' do
create_file('example.rb', ['if x== 100000000000000 ',
' ' + '#' * 130,
" #{'#' * 130}",
"\ty",
'end'])
expect(cli.run(%w[-f offenses --only Layout example.rb])).to eq(1)
Expand All @@ -559,7 +559,7 @@ class SomeCop < Cop
context 'when three namespaces are given' do
it 'runs all enabled cops in those namespaces' do
create_file('example.rb', ['if x== 100000000000000 ',
' # ' + '-' * 130,
" # #{'-' * 130}",
"\ty",
'end'])
create_file('.rubocop.yml', <<~YAML)
Expand Down