Merge pull request #276 from koic/new_add_new_performance_string_iden…

…tifier_argument_cop

Add new `Performance/StringIdentifierArgument` cop

changelog/new_add_new_performance_string_identifier_argument_cop.md

@@ -0,0 +1 @@
+* [#276](https://github.com/rubocop/rubocop-performance/pull/276): Add new `Performance/StringIdentifierArgument` cop. ([@koic][])

config/default.yml

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

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

lib/rubocop/cop/performance_cops.rb

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

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