diff --git a/changelog/change_add_autocorrect_support_to.md b/changelog/change_add_autocorrect_support_to.md new file mode 100644 index 00000000000..6e611aa2232 --- /dev/null +++ b/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][]) diff --git a/config/default.yml b/config/default.yml index 1f2545657d0..f3337f34158 100644 --- a/config/default.yml +++ b/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.' diff --git a/lib/rubocop/cop/style/access_modifier_declarations.rb b/lib/rubocop/cop/style/access_modifier_declarations.rb index fd99ad8b631..9ec3268a2ec 100644 --- a/lib/rubocop/cop/style/access_modifier_declarations.rb +++ b/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 = [ '`%s` should not be', @@ -88,7 +96,10 @@ 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 @@ -96,6 +107,23 @@ def on_send(node) 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 diff --git a/spec/rubocop/cop/style/access_modifier_declarations_spec.rb b/spec/rubocop/cop/style/access_modifier_declarations_spec.rb index b73b35fa9dc..b0a78ca1158 100644 --- a/spec/rubocop/cop/style/access_modifier_declarations_spec.rb +++ b/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