Skip to content

Commit

Permalink
Merge pull request #32971 from y-yagi/use_plus_operator_to_unfreeze_s…
Browse files Browse the repository at this point in the history
…tring

Enable `Performance/UnfreezeString` cop
  • Loading branch information
y-yagi committed Sep 23, 2018
2 parents d3b9521 + 1b86d90 commit 82d1848
Show file tree
Hide file tree
Showing 101 changed files with 366 additions and 364 deletions.
3 changes: 3 additions & 0 deletions .rubocop.yml
Expand Up @@ -208,3 +208,6 @@ Performance/EndWith:

Performance/RegexpMatch:
Enabled: true

Performance/UnfreezeString:
Enabled: true
2 changes: 1 addition & 1 deletion actioncable/lib/action_cable/channel/base.rb
Expand Up @@ -270,7 +270,7 @@ def dispatch_action(action, data)
end

def action_signature(action, data)
"#{self.class.name}##{action}".dup.tap do |signature|
(+"#{self.class.name}##{action}").tap do |signature|
if (arguments = data.except("action")).any?
signature << "(#{arguments.inspect})"
end
Expand Down
4 changes: 2 additions & 2 deletions actionmailer/test/base_test.rb
Expand Up @@ -122,7 +122,7 @@ class BaseTest < ActiveSupport::TestCase
email = BaseMailer.attachment_with_hash
assert_equal(1, email.attachments.length)
assert_equal("invoice.jpg", email.attachments[0].filename)
expected = "\312\213\254\232)b".dup
expected = +"\312\213\254\232)b"
expected.force_encoding(Encoding::BINARY)
assert_equal expected, email.attachments["invoice.jpg"].decoded
end
Expand All @@ -131,7 +131,7 @@ class BaseTest < ActiveSupport::TestCase
email = BaseMailer.attachment_with_hash_default_encoding
assert_equal(1, email.attachments.length)
assert_equal("invoice.jpg", email.attachments[0].filename)
expected = "\312\213\254\232)b".dup
expected = +"\312\213\254\232)b"
expected.force_encoding(Encoding::BINARY)
assert_equal expected, email.attachments["invoice.jpg"].decoded
end
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_controller/log_subscriber.rb
Expand Up @@ -26,7 +26,7 @@ def process_action(event)
exception_class_name = payload[:exception].first
status = ActionDispatch::ExceptionWrapper.status_code_for_exception(exception_class_name)
end
message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in #{event.duration.round}ms".dup
message = +"Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in #{event.duration.round}ms"
message << " (#{additions.join(" | ".freeze)})" unless additions.empty?
message << "\n\n" if defined?(Rails.env) && Rails.env.development?

Expand Down
Expand Up @@ -474,7 +474,7 @@ def params_array_from(raw_params)

# This removes the <tt>"</tt> characters wrapping the value.
def rewrite_param_values(array_params)
array_params.each { |param| (param[1] || "".dup).gsub! %r/^"|"$/, "" }
array_params.each { |param| (param[1] || +"").gsub! %r/^"|"$/, "" }
end

# This method takes an authorization body and splits up the key-value
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_controller/metal/live.rb
Expand Up @@ -297,7 +297,7 @@ def log_error(exception)
return unless logger

logger.fatal do
message = "\n#{exception.class} (#{exception.message}):\n".dup
message = +"\n#{exception.class} (#{exception.message}):\n"
message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code)
message << " " << exception.backtrace.join("\n ")
"#{message}\n\n"
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_controller/metal/rendering.rb
Expand Up @@ -40,7 +40,7 @@ def render(*args) #:nodoc:
def render_to_string(*)
result = super
if result.respond_to?(:each)
string = "".dup
string = +""
result.each { |r| string << r }
string
else
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_dispatch/http/response.rb
Expand Up @@ -105,7 +105,7 @@ def initialize(response, buf)

def body
@str_body ||= begin
buf = "".dup
buf = +""
each { |chunk| buf << chunk }
buf
end
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_dispatch/http/url.rb
Expand Up @@ -157,7 +157,7 @@ def normalize_host(_host, options)
subdomain = options.fetch :subdomain, true
domain = options[:domain]

