diff --git a/CHANGELOG.md b/CHANGELOG.md index 57419a50802..02e91f90df9 100644 --- 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 index 24ed9b4c379..2663f926824 100644 --- 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 index a30d5e04f18..7e6b6ddb3e7 100644 --- 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 index 8c82c47b6c4..21853fa9dbc 100644 --- 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 index 789c15e54f0..e0eb39d9a88 100644 --- a/manual/cops_style.md +++ b/manual/cops_style.md @@ -4,10 +4,13 @@ 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 @@ -15,7 +18,6 @@ or inline before each method, depending on configuration. ```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 index 2c5adac5ef7..f874536b89f 100644 --- 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