Skip to content

Commit

Permalink
Add new Performance/StringIdentifierArgument cop
Browse files Browse the repository at this point in the history
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 example is a part 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
koic committed Dec 7, 2021
1 parent bc3a349 commit c922d5d
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 0 deletions.
@@ -0,0 +1 @@
* [#276](https://github.com/rubocop/rubocop-performance/pull/276): Add new `Performance/StringIdentifierArgument` cop. ([@koic][])
5 changes: 5 additions & 0 deletions config/default.yml
Expand Up @@ -311,6 +311,11 @@ Performance/StartWith:
VersionAdded: '0.36'
VersionChanged: '1.10'

Performance/StringIdentifierArgument:
Description: 'Use symbol identifier argument instead of string identifier argument.'
Enabled: pending
VersionAdded: '<<next>>'

Performance/StringInclude:
Description: 'Use `String#include?` instead of a regex match with literal-only pattern.'
Enabled: 'pending'
Expand Down
59 changes: 59 additions & 0 deletions lib/rubocop/cop/performance/string_identifier_argument.rb
@@ -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 example is a part 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
1 change: 1 addition & 0 deletions lib/rubocop/cop/performance_cops.rb
Expand Up @@ -44,6 +44,7 @@
require_relative 'performance/sort_reverse'
require_relative 'performance/squeeze'
require_relative 'performance/start_with'
require_relative 'performance/string_identifier_argument'
require_relative 'performance/string_include'
require_relative 'performance/string_replacement'
require_relative 'performance/sum'
Expand Down
53 changes: 53 additions & 0 deletions spec/rubocop/cop/performance/string_identifier_argument_spec.rb
@@ -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

0 comments on commit c922d5d

Please sign in to comment.