Merge pull request #1438 from ydakuka/912-add-more-delegation-targets-to-rails-delegate

[Fix #912] Enhance `Rails/Delegate` by adding delegation detection for `self.class`, constants, instance variables, and class variables

Author: koic

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, class variables, global variables, and instance variables. ([@ydakuka][])

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)

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