-
-
Notifications
You must be signed in to change notification settings - Fork 81
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add new
Performance/StringIdentifierArgument
cop
This PR adds new `Performance/StringIdentifierArgument` cop. This cop identifies places where string identifier argument can be replaced by symbol identifier argument. It prevents the redundancy of the internal string-to-symbol conversion. This cop targets methods that take identifier (e.g. method name) argument and the following examples are parts of it. ```ruby # bad send('do_something') attr_accessor 'do_something' instance_variable_get('@ivar') # good send(:do_something) attr_accessor :do_something instance_variable_get(:@ivar) ``` The `send` method is a representative and other similar methods are similar benchmarks. The following is an example benchmark. ```ruby % cat symbol.rb puts `ruby -v` require 'benchmark/ips' def foo end Benchmark.ips do |x| x.report('symbol arg') { send(:foo) } x.report('string arg') { send('foo') } x.compare! end ``` ```console % ruby symbol.rb ruby 3.1.0dev (2021-12-05T10:23:42Z master 19f037e452) [x86_64-darwin19] Warming up -------------------------------------- symbol arg 1.025M i/100ms string arg 590.038k i/100ms Calculating ------------------------------------- symbol arg 10.665M (± 1.5%) i/s - 53.318M in 5.000551s string arg 5.895M (± 1.0%) i/s - 29.502M in 5.004999s Comparison: symbol arg: 10665035.8 i/s string arg: 5895132.3 i/s - 1.81x (± 0.00) slower ``` I learned about this performance difference from the book "Polished Ruby Programming".
- Loading branch information
Showing
5 changed files
with
119 additions
and
0 deletions.
There are no files selected for viewing
1 change: 1 addition & 0 deletions
1
changelog/new_add_new_performance_string_identifier_argument_cop.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
* [#276](https://github.com/rubocop/rubocop-performance/pull/276): Add new `Performance/StringIdentifierArgument` cop. ([@koic][]) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
# frozen_string_literal: true | ||
|
||
module RuboCop | ||
module Cop | ||
module Performance | ||
# This cop identifies places where string identifier argument can be replaced | ||
# by symbol identifier argument. | ||
# It prevents the redundancy of the internal string-to-symbol conversion. | ||
# | ||
# This cop targets methods that take identifier (e.g. method name) argument | ||
# and the following examples are parts of it. | ||
# | ||
# @example | ||
# | ||
# # bad | ||
# send('do_something') | ||
# attr_accessor 'do_something' | ||
# instance_variable_get('@ivar') | ||
# | ||
# # good | ||
# send(:do_something) | ||
# attr_accessor :do_something | ||
# instance_variable_get(:@ivar) | ||
# | ||
class StringIdentifierArgument < Base | ||
extend AutoCorrector | ||
|
||
MSG = 'Use `%<symbol_arg>s` instead of `%<string_arg>s`.' | ||
|
||
RESTRICT_ON_SEND = %i[ | ||
alias_method attr attr_accessor attr_reader attr_writer autoload autoload? | ||
class_variable_defined? const_defined? const_get const_set const_source_location | ||
define_method instance_method method_defined? private_class_method? private_method_defined? | ||
protected_method_defined? public_class_method public_instance_method public_method_defined? | ||
remove_class_variable remove_method undef_method class_variable_get class_variable_set | ||
deprecate_constant module_function private private_constant protected public public_constant | ||
remove_const ruby2_keywords | ||
define_singleton_method instance_variable_defined instance_variable_get instance_variable_set | ||
method public_method public_send remove_instance_variable respond_to? send singleton_method | ||
__send__ | ||
].freeze | ||
|
||
def on_send(node) | ||
return unless (first_argument = node.first_argument) | ||
return unless first_argument.str_type? | ||
return if first_argument.value.include?(' ') | ||
|
||
replacement = first_argument.value.to_sym.inspect | ||
|
||
message = format(MSG, symbol_arg: replacement, string_arg: first_argument.source) | ||
|
||
add_offense(first_argument, message: message) do |corrector| | ||
corrector.replace(first_argument, replacement) | ||
end | ||
end | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
53 changes: 53 additions & 0 deletions
53
spec/rubocop/cop/performance/string_identifier_argument_spec.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
# frozen_string_literal: true | ||
|
||
RSpec.describe RuboCop::Cop::Performance::StringIdentifierArgument, :config do | ||
RuboCop::Cop::Performance::StringIdentifierArgument::RESTRICT_ON_SEND.each do |method| | ||
it "registers an offense when using string argument for `#{method}` method" do | ||
expect_offense(<<~RUBY, method: method) | ||
#{method}('do_something') | ||
_{method} ^^^^^^^^^^^^^^ Use `:do_something` instead of `'do_something'`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
#{method}(:do_something) | ||
RUBY | ||
end | ||
|
||
it "does not register an offense when using symbol argument for `#{method}` method" do | ||
expect_no_offenses(<<~RUBY) | ||
#{method}(:do_something) | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using interpolated string argument' do | ||
expect_no_offenses(<<~'RUBY') | ||
send("do_something_#{var}") | ||
RUBY | ||
end | ||
end | ||
|
||
it 'does not register an offense when no arguments' do | ||
expect_no_offenses(<<~RUBY) | ||
send | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using integer argument' do | ||
expect_no_offenses(<<~RUBY) | ||
send(42) | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using symbol argument for no identifier argument' do | ||
expect_no_offenses(<<~RUBY) | ||
foo('do_something') | ||
RUBY | ||
end | ||
|
||
# e.g. Trunip https://github.com/jnicklas/turnip#calling-steps-from-other-steps | ||
it 'does not register an offense when using string argument includes spaces' do | ||
expect_no_offenses(<<~RUBY) | ||
send(':foo is :bar', foo, bar) | ||
RUBY | ||
end | ||
end |