Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add autocorrect support to Style/AccessModifierDeclarations #10966

Merged
merged 1 commit into from Aug 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions 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][])
1 change: 1 addition & 0 deletions config/default.yml
Expand Up @@ -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.'
Expand Down
78 changes: 77 additions & 1 deletion lib/rubocop/cop/style/access_modifier_declarations.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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',
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
156 changes: 156 additions & 0 deletions spec/rubocop/cop/style/access_modifier_declarations_spec.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down