Skip to content

Commit

Permalink
[Fix #425] Fix a false positive for`Performance/StringIdentifierArgum…
Browse files Browse the repository at this point in the history
…ent`
  • Loading branch information
Earlopain committed Jan 3, 2024
1 parent 6e7c4cf commit 749d072
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 16 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#425](https://github.com/rubocop/rubocop-performance/issues/425): Fix a false positive for `Performance/StringIdentifierArgument` when using string interpolation with methods that don't support symbols with `::` inside them. ([@earlopain][])
26 changes: 19 additions & 7 deletions lib/rubocop/cop/performance/string_identifier_argument.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,19 @@ module Performance
# send('do_something')
# attr_accessor 'do_something'
# instance_variable_get('@ivar')
# const_get("string_#{interpolation}")
# respond_to?("string_#{interpolation}")
#
# # good
# send(:do_something)
# attr_accessor :do_something
# instance_variable_get(:@ivar)
# const_get(:"string_#{interpolation}")
# 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")
#
#
class StringIdentifierArgument < Base
extend AutoCorrector
Expand All @@ -34,6 +40,8 @@ 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
Expand All @@ -44,14 +52,14 @@ class StringIdentifierArgument < Base
# And `attr` may not be used because `Style/Attr` registers an offense.
# https://github.com/rubocop/rubocop-performance/issues/278
RESTRICT_ON_SEND = (%i[
class_variable_defined? const_defined? const_get const_set const_source_location
class_variable_defined? const_set
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 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
] + COMMAND_METHODS + INTERPOLATION_IGNORE_METHODS).freeze

def on_send(node)
return if COMMAND_METHODS.include?(node.method_name) && node.receiver
Expand All @@ -75,9 +83,13 @@ def string_arguments(node)
[node.first_argument]
end

arguments.compact.filter do |argument|
argument.str_type? || argument.dstr_type?
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) }
end

def register_offense(argument, argument_value)
Expand Down
29 changes: 20 additions & 9 deletions spec/rubocop/cop/performance/string_identifier_argument_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,26 @@
RUBY
end

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
if RuboCop::Cop::Performance::StringIdentifierArgument::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
end
end
end
Expand Down

0 comments on commit 749d072

Please sign in to comment.