[Fix #7740] Make Style/AccessModifierDeclarations not flag symbol m…

…ethod-name Closes #7740
rubocop · Mar 22, 2020 · fcfa953 · fcfa953
1 parent 22cc72c
commit fcfa953
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@
 * [#7637](https://github.com/rubocop-hq/rubocop/pull/7637): Add new `Style/TrailingCommaInBlockArgs` cop. ([@pawptart][])
 * [#7809](https://github.com/rubocop-hq/rubocop/pull/7809): Add auto-correction for `Style/EndBlock` cop. ([@tejasbubane][])
 * [#7739](https://github.com/rubocop-hq/rubocop/pull/7739): Add `IgnoreNotImplementedMethods` configuration to `Lint/UnusedMethodArgument`. ([@tejasbubane][])
+* [#7740](https://github.com/rubocop-hq/rubocop/issues/7740): Add `AllowModifiersOnSymbols` configuration to `Style/AccessModifierDeclarations`. ([@tejasbubane][])
 
 ### Bug fixes
 

diff --git a/config/default.yml b/config/default.yml
@@ -2212,10 +2212,12 @@ Style/AccessModifierDeclarations:
   Description: 'Checks style of how access modifiers are used.'
   Enabled: true
   VersionAdded: '0.57'
+  VersionChanged: '0.81'
   EnforcedStyle: group
   SupportedStyles:
     - inline
     - group
+  AllowModifiersOnSymbols: true
 
 Style/Alias:
   Description: 'Use alias instead of alias_method.'

diff --git a/lib/rubocop/ast/node.rb b/lib/rubocop/ast/node.rb
@@ -98,7 +98,7 @@ def complete?
         @mutable_attributes.frozen?
       end
 
-      protected :parent= # rubocop:disable Style/AccessModifierDeclarations
+      protected :parent=
 
       # Override `AST::Node#updated` so that `AST::Processor` does not try to
       # mutate our ASTs. Since we keep references from children to parents and
@@ -313,9 +313,8 @@ def const_name
          (casgn $_ $_        (send (const nil? {:Class :Module}) :new ...))
          (casgn $_ $_ (block (send (const nil? {:Class :Module}) :new ...) ...))}
       PATTERN
-      # rubocop:disable Style/AccessModifierDeclarations
+
       private :defined_module0
-      # rubocop:enable Style/AccessModifierDeclarations
 
       def defined_module
         namespace, name = *defined_module0

diff --git a/lib/rubocop/cop/style/access_modifier_declarations.rb b/lib/rubocop/cop/style/access_modifier_declarations.rb
@@ -5,11 +5,12 @@ module Cop
     module Style
       # Access modifiers should be declared to apply to a group of methods
       # or inline before each method, depending on configuration.
+      # EnforcedStyle config covers only method definitions.
+      # Applications of visibility methods to symbols can be controlled
+      # using AllowModifiersOnSymbols config.
       #
       # @example EnforcedStyle: group (default)
-      #
       #   # bad
-      #
       #   class Foo
       #
       #     private def bar; end
@@ -18,7 +19,6 @@ module Style
       #   end
       #
       #   # good
-      #
       #   class Foo
       #
       #     private
@@ -27,10 +27,9 @@ module Style
       #     def baz; end
       #
       #   end
-      # @example EnforcedStyle: inline
       #
+      # @example EnforcedStyle: inline
       #   # bad
-      #
       #   class Foo
       #
       #     private
@@ -41,13 +40,28 @@ module Style
       #   end
       #
       #   # good
-      #
       #   class Foo
       #
       #     private def bar; end
       #     private def baz; end
       #
       #   end
+      #
+      # @example AllowModifiersOnSymbols: true
+      #   # good
+      #   class Foo
+      #
+      #     private :bar, :baz
+      #
+      #   end
+      #
+      # @example AllowModifiersOnSymbols: false
+      #   # bad
+      #   class Foo
+      #
+      #     private :bar, :baz
+      #
+      #   end
       class AccessModifierDeclarations < Cop
         include ConfigurableEnforcedStyle
 
@@ -61,9 +75,15 @@ class AccessModifierDeclarations < Cop
           'inlined in method definitions.'
         ].join(' ')
 
+        def_node_matcher :access_modifier_with_symbol?, <<~PATTERN
+          (send nil? {:private :protected :public} (sym _))
+        PATTERN
+
         def on_send(node)
           return unless node.access_modifier?
           return if node.parent.pair_type?
+          return if cop_config['AllowModifiersOnSymbols'] &&
+                    access_modifier_with_symbol?(node)
 
           if offense?(node)
             add_offense(node, location: :selector) do

diff --git a/manual/cops_style.md b/manual/cops_style.md
@@ -4,18 +4,20 @@
 
 Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
 --- | --- | --- | --- | ---
-Enabled | Yes | No | 0.57 | -
+Enabled | Yes | No | 0.57 | 0.81
 
 Access modifiers should be declared to apply to a group of methods
 or inline before each method, depending on configuration.
+EnforcedStyle config covers only method definitions.
+Applications of visibility methods to symbols can be controlled
+using AllowModifiersOnSymbols config.
 
 ### Examples
 
 #### EnforcedStyle: group (default)
 
 ```ruby
 # bad
-
 class Foo
 
   private def bar; end
@@ -24,7 +26,6 @@ class Foo
 end
 
 # good
-
 class Foo
 
   private
@@ -38,7 +39,6 @@ end
 
 ```ruby
 # bad
-
 class Foo
 
   private
@@ -49,12 +49,31 @@ class Foo
 end
 
 # good
-
 class Foo
 
   private def bar; end
   private def baz; end
 
+end
+```
+#### AllowModifiersOnSymbols: true
+
+```ruby
+# good
+class Foo
+
+  private :bar, :baz
+
+end
+```
+#### AllowModifiersOnSymbols: false
+
+```ruby
+# bad
+class Foo
+
+  private :bar, :baz
+
 end
 ```
 
@@ -63,6 +82,7 @@ end
 Name | Default value | Configurable values
 --- | --- | ---
 EnforcedStyle | `group` | `inline`, `group`
+AllowModifiersOnSymbols | `true` | Boolean
 
 ## Style/Alias
 

diff --git a/spec/rubocop/cop/style/access_modifier_declarations_spec.rb b/spec/rubocop/cop/style/access_modifier_declarations_spec.rb
@@ -12,6 +12,33 @@ class Foo
         end
       RUBY
     end
+
+    context 'allow access modifiers on symbols' do
+      let(:cop_config) { { 'AllowModifiersOnSymbols' => true } }
+
+      it 'accepts when argument to #{access_modifier} is a symbol' do
+        expect_no_offenses(<<~RUBY)
+          class Foo
+            foo
+            #{access_modifier} :bar
+          end
+        RUBY
+      end
+    end
+
+    context 'do not allow access modifiers on symbols' do
+      let(:cop_config) { { 'AllowModifiersOnSymbols' => false } }
+
+      it 'accepts when argument to #{access_modifier} is a symbol' do
+        expect_offense(<<~RUBY)
+          class Foo
+            foo
+            #{access_modifier} :bar
+            #{'^' * access_modifier.length} `#{access_modifier}` should not be inlined in method definitions.
+          end
+        RUBY
+      end
+    end
   end
 
   context 'when `group` is configured' do
@@ -31,17 +58,6 @@ class Test
         RUBY
       end
 
-      it "offends when #{access_modifier} is inlined with a symbol" do
-        expect_offense(<<~RUBY)
-          class Test
-            #{access_modifier} :foo
-            #{'^' * access_modifier.length} `#{access_modifier}` should not be inlined in method definitions.
-
-            def foo; end
-          end
-        RUBY
-      end
-
       it "does not offend when #{access_modifier} is not inlined" do
         expect_no_offenses(<<~RUBY)
           class Test