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 Performance/StringIdentifierArgument cop #276

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
@@ -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 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
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