Merge pull request #10966 from r7kamura/feature/modifier-autocorrect

Add autocorrect support to `Style/AccessModifierDeclarations`

changelog/change_add_autocorrect_support_to.md

@@ -0,0 +1 @@
+* [#10966](https://github.com/rubocop/rubocop/pull/10966): Add autocorrect support to `Style/AccessModifierDeclarations`. ([@r7kamura][])

config/default.yml

@@ -2949,6 +2949,7 @@ Style/AccessModifierDeclarations:
     - inline
     - group
   AllowModifiersOnSymbols: true
+  SafeAutoCorrect: false
 
 Style/AccessorGrouping:
   Description: 'Checks for grouping of accessors in `class` and `module` bodies.'

lib/rubocop/cop/style/access_modifier_declarations.rb

@@ -9,6 +9,11 @@ module Style
       # Applications of visibility methods to symbols can be controlled
       # using AllowModifiersOnSymbols config.
       #
+      # @safety
+      #   Autocorrection is not safe, because the visibility of dynamically
+      #   defined methods can vary depending on the state determined by
+      #   the group access modifier.
+      #
       # @example EnforcedStyle: group (default)
       #   # bad
       #   class Foo
@@ -63,7 +68,10 @@ module Style
       #
       #   end
       class AccessModifierDeclarations < Base
+        extend AutoCorrector
+
         include ConfigurableEnforcedStyle
+        include RangeHelp
 
         GROUP_STYLE_MESSAGE = [
           '`%<access_modifier>s` should not be',
@@ -88,14 +96,34 @@ def on_send(node)
           return if allow_modifiers_on_symbols?(node)
 
           if offense?(node)
-            add_offense(node.loc.selector) { opposite_style_detected }
+            add_offense(node.loc.selector) do |corrector|
+              autocorrect(corrector, node)
+            end
+            opposite_style_detected
           else
             correct_style_detected
           end
         end
 
         private
 
+        def autocorrect(corrector, node)
+          case style
+          when :group
+            def_node = find_corresponding_def_node(node)
+            return unless def_node
+
+            remove_node(corrector, def_node)
+            remove_node(corrector, node)
+            insert_def(corrector, node, def_node.source)
+          when :inline
+            remove_node(corrector, node)
+            select_grouped_def_nodes(node).each do |grouped_def_node|
+              insert_inline_modifier(corrector, grouped_def_node, node.method_name)
+            end
+          end
+        end
+
         def allow_modifiers_on_symbols?(node)
           cop_config['AllowModifiersOnSymbols'] && access_modifier_with_symbol?(node)
         end
@@ -130,6 +158,54 @@ def message(range)
             format(INLINE_STYLE_MESSAGE, access_modifier: access_modifier)
           end
         end
+
+        def find_corresponding_def_node(node)
+          if access_modifier_with_symbol?(node)
+            method_name = node.arguments.first.value
+            node.parent.each_child_node(:def).find do |child|
+              child.method?(method_name)
+            end
+          else
+            node.arguments.first
+          end
+        end
+
+        def find_argument_less_modifier_node(node)
+          node.parent.each_child_node(:send).find do |child|
+            child.method?(node.method_name) && child.arguments.empty?
+          end
+        end
+
+        def select_grouped_def_nodes(node)
+          node.right_siblings.take_while do |sibling|
+            !(sibling.send_type? && sibling.bare_access_modifier_declaration?)
+          end.select(&:def_type?)
+        end
+
+        def insert_def(corrector, node, source)
+          argument_less_modifier_node = find_argument_less_modifier_node(node)
+          if argument_less_modifier_node
+            corrector.insert_after(argument_less_modifier_node, "\n\n#{source}")
+          else
+            corrector.insert_before(
+              node.each_ancestor(:block, :class, :module).first.location.end,
+              "#{node.method_name}\n\n#{source}\n"
+            )
+          end
+        end
+
+        def insert_inline_modifier(corrector, node, modifier_name)
+          corrector.insert_before(node, "#{modifier_name} ")
+        end
+
+        def remove_node(corrector, node)
+          corrector.remove(
+            range_by_whole_lines(
+              node.location.expression,
+              include_final_newline: true
+            )
+          )
+        end
       end
     end
   end

spec/rubocop/cop/style/access_modifier_declarations_spec.rb

@@ -35,6 +35,8 @@ class Foo
             ^{access_modifier} `#{access_modifier}` should not be inlined in method definitions.
           end
         RUBY
+
+        expect_no_corrections
       end
     end
   end
@@ -50,6 +52,14 @@ class Test
             ^{access_modifier} `#{access_modifier}` should not be inlined in method definitions.
           end
         RUBY
+
+        expect_correction(<<~RUBY)
+          class Test
+          #{access_modifier}
+
+          def foo; end
+          end
+        RUBY
       end
 
       it "does not offend when #{access_modifier} is not inlined" do
@@ -90,6 +100,106 @@ class TestThree
             ^{access_modifier} `#{access_modifier}` should not be inlined in method definitions.
           end
         RUBY
+
+        expect_correction(<<~RUBY)
+          class TestOne
+            #{access_modifier}
+          end
+
+          class TestTwo
+          #{access_modifier}
+
+          def foo; end
+          end
+
+          class TestThree
+          #{access_modifier}
+
+          def foo; end
+          end
+        RUBY
+      end
+
+      context 'when method is modified by inline modifier' do
+        it 'registers and autocorrects an offense' do
+          expect_offense(<<~RUBY, access_modifier: access_modifier)
+            class Test
+              #{access_modifier} def foo; end
+              ^{access_modifier} `#{access_modifier}` should not be inlined in method definitions.
+            end
+          RUBY
+
+          expect_correction(<<~RUBY)
+            class Test
+            #{access_modifier}
+
+            def foo; end
+            end
+          RUBY
+        end
+      end
+
+      context 'when method is modified by inline modifier with disallowed symbol' do
+        let(:cop_config) do
+          { 'AllowModifiersOnSymbols' => false }
+        end
+
+        it 'registers and autocorrects an offense' do
+          expect_offense(<<~RUBY, access_modifier: access_modifier)
+            class Test
+              def foo; end
+              #{access_modifier} :foo
+              ^{access_modifier} `#{access_modifier}` should not be inlined in method definitions.
+            end
+          RUBY
+
+          expect_correction(<<~RUBY)
+            class Test
+            #{access_modifier}
+
+            def foo; end
+            end
+          RUBY
+        end
+      end
+
+      context 'when non-existent method is modified by inline modifier with disallowed symbol' do
+        let(:cop_config) do
+          { 'AllowModifiersOnSymbols' => false }
+        end
+
+        it 'registers an offense but does not autocorrect it' do
+          expect_offense(<<~RUBY, access_modifier: access_modifier)
+            class Test
+              #{access_modifier} :foo
+              ^{access_modifier} `#{access_modifier}` should not be inlined in method definitions.
+            end
+          RUBY
+
+          expect_no_corrections
+        end
+      end
+
+      context 'when method is modified by inline modifier where group modifier already exists' do
+        it 'registers and autocorrects an offense' do
+          expect_offense(<<~RUBY, access_modifier: access_modifier)
+            class Test
+              #{access_modifier} def foo; end
+              ^{access_modifier} `#{access_modifier}` should not be inlined in method definitions.
+
+              #{access_modifier}
+            end
+          RUBY
+
+          expect_correction(<<~RUBY)
+            class Test
+
+              #{access_modifier}
+
+            def foo; end
+            end
+          RUBY
+        end
       end
 
       include_examples 'always accepted', access_modifier
@@ -107,6 +217,11 @@ class Test
             ^{access_modifier} `#{access_modifier}` should be inlined in method definitions.
           end
         RUBY
+
+        expect_correction(<<~RUBY)
+          class Test
+          end
+        RUBY
       end
 
       it "offends when #{access_modifier} is not inlined and has a comment" do
@@ -116,6 +231,11 @@ class Test
             ^{access_modifier} `#{access_modifier}` should be inlined in method definitions.
           end
         RUBY
+
+        expect_correction(<<~RUBY)
+          class Test
+          end
+        RUBY
       end
 
       it "does not offend when #{access_modifier} is inlined with a method" do
@@ -152,6 +272,42 @@ class TestThree
             ^{access_modifier} `#{access_modifier}` should be inlined in method definitions.
           end
         RUBY
+
+        expect_correction(<<~RUBY)
+          class TestOne
+            #{access_modifier} def foo; end
+          end
+
+          class TestTwo
+          end
+
+          class TestThree
+          end
+        RUBY
+      end
+
+      context 'when methods are modified by group modifier' do
+        it 'registers and autocorrects an offense' do
+          expect_offense(<<~RUBY, access_modifier: access_modifier)
+            class Test
+              #{access_modifier}
+              ^{access_modifier} `#{access_modifier}` should be inlined in method definitions.
+
+              def foo; end
+
+              def bar; end
+            end
+          RUBY
+
+          expect_correction(<<~RUBY)
+            class Test
+
+              #{access_modifier} def foo; end
+
+              #{access_modifier} def bar; end
+            end
+          RUBY
+        end
       end
 
       include_examples 'always accepted', access_modifier