host = "".dup
host = +""
if subdomain == true
return _host if domain.nil?

Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_dispatch/journey/formatter.rb
Expand Up @@ -50,7 +50,7 @@ def generate(name, options, path_parameters, parameterize = nil)
unmatched_keys = (missing_keys || []) & constraints.keys
missing_keys = (missing_keys || []) - unmatched_keys

message = "No route matches #{Hash[constraints.sort_by { |k, v| k.to_s }].inspect}".dup
message = +"No route matches #{Hash[constraints.sort_by { |k, v| k.to_s }].inspect}"
message << ", missing required keys: #{missing_keys.sort.inspect}" if missing_keys && !missing_keys.empty?
message << ", possible unmatched constraints: #{unmatched_keys.sort.inspect}" if unmatched_keys && !unmatched_keys.empty?

Expand Down
6 changes: 3 additions & 3 deletions actionpack/lib/action_dispatch/journey/router/utils.rb
Expand Up @@ -17,11 +17,11 @@ class Utils # :nodoc:
def self.normalize_path(path)
path ||= ""
encoding = path.encoding
path = "/#{path}".dup
path = +"/#{path}"
path.squeeze!("/".freeze)
path.sub!(%r{/+\Z}, "".freeze)
path.gsub!(/(%[a-f0-9]{2})/) { $1.upcase }
path = "/".dup if path == "".freeze
path = +"/" if path == "".freeze
path.force_encoding(encoding)
path
end
Expand All @@ -32,7 +32,7 @@ class UriEncoder # :nodoc:
ENCODE = "%%%02X".freeze
US_ASCII = Encoding::US_ASCII
UTF_8 = Encoding::UTF_8
EMPTY = "".dup.force_encoding(US_ASCII).freeze
EMPTY = (+"").force_encoding(US_ASCII).freeze
DEC2HEX = (0..255).to_a.map { |i| ENCODE % i }.map { |s| s.force_encoding(US_ASCII) }

ALPHA = "a-zA-Z".freeze
Expand Down
Expand Up @@ -23,7 +23,7 @@ def debug_params(params)
if clean_params.empty?
"None"
else
PP.pp(clean_params, "".dup, 200)
PP.pp(clean_params, +"", 200)
end
end

Expand Down
8 changes: 4 additions & 4 deletions actionpack/lib/action_dispatch/middleware/debug_locks.rb
Expand Up @@ -63,19 +63,19 @@ def render_details(req)

str = threads.map do |thread, info|
if info[:exclusive]
lock_state = "Exclusive".dup
lock_state = +"Exclusive"
elsif info[:sharing] > 0
lock_state = "Sharing".dup
lock_state = +"Sharing"
lock_state << " x#{info[:sharing]}" if info[:sharing] > 1
else
lock_state = "No lock".dup
lock_state = +"No lock"
end

if info[:waiting]
lock_state << " (yielded share)"
end

msg = "Thread #{info[:index]} [0x#{thread.__id__.to_s(16)} #{thread.status || 'dead'}] #{lock_state}\n".dup
msg = +"Thread #{info[:index]} [0x#{thread.__id__.to_s(16)} #{thread.status || 'dead'}] #{lock_state}\n"

if info[:sleeper]
msg << " Waiting in #{info[:sleeper]}"
Expand Down
4 changes: 2 additions & 2 deletions actionpack/lib/action_dispatch/middleware/ssl.rb
Expand Up @@ -102,7 +102,7 @@ def normalize_hsts_options(options)

# https://tools.ietf.org/html/rfc6797#section-6.1
def build_hsts_header(hsts)
value = "max-age=#{hsts[:expires].to_i}".dup
value = +"max-age=#{hsts[:expires].to_i}"
value << "; includeSubDomains" if hsts[:subdomains]
value << "; preload" if hsts[:preload]
value
Expand Down Expand Up @@ -141,7 +141,7 @@ def https_location_for(request)
host = @redirect[:host] || request.host
port = @redirect[:port] || request.port

