Fix false positives for Style/BisectedAttrAccessor when accessors h…

…ave different access modifiers

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Bug fixes
 
 * [#8252](https://github.com/rubocop-hq/rubocop/issues/8252): Fix a command line option name from `--safe-autocorrect` to `--safe-auto-correct`, which is compatible with RuboCop 0.86 and lower. ([@koic][])
+* [#8259](https://github.com/rubocop-hq/rubocop/issues/8259): Fix false positives for `Style/BisectedAttrAccessor` when accessors have different access modifiers. ([@fatkodima][])
 * [#8253](https://github.com/rubocop-hq/rubocop/issues/8253): Fix false positives for `Style/AccessorGrouping` when accessors have different access modifiers. ([@fatkodima][])
 * [#8257](https://github.com/rubocop-hq/rubocop/issues/8257): Fix an error for `Style/BisectedAttrAccessor` when using `attr_reader` and `attr_writer` with splat arguments. ([@fatkodima][])
 * [#8239](https://github.com/rubocop-hq/rubocop/pull/8239): Don't load `.rubocop.yml` from personal folders to check for exclusions if given a custom configuration file. ([@deivid-rodriguez][])

lib/rubocop/cop/style/bisected_attr_accessor.rb

@@ -21,15 +21,20 @@ module Style
       #   end
       #
       class BisectedAttrAccessor < Cop
+        include VisibilityHelp
+
         MSG = 'Combine both accessors into `attr_accessor %<name>s`.'
 
         def on_class(class_node)
-          reader_names, writer_names = accessor_names(class_node)
+          VISIBILITY_SCOPES.each do |visibility|
+            reader_names, writer_names = accessor_names(class_node, visibility)
 
-          accessor_macroses(class_node).each do |macro|
-            check(macro, reader_names, writer_names)
+            accessor_macroses(class_node, visibility).each do |macro|
+              check(macro, reader_names, writer_names)
+            end
           end
         end
+        alias on_sclass on_class
         alias on_module on_class
 
         def autocorrect(node)
@@ -42,11 +47,11 @@ def autocorrect(node)
 
         private
 
-        def accessor_names(class_node)
+        def accessor_names(class_node, visibility)
           reader_names = Set.new
           writer_names = Set.new
 
-          accessor_macroses(class_node).each do |macro|
+          accessor_macroses(class_node, visibility).each do |macro|
             names = macro.arguments.map(&:source)
 
             names.each do |name|
@@ -61,7 +66,7 @@ def accessor_names(class_node)
           [reader_names, writer_names]
         end
 
-        def accessor_macroses(class_node)
+        def accessor_macroses(class_node, visibility)
           class_def = class_node.body
           return [] if !class_def || class_def.def_type?
 
@@ -72,7 +77,13 @@ def accessor_macroses(class_node)
               class_def.each_child_node(:send)
             end
 
-          send_nodes.select { |node| node.macro? && (attr_reader?(node) || attr_writer?(node)) }
+          send_nodes.select { |node| attr_within_visibility_scope?(node, visibility) }
+        end
+
+        def attr_within_visibility_scope?(node, visibility)
+          node.macro? &&
+            (attr_reader?(node) || attr_writer?(node)) &&
+            node_visibility(node) == visibility
         end
 
         def attr_reader?(send_node)
@@ -95,8 +106,8 @@ def check(macro, reader_names, writer_names)
         end
 
         def replacement(macro, node)
-          class_node = macro.each_ancestor(:class, :module).first
-          reader_names, writer_names = accessor_names(class_node)
+          class_node = macro.each_ancestor(:class, :sclass, :module).first
+          reader_names, writer_names = accessor_names(class_node, node_visibility(macro))
 
           rest_args = rest_args(macro.arguments, reader_names, writer_names)
 

spec/rubocop/cop/style/bisected_attr_accessor_spec.rb

@@ -86,6 +86,70 @@ class Foo
     RUBY
   end
 
+  it 'registers an offense and corrects when both accessors are in the same visibility scope' do
+    expect_offense(<<~RUBY)
+      class Foo
+        attr_reader :bar
+                    ^^^^ Combine both accessors into `attr_accessor :bar`.
+        attr_writer :bar
+                    ^^^^ Combine both accessors into `attr_accessor :bar`.
+
+        private
+
+        attr_writer :baz
+                    ^^^^ Combine both accessors into `attr_accessor :baz`.
+        attr_reader :baz
+                    ^^^^ Combine both accessors into `attr_accessor :baz`.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      class Foo
+        attr_accessor :bar
+        
+
+        private
+
+        
+        attr_accessor :baz
+      end
+    RUBY
+  end
+
+  it 'registers an offense and corrects when withing eigenclass' do
+    expect_offense(<<~RUBY)
+      class Foo
+        attr_reader :bar
+
+        class << self
+          attr_reader :baz
+                      ^^^^ Combine both accessors into `attr_accessor :baz`.
+          attr_writer :baz
+                      ^^^^ Combine both accessors into `attr_accessor :baz`.
+
+          private
+
+          attr_reader :quux
+        end
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      class Foo
+        attr_reader :bar
+
+        class << self
+          attr_accessor :baz
+          
+
+          private
+
+          attr_reader :quux
+        end
+      end
+    RUBY
+  end
+
   it 'does not register an offense when only one accessor of the name exists' do
     expect_no_offenses(<<~RUBY)
       class Foo
@@ -95,6 +159,17 @@ class Foo
     RUBY
   end
 
+  it 'does not register an offense when accessors are withing different visibility scopes' do
+    expect_no_offenses(<<~RUBY)
+      class Foo
+        attr_reader :bar
+
+        private
+        attr_writer :baz
+      end
+    RUBY
+  end
+
   it 'does not register an offense when using `attr_accessor`' do
     expect_no_offenses(<<~RUBY)
       class Foo