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

Support autocorrection for Lint/UselessAccessModifier #7917

Merged
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.md
Expand Up @@ -6,6 +6,7 @@

* [#7895](https://github.com/rubocop-hq/rubocop/pull/7895): Include `.simplecov` file by default. ([@robotdana][])
* [#7916](https://github.com/rubocop-hq/rubocop/pull/7916): Support autocorrection for `Lint/AmbiguousRegexpLiteral`. ([@koic][])
* [#7917](https://github.com/rubocop-hq/rubocop/pull/7917): Support autocorrection for `Lint/UselessAccessModifier`. ([@koic][])

### Bug fixes

Expand Down
2 changes: 1 addition & 1 deletion config/default.yml
Expand Up @@ -1810,7 +1810,7 @@ Lint/UselessAccessModifier:
Description: 'Checks for useless access modifiers.'
Enabled: true
VersionAdded: '0.20'
VersionChanged: '0.47'
VersionChanged: '0.83'
ContextCreatingMethods: []
MethodCreatingMethods: []

Expand Down
12 changes: 12 additions & 0 deletions lib/rubocop/cop/lint/useless_access_modifier.rb
Expand Up @@ -125,6 +125,8 @@ module Lint
# delegate :method_a, to: :method_b
# end
class UselessAccessModifier < Cop
include RangeHelp

MSG = 'Useless `%<current>s` access modifier.'

def on_class(node)
Expand All @@ -145,6 +147,16 @@ def on_sclass(node)
check_node(node.children[1]) # singleton class body
end

def autocorrect(node)
lambda do |corrector|
range = range_by_whole_lines(
node.source_range, include_final_newline: true
)

corrector.remove(range)
end
end

private

def_node_matcher :static_method_definition?, <<~PATTERN
Expand Down
2 changes: 1 addition & 1 deletion manual/cops_lint.md
Expand Up @@ -2773,7 +2773,7 @@ URI::DEFAULT_PARSER.make_regexp('http://example.com')

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | No | 0.20 | 0.47
Enabled | Yes | Yes | 0.20 | 0.83

This cop checks for redundant access modifiers, including those with no
code, those which are repeated, and leading `public` modifiers in a
Expand Down
165 changes: 152 additions & 13 deletions spec/rubocop/cop/lint/useless_access_modifier_spec.rb
Expand Up @@ -6,7 +6,7 @@
let(:config) { RuboCop::Config.new }

context 'when an access modifier has no effect' do
it 'registers an offense' do
it 'registers an offense and corrects' do
expect_offense(<<~RUBY)
class SomeClass
def some_method
Expand All @@ -19,11 +19,22 @@ def self.some_method
end
end
RUBY

expect_correction(<<~RUBY)
class SomeClass
def some_method
puts 10
end
def self.some_method
puts 10
end
end
RUBY
end
end

context 'when an access modifier has no methods' do
it 'registers an offense' do
it 'registers an offense and corrects' do
expect_offense(<<~RUBY)
class SomeClass
def some_method
Expand All @@ -33,6 +44,14 @@ def some_method
^^^^^^^^^ Useless `protected` access modifier.
end
RUBY

expect_correction(<<~RUBY)
class SomeClass
def some_method
puts 10
end
end
RUBY
end
end

Expand All @@ -55,7 +74,7 @@ class SomeClass

context 'when an access modifier is followed by a ' \
'class method defined on constant' do
it 'registers an offense' do
it 'registers an offense and corrects' do
expect_offense(<<~RUBY)
class SomeClass
protected
Expand All @@ -64,11 +83,18 @@ def SomeClass.some_method
end
end
RUBY

expect_correction(<<~RUBY)
class SomeClass
def SomeClass.some_method
end
end
RUBY
end
end

context 'when there are consecutive access modifiers' do
it 'registers an offense' do
it 'registers an offense and corrects' do
expect_offense(<<~RUBY)
class SomeClass
private
Expand All @@ -82,6 +108,18 @@ def some_other_method
end
end
RUBY

expect_correction(<<~RUBY)
class SomeClass
private
def some_method
puts 10
end
def some_other_method
puts 10
end
end
RUBY
end
end

Expand All @@ -99,18 +137,23 @@ def some_method
end

context 'when class is empty save modifier' do
it 'registers an offense' do
it 'registers an offense and corrects' do
expect_offense(<<~RUBY)
class SomeClass
private
^^^^^^^ Useless `private` access modifier.
end
RUBY

expect_correction(<<~RUBY)
class SomeClass
end
RUBY
end
end

context 'when multiple class definitions in file but only one has offense' do
it 'registers an offense' do
it 'registers an offense and corrects' do
expect_offense(<<~RUBY)
class SomeClass
private
Expand All @@ -119,6 +162,13 @@ class SomeClass
class SomeOtherClass
end
RUBY

expect_correction(<<~RUBY)
class SomeClass
end
class SomeOtherClass
end
RUBY
end
end

Expand All @@ -137,14 +187,20 @@ class SomeClass
context 'when only a constant or local variable is defined after the ' \
'modifier' do
%w[CONSTANT some_var].each do |binding_name|
it 'registers an offense' do
it 'registers an offense and corrects' do
expect_offense(<<~RUBY)
class SomeClass
private
^^^^^^^ Useless `private` access modifier.
#{binding_name} = 1
end
RUBY

expect_correction(<<~RUBY)
class SomeClass
#{binding_name} = 1
end
RUBY
end
end
end
Expand All @@ -163,7 +219,7 @@ class SomeClass
end

context 'when private_class_method is used without arguments' do
it 'registers an offense' do
it 'registers an offense and corrects' do
expect_offense(<<~RUBY)
class SomeClass
private_class_method
Expand All @@ -174,6 +230,15 @@ def self.some_method
end
end
RUBY

expect_correction(<<~RUBY)
class SomeClass

def self.some_method
puts 10
end
end
RUBY
end
end

Expand Down Expand Up @@ -246,11 +311,34 @@ def another_method
end
end
RUBY

expect_correction(<<~RUBY)
class SomeClass
concerning :FirstThing do
def foo
end
private

def method
end
end

concerning :SecondThing do
def omg
end
private
def method
end
def another_method
end
end
end
RUBY
end
end

context 'when using ActiveSupport behavior when Rails is not eabled' do
it 'reports offenses' do
it 'reports offenses and corrects' do
expect_offense(<<~RUBY)
module SomeModule
extend ActiveSupport::Concern
Expand All @@ -269,6 +357,23 @@ def some_private_instance_method
end
end
RUBY

expect_correction(<<~RUBY)
module SomeModule
extend ActiveSupport::Concern
class_methods do
def some_public_class_method
end
private
def some_private_class_method
end
end
def some_public_instance_method
end
def some_private_instance_method
end
end
RUBY
end
end

Expand Down Expand Up @@ -330,11 +435,18 @@ class SomeClass
^^^^^^^ Useless `private` access modifier.
end
RUBY

expect_correction(<<~RUBY)
class SomeClass
delegate :foo, to: :bar

end
RUBY
end
end

shared_examples 'at the top of the body' do |keyword|
it 'registers an offense for `public`' do
it 'registers an offense and corrects for `public`' do
expect_offense(<<~RUBY)
#{keyword} A
public
Expand All @@ -343,6 +455,13 @@ def method
end
end
RUBY

expect_correction(<<~RUBY)
#{keyword} A
def method
end
end
RUBY
end

it "doesn't register an offense for `protected`" do
Expand Down Expand Up @@ -385,7 +504,7 @@ def method2
end

shared_examples 'non-repeated visibility modifiers' do |keyword|
it 'registers an offense even when `public` is not repeated' do
it 'registers an offense and corrects even when `public` is not repeated' do
expect_offense(<<~RUBY)
#{keyword} A
def method1
Expand All @@ -396,6 +515,15 @@ def method2
end
end
RUBY

expect_correction(<<~RUBY)
#{keyword} A
def method1
end
def method2
end
end
RUBY
end

it "doesn't register an offense when `protected` is not repeated" do
Expand Down Expand Up @@ -491,8 +619,8 @@ def foo
end

shared_examples 'unused visibility modifiers' do |keyword|
it 'registers an error when visibility is immediately changed ' \
'without any intervening defs' do
it 'registers an offense and corrects when visibility is ' \
'immediately changed without any intervening defs' do
expect_offense(<<~RUBY)
#{keyword} A
private
Expand All @@ -505,6 +633,17 @@ def method2
end
end
RUBY

expect_correction(<<~RUBY)
#{keyword} A
private
def method1
end
private
def method2
end
end
RUBY
end
end

Expand Down