location = "https://#{host}".dup
location = +"https://#{host}"
location << ":#{port}" if port != 80 && port != 443
location << request.fullpath
location
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_dispatch/routing/mapper.rb
Expand Up @@ -308,7 +308,7 @@ def app(blocks)
def check_controller_and_action(path_params, controller, action)
hash = check_part(:controller, controller, path_params, {}) do |part|
translate_controller(part) {
message = "'#{part}' is not a supported controller name. This can lead to potential routing problems.".dup
message = +"'#{part}' is not a supported controller name. This can lead to potential routing problems."
message << " See https://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use"

raise ArgumentError, message
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_dispatch/routing/route_set.rb
Expand Up @@ -245,7 +245,7 @@ def raise_generation_error(args)
missing_keys << missing_key
}
constraints = Hash[@route.requirements.merge(params).sort_by { |k, v| k.to_s }]
message = "No route matches #{constraints.inspect}".dup
message = +"No route matches #{constraints.inspect}"
message << ", missing required keys: #{missing_keys.sort.inspect}"

raise ActionController::UrlGenerationError, message
Expand Down
Expand Up @@ -65,7 +65,7 @@ def output_type
end

def display_image
message = "[Screenshot]: #{image_path}\n".dup
message = +"[Screenshot]: #{image_path}\n"

case output_type
when "artifact"
Expand Down
5 changes: 2 additions & 3 deletions actionpack/lib/action_dispatch/testing/assertions/response.rb
Expand Up @@ -79,9 +79,8 @@ def normalize_argument_to_redirection(fragment)
end

def generate_response_message(expected, actual = @response.response_code)
"Expected response to be a <#{code_with_name(expected)}>,"\
" but was a <#{code_with_name(actual)}>"
.dup.concat(location_if_redirected).concat(response_body_if_short)
(+"Expected response to be a <#{code_with_name(expected)}>,"\
" but was a <#{code_with_name(actual)}>").concat(location_if_redirected).concat(response_body_if_short)
end

def response_body_if_short
Expand Down
Expand Up @@ -150,7 +150,7 @@ def authenticate_long_credentials
end

test "token_and_options returns empty string with empty token" do
token = "".dup
token = +""
actual = ActionController::HttpAuthentication::Token.token_and_options(sample_request(token)).first
expected = token
assert_equal(expected, actual)
Expand Down
2 changes: 1 addition & 1 deletion actionpack/test/controller/new_base/bare_metal_test.rb
Expand Up @@ -13,7 +13,7 @@ class BareTest < ActiveSupport::TestCase
test "response body is a Rack-compatible response" do
status, headers, body = BareController.action(:index).call(Rack::MockRequest.env_for("/"))
assert_equal 200, status
string = "".dup
string = +""

body.each do |part|
assert part.is_a?(String), "Each part of the body must be a String"
Expand Down
2 changes: 1 addition & 1 deletion actionpack/test/controller/routing_test.rb
Expand Up @@ -674,7 +674,7 @@ def test_route_with_text_default
assert_equal "/page/foo", url_for(rs, controller: "content", action: "show_page", id: "foo")
assert_equal({ controller: "content", action: "show_page", id: "foo" }, rs.recognize_path("/page/foo"))

token = "\321\202\320\265\320\272\321\201\321\202".dup # 'text' in Russian
token = +"\321\202\320\265\320\272\321\201\321\202" # 'text' in Russian
token.force_encoding(Encoding::BINARY)
escaped_token = CGI.escape(token)

Expand Down
2 changes: 1 addition & 1 deletion actionpack/test/dispatch/debug_exceptions_test.rb
Expand Up @@ -368,7 +368,7 @@ def call(env)
})
assert_response 500

assert_includes(body, CGI.escapeHTML(PP.pp(params, "".dup, 200)))
assert_includes(body, CGI.escapeHTML(PP.pp(params, +"", 200)))
end

test "sets the HTTP charset parameter" do
Expand Down
2 changes: 1 addition & 1 deletion actionpack/test/dispatch/prefix_generation_test.rb
Expand Up @@ -13,7 +13,7 @@ def to_param
end

