From a228e9c87b23a05ce23b9d8ea74ec8f6d30e736f Mon Sep 17 00:00:00 2001 From: Yauheni Dakuka Date: Sun, 9 Feb 2025 22:25:26 +0400 Subject: [PATCH] [Fix rubocop#912] Enhance `Rails/Delegate` by adding delegation detection for `self.class`, constants, instance variables, and class variables --- ..._instance_variables_and_class_variables.md | 1 + lib/rubocop/cop/rails/delegate.rb | 49 +++++++++-- spec/rubocop/cop/rails/delegate_spec.rb | 87 +++++++++++++++++-- 3 files changed, 120 insertions(+), 17 deletions(-) create mode 100644 changelog/fix_rails_delegate_to_handle_self_class_constants_instance_variables_and_class_variables.md diff --git a/changelog/fix_rails_delegate_to_handle_self_class_constants_instance_variables_and_class_variables.md b/changelog/fix_rails_delegate_to_handle_self_class_constants_instance_variables_and_class_variables.md new file mode 100644 index 0000000000..dde58680d1 --- /dev/null +++ b/changelog/fix_rails_delegate_to_handle_self_class_constants_instance_variables_and_class_variables.md @@ -0,0 +1 @@ +* [#912](https://github.com/rubocop/rubocop-rails/issues/912): Enhance `Rails/Delegate` by adding delegation detection for `self.class`, constants, instance variables, and class variables. ([@ydakuka][]) diff --git a/lib/rubocop/cop/rails/delegate.rb b/lib/rubocop/cop/rails/delegate.rb index a50f5d3e84..46a7563959 100644 --- a/lib/rubocop/cop/rails/delegate.rb +++ b/lib/rubocop/cop/rails/delegate.rb @@ -68,7 +68,7 @@ class Delegate < Base def_node_matcher :delegate?, <<~PATTERN (def _method_name _args - (send {(send nil? _) (self)} _ ...)) + (send {(send nil? _) (self) (send (self) :class) ({cvar gvar ivar} _) (const _ _)} _ ...)) PATTERN def on_def(node) @@ -82,17 +82,39 @@ def on_def(node) def register_offense(node) add_offense(node.loc.keyword) do |corrector| - body = node.body + receiver = determine_register_offense_receiver(node.body.receiver) + delegation = build_delegation(node, receiver) - receiver = body.receiver.self_type? ? 'self' : ":#{body.receiver.method_name}" - - delegation = ["delegate :#{body.method_name}", "to: #{receiver}"] - delegation << ['prefix: true'] if node.method?(prefixed_method_name(node.body)) + corrector.replace(node, delegation) + end + end - corrector.replace(node, delegation.join(', ')) + def determine_register_offense_receiver(receiver) + case receiver.type + when :self + 'self' + when :const + full_name = full_const_name(receiver) + full_name.include?('::') ? ":'#{full_name}'" : ":#{full_name}" + when :cvar, :gvar, :ivar + ":#{receiver.source}" + else + ":#{receiver.method_name}" end end + def build_delegation(node, receiver) + delegation = ["delegate :#{node.body.method_name}", "to: #{receiver}"] + delegation << ['prefix: true'] if node.method?(prefixed_method_name(node.body)) + delegation.join(', ') + end + + def full_const_name(node) + return node.source unless node.namespace + + "#{full_const_name(node.namespace)}::#{node.children.last}" + end + def trivial_delegate?(def_node) delegate?(def_node) && method_name_matches?(def_node.method_name, def_node.body) && @@ -120,7 +142,18 @@ def include_prefix_case? def prefixed_method_name(body) return '' if body.receiver.self_type? - [body.receiver.method_name, body.method_name].join('_').to_sym + [determine_prefixed_method_receiver_name(body.receiver), body.method_name].join('_').to_sym + end + + def determine_prefixed_method_receiver_name(receiver) + case receiver.type + when :cvar, :gvar, :ivar + receiver.source + when :const + full_const_name(receiver) + else + receiver.method_name.to_s + end end def private_or_protected_delegation(node) diff --git a/spec/rubocop/cop/rails/delegate_spec.rb b/spec/rubocop/cop/rails/delegate_spec.rb index 401e19b19f..22b68f696a 100644 --- a/spec/rubocop/cop/rails/delegate_spec.rb +++ b/spec/rubocop/cop/rails/delegate_spec.rb @@ -211,15 +211,6 @@ def new RUBY end - it 'ignores delegation to constant' do - expect_no_offenses(<<~RUBY) - FOO = [] - def size - FOO.size - end - RUBY - end - it 'ignores code with no receiver' do expect_no_offenses(<<~RUBY) def change @@ -249,4 +240,82 @@ def foo end RUBY end + + it 'detects delegation to `self.class`' do + expect_offense(<<~RUBY) + def foo + ^^^ Use `delegate` to define delegations. + self.class.foo + end + RUBY + + expect_correction(<<~RUBY) + delegate :foo, to: :class + RUBY + end + + it 'detects delegation to a constant' do + expect_offense(<<~RUBY) + def foo + ^^^ Use `delegate` to define delegations. + CONST.foo + end + RUBY + + expect_correction(<<~RUBY) + delegate :foo, to: :CONST + RUBY + end + + it 'detects delegation to a namespaced constant' do + expect_offense(<<~RUBY) + def foo + ^^^ Use `delegate` to define delegations. + SomeModule::CONST.foo + end + RUBY + + expect_correction(<<~RUBY) + delegate :foo, to: :'SomeModule::CONST' + RUBY + end + + it 'detects delegation to an instance variable' do + expect_offense(<<~RUBY) + def foo + ^^^ Use `delegate` to define delegations. + @instance_variable.foo + end + RUBY + + expect_correction(<<~RUBY) + delegate :foo, to: :@instance_variable + RUBY + end + + it 'detects delegation to a class variable' do + expect_offense(<<~RUBY) + def foo + ^^^ Use `delegate` to define delegations. + @@class_variable.foo + end + RUBY + + expect_correction(<<~RUBY) + delegate :foo, to: :@@class_variable + RUBY + end + + it 'detects delegation to a global variable' do + expect_offense(<<~RUBY) + def foo + ^^^ Use `delegate` to define delegations. + $global_variable.foo + end + RUBY + + expect_correction(<<~RUBY) + delegate :foo, to: :$global_variable + RUBY + end end