From 459aff0c3c78d37b8484a2cb93cf024d833c3294 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Wed, 17 Apr 2024 01:37:24 +0900 Subject: [PATCH] [Fix #12842] Add new `Style/SendWithLiteralMethodName` cop Fixes #12842. ## Summary Detects the use of the `public_send` method with a literal method name argument. Since the `send` method can be used to call private methods, by default, only the `public_send` method is detected. ```ruby # bad obj.public_send(:method_name) obj.public_send('method_name') # good obj.method_name ``` ## Safety This cop is not safe because it can incorrectly detect based on the receiver. Additionally, when `AllowSend` is set to `true`, it cannot determine whether the `send` method being detected is calling a private method. ## `AllowSend` option This cop has `AllowSend` option. ### AllowSend: true (default) ```ruby # good obj.send(:method_name) obj.send('method_name') obj.__send__(:method_name) obj.__send__('method_name') ``` ### AllowSend: false ```ruby # bad obj.send(:method_name) obj.send('method_name') obj.__send__(:method_name) obj.__send__('method_name') # good obj.method_name ``` --- ...style_send_with_literal_method_name_cop.md | 1 + config/default.yml | 7 + lib/rubocop.rb | 1 + .../style/send_with_literal_method_name.rb | 83 +++++++++++ .../send_with_literal_method_name_spec.rb | 130 ++++++++++++++++++ 5 files changed, 222 insertions(+) create mode 100644 changelog/new_add_new_style_send_with_literal_method_name_cop.md create mode 100644 lib/rubocop/cop/style/send_with_literal_method_name.rb create mode 100644 spec/rubocop/cop/style/send_with_literal_method_name_spec.rb diff --git a/changelog/new_add_new_style_send_with_literal_method_name_cop.md b/changelog/new_add_new_style_send_with_literal_method_name_cop.md new file mode 100644 index 000000000000..5e54bdb1529f --- /dev/null +++ b/changelog/new_add_new_style_send_with_literal_method_name_cop.md @@ -0,0 +1 @@ +* [#12842](https://github.com/rubocop/rubocop/issues/12842): Add new `Style/SendWithLiteralMethodName` cop. ([@koic][]) diff --git a/config/default.yml b/config/default.yml index e0905e865de8..bcb061af4446 100644 --- a/config/default.yml +++ b/config/default.yml @@ -5240,6 +5240,13 @@ Style/Send: Enabled: false VersionAdded: '0.33' +Style/SendWithLiteralMethodName: + Description: 'Detects the use of the `public_send` method with a static method name argument.' + Enabled: pending + Safe: false + AllowSend: true + VersionAdded: '<>' + Style/SignalException: Description: 'Checks for proper usage of fail and raise.' StyleGuide: '#prefer-raise-over-fail' diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 713569adf382..5c9f67275bd0 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -666,6 +666,7 @@ require_relative 'rubocop/cop/style/self_assignment' require_relative 'rubocop/cop/style/semicolon' require_relative 'rubocop/cop/style/send' +require_relative 'rubocop/cop/style/send_with_literal_method_name' require_relative 'rubocop/cop/style/signal_exception' require_relative 'rubocop/cop/style/single_argument_dig' require_relative 'rubocop/cop/style/single_line_block_params' diff --git a/lib/rubocop/cop/style/send_with_literal_method_name.rb b/lib/rubocop/cop/style/send_with_literal_method_name.rb new file mode 100644 index 000000000000..698962eb2757 --- /dev/null +++ b/lib/rubocop/cop/style/send_with_literal_method_name.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # Detects the use of the `public_send` method with a literal method name argument. + # Since the `send` method can be used to call private methods, by default, + # only the `public_send` method is detected. + # + # @safety + # This cop is not safe because it can incorrectly detect based on the receiver. + # Additionally, when `AllowSend` is set to `true`, it cannot determine whether + # the `send` method being detected is calling a private method. + # + # @example + # # bad + # obj.public_send(:method_name) + # obj.public_send('method_name') + # + # # good + # obj.method_name + # + # @example AllowSend: true (default) + # # good + # obj.send(:method_name) + # obj.send('method_name') + # obj.__send__(:method_name) + # obj.__send__('method_name') + # + # @example AllowSend: false + # # bad + # obj.send(:method_name) + # obj.send('method_name') + # obj.__send__(:method_name) + # obj.__send__('method_name') + # + # # good + # obj.method_name + # + class SendWithLiteralMethodName < Base + extend AutoCorrector + + MSG = 'Use `%s` method call directly instead.' + RESTRICT_ON_SEND = %i[public_send send __send__].freeze + STATIC_METHOD_NAME_NODE_TYPES = %i[sym str].freeze + + # rubocop:disable Metrics/AbcSize + def on_send(node) + return if allow_send? && !node.method?(:public_send) + return unless (first_argument = node.first_argument) + return unless STATIC_METHOD_NAME_NODE_TYPES.include?(first_argument.type) + + offense_range = offense_range(node) + method_name = first_argument.value + + add_offense(offense_range, message: format(MSG, method_name: method_name)) do |corrector| + if node.arguments.one? + corrector.replace(offense_range, method_name) + else + corrector.replace(node.loc.selector, method_name) + corrector.remove(removal_argument_range(first_argument, node.arguments[1])) + end + end + end + # rubocop:enable Metrics/AbcSize + + private + + def allow_send? + !!cop_config['AllowSend'] + end + + def offense_range(node) + node.loc.selector.join(node.source_range.end) + end + + def removal_argument_range(first_argument, second_argument) + first_argument.source_range.begin.join(second_argument.source_range.begin) + end + end + end + end +end diff --git a/spec/rubocop/cop/style/send_with_literal_method_name_spec.rb b/spec/rubocop/cop/style/send_with_literal_method_name_spec.rb new file mode 100644 index 000000000000..1d41d4598294 --- /dev/null +++ b/spec/rubocop/cop/style/send_with_literal_method_name_spec.rb @@ -0,0 +1,130 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::SendWithLiteralMethodName, :config do + it 'registers an offense when using `public_send` with symbol literal argument' do + expect_offense(<<~RUBY) + obj.public_send(:foo) + ^^^^^^^^^^^^^^^^^ Use `foo` method call directly instead. + RUBY + + expect_correction(<<~RUBY) + obj.foo + RUBY + end + + it 'registers an offense when using `public_send` with symbol literal argument and some arguments with parentheses' do + expect_offense(<<~RUBY) + obj.public_send(:foo, bar, 42) + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `foo` method call directly instead. + RUBY + + expect_correction(<<~RUBY) + obj.foo(bar, 42) + RUBY + end + + it 'registers an offense when using `public_send` with symbol literal argument and some arguments without parentheses' do + expect_offense(<<~RUBY) + obj.public_send :foo, bar, 42 + ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `foo` method call directly instead. + RUBY + + expect_correction(<<~RUBY) + obj.foo bar, 42 + RUBY + end + + it 'registers an offense when using `public_send` with symbol literal argument without receiver' do + expect_offense(<<~RUBY) + public_send(:foo) + ^^^^^^^^^^^^^^^^^ Use `foo` method call directly instead. + RUBY + + expect_correction(<<~RUBY) + foo + RUBY + end + + it 'registers an offense when using `public_send` with string literal argument' do + expect_offense(<<~RUBY) + obj.public_send('foo') + ^^^^^^^^^^^^^^^^^^ Use `foo` method call directly instead. + RUBY + + expect_correction(<<~RUBY) + obj.foo + RUBY + end + + it 'does not register an offense when using `public_send` with variable argument' do + expect_no_offenses(<<~RUBY) + obj.public_send(variable) + RUBY + end + + it 'does not register an offense when using `public_send` with interpolated string argument' do + expect_no_offenses(<<~'RUBY') + obj.public_send("#{interpolated}string") + RUBY + end + + it 'does not register an offense when using `public_send` with integer literal argument' do + expect_no_offenses(<<~RUBY) + obj.public_send(42) + RUBY + end + + it 'does not register an offense when using `public_send` with no arguments' do + expect_no_offenses(<<~RUBY) + obj.public_send + RUBY + end + + it 'does not register an offense when using method call without `public_send`' do + expect_no_offenses(<<~RUBY) + obj.foo + RUBY + end + + context 'when `AllowSend: true`' do + let(:cop_config) { { 'AllowSend' => true } } + + it 'does not register an offense when using `send` with symbol literal argumen' do + expect_no_offenses(<<~RUBY) + obj.send(:foo) + RUBY + end + + it 'does not register an offense when using `__send__` with symbol literal argument' do + expect_no_offenses(<<~RUBY) + obj.__send__(:foo) + RUBY + end + end + + context 'when `AllowSend: false`' do + let(:cop_config) { { 'AllowSend' => false } } + + it 'registers an offense when using `send` with symbol literal argument' do + expect_offense(<<~RUBY) + obj.send(:foo) + ^^^^^^^^^^ Use `foo` method call directly instead. + RUBY + + expect_correction(<<~RUBY) + obj.foo + RUBY + end + + it 'registers an offense when using `__send__` with symbol literal argument' do + expect_offense(<<~RUBY) + obj.__send__(:foo) + ^^^^^^^^^^^^^^ Use `foo` method call directly instead. + RUBY + + expect_correction(<<~RUBY) + obj.foo + RUBY + end + end +end