def self.model_name
klass = "Post".dup
klass = +"Post"
def klass.name; self end

ActiveModel::Name.new(klass)
Expand Down
4 changes: 2 additions & 2 deletions actionpack/test/dispatch/static_test.rb
Expand Up @@ -31,15 +31,15 @@ def test_handles_urls_with_bad_encoding
end

def test_handles_urls_with_ascii_8bit
assert_equal "Hello, World!", get("/doorkeeper%E3E4".dup.force_encoding("ASCII-8BIT")).body
assert_equal "Hello, World!", get((+"/doorkeeper%E3E4").force_encoding("ASCII-8BIT")).body
end

def test_handles_urls_with_ascii_8bit_on_win_31j
silence_warnings do
Encoding.default_internal = "Windows-31J"
Encoding.default_external = "Windows-31J"
end
assert_equal "Hello, World!", get("/doorkeeper%E3E4".dup.force_encoding("ASCII-8BIT")).body
assert_equal "Hello, World!", get((+"/doorkeeper%E3E4").force_encoding("ASCII-8BIT")).body
end

def test_handles_urls_with_null_byte
Expand Down
2 changes: 1 addition & 1 deletion actionpack/test/journey/router/utils_test.rb
Expand Up @@ -23,7 +23,7 @@ def test_uri_unescape
end

def test_uri_unescape_with_utf8_string
assert_equal "Šašinková", Utils.unescape_uri("%C5%A0a%C5%A1inkov%C3%A1".dup.force_encoding(Encoding::US_ASCII))
assert_equal "Šašinková", Utils.unescape_uri((+"%C5%A0a%C5%A1inkov%C3%A1").force_encoding(Encoding::US_ASCII))
end

def test_normalize_path_not_greedy
Expand Down
4 changes: 2 additions & 2 deletions actionview/lib/action_view/helpers/date_helper.rb
Expand Up @@ -1053,7 +1053,7 @@ def build_select(type, select_options_as_html)
select_options[:disabled] = "disabled" if @options[:disabled]
select_options[:class] = css_class_attribute(type, select_options[:class], @options[:with_css_classes]) if @options[:with_css_classes]

select_html = "\n".dup
select_html = +"\n"
select_html << content_tag("option".freeze, "", value: "") + "\n" if @options[:include_blank]
select_html << prompt_option_tag(type, @options[:prompt]) + "\n" if @options[:prompt]
select_html << select_options_as_html
Expand Down Expand Up @@ -1135,7 +1135,7 @@ def input_id_from_type(type)
# Given an ordering of datetime components, create the selection HTML
# and join them with their appropriate separators.
def build_selects_from_types(order)
select = "".dup
select = +""
first_visible = order.find { |type| !@options[:"discard_#{type}"] }
order.reverse_each do |type|
separator = separator(type) unless type == first_visible # don't add before first visible field
Expand Down
4 changes: 2 additions & 2 deletions actionview/lib/action_view/helpers/javascript_helper.rb
Expand Up @@ -15,8 +15,8 @@ module JavaScriptHelper
"'" => "\\'"
}

JS_ESCAPE_MAP["\342\200\250".dup.force_encoding(Encoding::UTF_8).encode!] = "&#x2028;"
JS_ESCAPE_MAP["\342\200\251".dup.force_encoding(Encoding::UTF_8).encode!] = "&#x2029;"
JS_ESCAPE_MAP[(+"\342\200\250").force_encoding(Encoding::UTF_8).encode!] = "&#x2028;"
JS_ESCAPE_MAP[(+"\342\200\251").force_encoding(Encoding::UTF_8).encode!] = "&#x2029;"

# Escapes carriage returns and single and double quotes for JavaScript segments.
#
Expand Down
2 changes: 1 addition & 1 deletion actionview/lib/action_view/helpers/tag_helper.rb
Expand Up @@ -58,7 +58,7 @@ def content_tag_string(name, content, options, escape = true)

