From 80943f837cd41bee87611c786559798e295ebdad Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Wed, 15 Mar 2023 16:48:55 +0900 Subject: [PATCH] [Fix #346] Fix a false positive for `Performance/StringIdentifierArgument` Fixes #346. This PR fixes a false positive for `Performance/StringIdentifierArgument` when using a command method with receiver. It makes that cop to allow some command methods that don't normally specify a receiver. --- ..._performance_string_identifier_argument.md | 1 + .../performance/string_identifier_argument.rb | 25 +++++++++++-------- .../string_identifier_argument_spec.rb | 8 ++++++ 3 files changed, 24 insertions(+), 10 deletions(-) create mode 100644 changelog/fix_a_false_positive_for_performance_string_identifier_argument.md diff --git a/changelog/fix_a_false_positive_for_performance_string_identifier_argument.md b/changelog/fix_a_false_positive_for_performance_string_identifier_argument.md new file mode 100644 index 0000000000..0af3ab4978 --- /dev/null +++ b/changelog/fix_a_false_positive_for_performance_string_identifier_argument.md @@ -0,0 +1 @@ +* [#346](https://github.com/rubocop/rubocop-performance/issues/346): Fix a false positive for `Performance/StringIdentifierArgument` when using a command method with receiver. ([@koic][]) diff --git a/lib/rubocop/cop/performance/string_identifier_argument.rb b/lib/rubocop/cop/performance/string_identifier_argument.rb index 00a0bacd9e..3d3abb9640 100644 --- a/lib/rubocop/cop/performance/string_identifier_argument.rb +++ b/lib/rubocop/cop/performance/string_identifier_argument.rb @@ -27,28 +27,33 @@ class StringIdentifierArgument < Base MSG = 'Use `%s` instead of `%s`.' + COMMAND_METHODS = %i[ + alias_method attr_accessor attr_reader attr_writer autoload autoload? private private_constant + protected public public_constant module_function + ].freeze + # NOTE: `attr` method is not included in this list as it can cause false positives in Nokogiri API. # And `attr` may not be used because `Style/Attr` registers an offense. # https://github.com/rubocop/rubocop-performance/issues/278 - RESTRICT_ON_SEND = %i[ - alias_method attr_accessor attr_reader attr_writer autoload autoload? + RESTRICT_ON_SEND = (%i[ 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 + 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).freeze def on_send(node) + return if COMMAND_METHODS.include?(node.method_name) && node.receiver return unless (first_argument = node.first_argument) return unless first_argument.str_type? - return if first_argument.value.include?(' ') || first_argument.value.include?('::') - replacement = first_argument.value.to_sym.inspect + first_argument_value = first_argument.value + return if first_argument_value.include?(' ') || first_argument_value.include?('::') + + replacement = first_argument_value.to_sym.inspect message = format(MSG, symbol_arg: replacement, string_arg: first_argument.source) diff --git a/spec/rubocop/cop/performance/string_identifier_argument_spec.rb b/spec/rubocop/cop/performance/string_identifier_argument_spec.rb index 6fa6bf6e03..fd0f31582c 100644 --- a/spec/rubocop/cop/performance/string_identifier_argument_spec.rb +++ b/spec/rubocop/cop/performance/string_identifier_argument_spec.rb @@ -26,6 +26,14 @@ end end + RuboCop::Cop::Performance::StringIdentifierArgument::COMMAND_METHODS.each do |method| + it "does not register an offense when using string argument for `#{method}` method with receiver" do + expect_no_offenses(<<~RUBY) + obj.#{method}('do_something') + RUBY + end + end + it 'does not register an offense when no arguments' do expect_no_offenses(<<~RUBY) send