Merge pull request #619 from nvasilevski/fix-read-write-attribute-cop

[Fix #623] Skip method shadowing check if attr is not sym or str for ReadWriteAttribute

Author: koic

changelog/fix_skip_method_shadowing_check_if_attr_is_non_str_or_sym.md

@@ -0,0 +1 @@
+* [#623](https://github.com/rubocop/rubocop-rails/issues/623): Fix method shadowing check for `Rails/ReadWriteAttribute` cop. ([@nvasilevski][])

lib/rubocop/cop/rails/read_write_attribute.rb

@@ -59,12 +59,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)
+          first_arg = node.first_argument
+          return false unless first_arg.respond_to?(:value)
 
-            enclosing_method.method_name.to_s == shadowing_method_name
-          end
+          enclosing_method = node.each_ancestor(:def).first
+          return false unless enclosing_method
+
+          shadowing_method_name = first_arg.value.to_s
+          shadowing_method_name << '=' if node.method?(:write_attribute)
+          enclosing_method.method?(shadowing_method_name)
         end
 
         def build_message(node)

spec/rubocop/cop/rails/read_write_attribute_spec.rb

@@ -89,6 +89,42 @@ def foo
     it 'registers no offense with explicit receiver' do
       expect_no_offenses('res = object.read_attribute(:test)')
     end
+
+    context 'when used within a method' do
+      context 'when using variable for the attribute name' do
+        it 'registers an offense and corrects' do
+          expect_offense(<<~RUBY)
+            def do_the_read_from(column)
+              read_attribute(column)
+              ^^^^^^^^^^^^^^^^^^^^^^ Prefer `self[column]`.
+            end
+          RUBY
+
+          expect_correction(<<~RUBY)
+            def do_the_read_from(column)
+              self[column]
+            end
+          RUBY
+        end
+      end
+
+      context 'when using constant for the attribute name' do
+        it 'registers an offense and corrects' do
+          expect_offense(<<~RUBY)
+            def do_the_read
+              read_attribute(ATTR_NAME)
+              ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `self[ATTR_NAME]`.
+            end
+          RUBY
+
+          expect_correction(<<~RUBY)
+            def do_the_read
+              self[ATTR_NAME]
+            end
+          RUBY
+        end
+      end
+    end
   end
 
   context 'write_attribute' do
@@ -105,6 +141,42 @@ def foo
       end
     end
 
+    context 'when used within a method' do
+      context 'when using variable for the attribute name' do
+        it 'registers an offense and corrects' do
+          expect_offense(<<~RUBY)
+            def do_the_write_to(column)
+              write_attribute(column, 'value')
+              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `self[column] = 'value'`.
+            end
+          RUBY
+
+          expect_correction(<<~RUBY)
+            def do_the_write_to(column)
+              self[column] = 'value'
+            end
+          RUBY
+        end
+      end
+
+      context 'when using constant for the attribute name' do
+        it 'registers an offense and corrects' do
+          expect_offense(<<~RUBY)
+            def do_the_write(value)
+              write_attribute(ATTR_NAME, value)
+              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `self[ATTR_NAME] = value`.
+            end
+          RUBY
+
+          expect_correction(<<~RUBY)
+            def do_the_write(value)
+              self[ATTR_NAME] = value
+            end
+          RUBY
+        end
+      end
+    end
+
     context 'when using a string for the attribute' do
       it 'registers an offense and corrects' do
         expect_offense(<<~RUBY)