def tag_options(options, escape = true)
return if options.blank?
output = "".dup
output = +""
sep = " "
options.each_pair do |key, value|
if TAG_PREFIXES.include?(key) && value.is_a?(Hash)
Expand Down
2 changes: 1 addition & 1 deletion actionview/lib/action_view/helpers/translation_helper.rb
Expand Up @@ -98,7 +98,7 @@ def translate(key, options = {})
raise e if raise_error

keys = I18n.normalize_keys(e.locale, e.key, e.options[:scope])
title = "translation missing: #{keys.join('.')}".dup
title = +"translation missing: #{keys.join('.')}"

interpolations = options.except(:default, :scope)
if interpolations.any?
Expand Down
6 changes: 3 additions & 3 deletions actionview/lib/action_view/log_subscriber.rb
Expand Up @@ -16,15 +16,15 @@ def initialize

def render_template(event)
info do
message = " Rendered #{from_rails_root(event.payload[:identifier])}".dup
message = +" Rendered #{from_rails_root(event.payload[:identifier])}"
message << " within #{from_rails_root(event.payload[:layout])}" if event.payload[:layout]
message << " (#{event.duration.round(1)}ms)"
end
end

def render_partial(event)
info do
message = " Rendered #{from_rails_root(event.payload[:identifier])}".dup
message = +" Rendered #{from_rails_root(event.payload[:identifier])}"
message << " within #{from_rails_root(event.payload[:layout])}" if event.payload[:layout]
message << " (#{event.duration.round(1)}ms)"
message << " #{cache_message(event.payload)}" unless event.payload[:cache_hit].nil?
Expand Down Expand Up @@ -85,7 +85,7 @@ def cache_message(payload) # :doc:

def log_rendering_start(payload)
info do
message = " Rendering #{from_rails_root(payload[:identifier])}".dup
message = +" Rendering #{from_rails_root(payload[:identifier])}"
message << " within #{from_rails_root(payload[:layout])}" if payload[:layout]
message
end
Expand Down
Expand Up @@ -33,7 +33,7 @@ def log_error(exception)
logger = ActionView::Base.logger
return unless logger

message = "\n#{exception.class} (#{exception.message}):\n".dup
message = +"\n#{exception.class} (#{exception.message}):\n"
message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code)
message << " " << exception.backtrace.join("\n ")
logger.fatal("#{message}\n\n")
Expand Down
6 changes: 3 additions & 3 deletions actionview/lib/action_view/template.rb
Expand Up @@ -286,7 +286,7 @@ def compile(mod)

# Make sure that the resulting String to be eval'd is in the
# encoding of the code
source = <<-end_src.dup
source = +<<-end_src
def #{method_name}(local_assigns, output_buffer)
_old_virtual_path, @virtual_path = @virtual_path, #{@virtual_path.inspect};_old_output_buffer = @output_buffer;#{locals_code};#{code}
ensure
Expand Down Expand Up @@ -335,12 +335,12 @@ def locals_code
locals = locals.grep(/\A@?(?![A-Z0-9])(?:[[:alnum:]_]|[^\0-\177])+\z/)

# Assign for the same variable is to suppress unused variable warning
locals.each_with_object("".dup) { |key, code| code << "#{key} = local_assigns[:#{key}]; #{key} = #{key};" }
locals.each_with_object(+"") { |key, code| code << "#{key} = local_assigns[:#{key}]; #{key} = #{key};" }
end

def method_name
@method_name ||= begin
m = "_#{identifier_method_name}__#{@identifier.hash}_#{__id__}".dup
m = +"_#{identifier_method_name}__#{@identifier.hash}_#{__id__}"
m.tr!("-".freeze, "_".freeze)
m
end
Expand Down
2 changes: 1 addition & 1 deletion actionview/lib/action_view/template/resolver.rb
Expand Up @@ -16,7 +16,7 @@ class Path
alias_method :partial?, :partial

def self.build(name, prefix, partial)
virtual = "".dup
virtual = +""
virtual << "#{prefix}/" unless prefix.empty?
virtual << (partial ? "_#{name}" : name)
new name, prefix, partial, virtual
Expand Down

0 comments on commit 82d1848

Please sign in to comment.