Make ReadWriteAttribute cop aware of shadowing methods

Author: Drenmi

changelog/change_read_write_attribute_to_be_aware_of_shadowed_methods.md

@@ -0,0 +1 @@
+* [#585](https://github.com/rubocop/rubocop-rails/pull/585): Make `Rails/ReadWriteAttribute` cop aware of shadowing methods. ([@drenmi][])

lib/rubocop/cop/rails/read_write_attribute.rb

@@ -23,6 +23,17 @@ module Rails
       #   # good
       #   x = self[:attr]
       #   self[:attr] = val
+      #
+      # When called from within a method with the same name as the attribute,
+      # `read_attribute` and `write_attribute` must be used to prevent an
+      # infinite loop:
+      #
+      # @example
+      #
+      #   # good
+      #   def foo
+      #     bar || read_attribute(:foo)
+      #   end
       class ReadWriteAttribute < Base
         extend AutoCorrector
 
@@ -38,6 +49,7 @@ class ReadWriteAttribute < Base
 
         def on_send(node)
           return unless read_write_attribute?(node)
+          return if within_shadowing_method?(node)
 
           add_offense(node.loc.selector, message: message(node)) do |corrector|
             case node.method_name
@@ -53,6 +65,15 @@ def on_send(node)
 
         private
 
+        def within_shadowing_method?(node)
+          node.each_ancestor(:def).any? do |enclosing_method|
+            shadowing_method_name = node.first_argument.value.to_s
+            shadowing_method_name << '=' if node.method?(:write_attribute)
+
+            enclosing_method.method_name.to_s == shadowing_method_name
+          end
+        end
+
         def message(node)
           if node.method?(:read_attribute)
             format(MSG, prefer: 'self[:attr]', current: 'read_attribute(:attr)')

spec/rubocop/cop/rails/read_write_attribute_spec.rb

@@ -24,6 +24,29 @@
       RUBY
     end
 
+    it 'does not register an offense when called from a method with the same name' do
+      expect_no_offenses(<<~RUBY)
+        def foo
+          bar || read_attribute(:foo)
+        end
+      RUBY
+    end
+
+    it 'registers an offense when called from a method with a different name' do
+      expect_offense(<<~RUBY)
+        def foo
+          bar || read_attribute(:baz)
+                 ^^^^^^^^^^^^^^ Prefer `self[:attr]` over `read_attribute(:attr)`.
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        def foo
+          bar || self[:baz]
+        end
+      RUBY
+    end
+
     it 'autocorrects without parentheses' do
       expect_offense(<<~RUBY)
         res = read_attribute 'test'
@@ -106,6 +129,29 @@
       RUBY
     end
 
+    it 'does not register an offense when called from a method with the same name' do
+      expect_no_offenses(<<~RUBY)
+        def foo=(value)
+          bar(value) || write_attribute(:foo, "baz")
+        end
+      RUBY
+    end
+
+    it 'registers an offense when called from a method with a different name' do
+      expect_offense(<<~RUBY)
+        def foo=(value)
+          bar(value) || write_attribute(:baz, "baz")
+                        ^^^^^^^^^^^^^^^ Prefer `self[:attr] = val` over `write_attribute(:attr, val)`.
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        def foo=(value)
+          bar(value) || self[:baz] = "baz"
+        end
+      RUBY
+    end
+
     it 'corrects assignment with chained methods' do
       expect_offense(<<~RUBY)
         write_attribute(:attr, 'test_' + postfix)