From 3c080498659df5c4ab2ff740023f4523f7972a2c Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Mon, 27 Apr 2020 19:00:27 +0900 Subject: [PATCH] Support autocorrection for `Lint/UselessAccessModifier` This PR supports autocorrection for `Lint/UselessAccessModifier`. --- CHANGELOG.md | 1 + config/default.yml | 2 +- .../cop/lint/useless_access_modifier.rb | 12 ++ manual/cops_lint.md | 2 +- .../cop/lint/useless_access_modifier_spec.rb | 165 ++++++++++++++++-- 5 files changed, 167 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 127872e3db5..471378dcf8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/config/default.yml b/config/default.yml index 25b554d530d..9c25d5ee832 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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: [] diff --git a/lib/rubocop/cop/lint/useless_access_modifier.rb b/lib/rubocop/cop/lint/useless_access_modifier.rb index 115c5bfcdc1..877023b7fc8 100644 --- a/lib/rubocop/cop/lint/useless_access_modifier.rb +++ b/lib/rubocop/cop/lint/useless_access_modifier.rb @@ -125,6 +125,8 @@ module Lint # delegate :method_a, to: :method_b # end class UselessAccessModifier < Cop + include RangeHelp + MSG = 'Useless `%s` access modifier.' def on_class(node) @@ -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 diff --git a/manual/cops_lint.md b/manual/cops_lint.md index b02a75eea71..f094467ebd0 100644 --- a/manual/cops_lint.md +++ b/manual/cops_lint.md @@ -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 diff --git a/spec/rubocop/cop/lint/useless_access_modifier_spec.rb b/spec/rubocop/cop/lint/useless_access_modifier_spec.rb index b87c24aeb6c..72834ff6d71 100644 --- a/spec/rubocop/cop/lint/useless_access_modifier_spec.rb +++ b/spec/rubocop/cop/lint/useless_access_modifier_spec.rb @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -119,6 +162,13 @@ class SomeClass class SomeOtherClass end RUBY + + expect_correction(<<~RUBY) + class SomeClass + end + class SomeOtherClass + end + RUBY end end @@ -137,7 +187,7 @@ 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 @@ -145,6 +195,12 @@ class SomeClass #{binding_name} = 1 end RUBY + + expect_correction(<<~RUBY) + class SomeClass + #{binding_name} = 1 + end + RUBY end end end @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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