Merge pull request #493 from koic/fix_false_positives_for_performance_string_identifier_argument

[Fix #492] Fix false positives for `Performance/StringIdentifierArgument`

Author: koic

changelog/fix_false_positives_for_performance_string_identifier_argument.md

@@ -0,0 +1 @@
+* [#492](https://github.com/rubocop/rubocop-performance/issues/492): Fix false positives for `Performance/StringIdentifierArgument` when using interpolated string argument. ([@koic][])

lib/rubocop/cop/performance/string_identifier_argument.rb

@@ -16,19 +16,19 @@ module Performance
       #   send('do_something')
       #   attr_accessor 'do_something'
       #   instance_variable_get('@ivar')
-      #   respond_to?("string_#{interpolation}")
       #
       #   # good
       #   send(:do_something)
       #   attr_accessor :do_something
       #   instance_variable_get(:@ivar)
-      #   respond_to?(:"string_#{interpolation}")
       #
       #   # good - these methods don't support namespaced symbols
       #   const_get("#{module_path}::Base")
       #   const_source_location("#{module_path}::Base")
       #   const_defined?("#{module_path}::Base")
       #
+      #   # good - using a symbol when string interpolation is involved causes a performance regression.
+      #   respond_to?("string_#{interpolation}")
       #
       class StringIdentifierArgument < Base
         extend AutoCorrector
@@ -40,8 +40,6 @@ class StringIdentifierArgument < Base
           protected public public_constant module_function
         ].freeze
 
-        INTERPOLATION_IGNORE_METHODS = %i[const_get const_source_location const_defined?].freeze
-
         TWO_ARGUMENTS_METHOD = :alias_method
         MULTIPLE_ARGUMENTS_METHODS = %i[
           attr_accessor attr_reader attr_writer private private_constant
@@ -59,7 +57,7 @@ class StringIdentifierArgument < Base
           deprecate_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__
-        ] + COMMAND_METHODS + INTERPOLATION_IGNORE_METHODS).freeze
+        ] + COMMAND_METHODS).freeze
 
         def on_send(node)
           return if COMMAND_METHODS.include?(node.method_name) && node.receiver
@@ -83,13 +81,7 @@ def string_arguments(node)
                         [node.first_argument]
                       end
 
-          arguments.compact.filter { |argument| string_argument_compatible?(argument, node) }
-        end
-
-        def string_argument_compatible?(argument, node)
-          return true if argument.str_type?
-
-          argument.dstr_type? && INTERPOLATION_IGNORE_METHODS.none? { |method| node.method?(method) }
+          arguments.compact.filter(&:str_type?)
         end
 
         def register_offense(argument, argument_value)

spec/rubocop/cop/performance/string_identifier_argument_spec.rb

@@ -44,26 +44,19 @@
         RUBY
       end
 
-      if described_class::INTERPOLATION_IGNORE_METHODS.include?(method)
-        it 'does not register an offense when using string interpolation for `#{method}` method' do
-          # NOTE: These methods don't support `::` when passing a symbol. const_get('A::B') is valid
-          # but const_get(:'A::B') isn't. Since interpolated arguments may contain any content these
-          # cases are not detected as an offense to prevent false positives.
-          expect_no_offenses(<<~RUBY)
-            #{method}("\#{module_name}class_name")
-          RUBY
-        end
-      else
-        it 'registers an offense when using interpolated string argument' do
-          expect_offense(<<~RUBY, method: method)
-            #{method}("do_something_\#{var}")
-            _{method} ^^^^^^^^^^^^^^^^^^^^^ Use `:"do_something_\#{var}"` instead of `"do_something_\#{var}"`.
-          RUBY
-
-          expect_correction(<<~RUBY)
-            #{method}(:"do_something_\#{var}")
-          RUBY
-        end
+      it 'does not register an offense when using string interpolation for `#{method}` method' do
+        # NOTE: These methods don't support `::` when passing a symbol. const_get('A::B') is valid
+        # but const_get(:'A::B') isn't. Since interpolated arguments may contain any content these
+        # cases are not detected as an offense to prevent false positives.
+        expect_no_offenses(<<~RUBY)
+          #{method}("\#{module_name}class_name")
+        RUBY
+      end
+
+      it 'does not register an offense when using interpolated string argument' do
+        expect_no_offenses(<<~RUBY)
+          #{method}("do_something_\#{var}")
+        RUBY
       end
     end
   end