Skip to content

Commit

Permalink
[Fix #7331] Add 'forbidden' option to Style/ModuleFunction cop (#7451)
Browse files Browse the repository at this point in the history
Co-authored-by: Bozhidar Batsov <bozhidar@batsov.com>
  • Loading branch information
weh and bbatsov committed Mar 21, 2020
1 parent 0aa2e59 commit 13a2452
Show file tree
Hide file tree
Showing 5 changed files with 150 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -72,6 +72,7 @@
* [#7641](https://github.com/rubocop-hq/rubocop/issues/7641): **(Breaking)** Remove `Style/BracesAroundHashParameters` cop. ([@pocke][])
* Add the method name to highlight area of `Layout/EmptyLineBetweenDefs` to help provide more context. ([@rrosenblum][])
* [#7652](https://github.com/rubocop-hq/rubocop/pull/7652): Allow `pp` to allowed names of `Naming/MethodParameterName` cop in default config. ([@masarakki][])
* [#7331](https://github.com/rubocop-hq/rubocop/issues/7331): Add `forbidden` option to `Style/ModuleFunction` cop. ([@weh][])
* [#7309](https://github.com/rubocop-hq/rubocop/pull/7309): Mark `Lint/UselessSetterCall` an "not safe" and improve documentation. ([@jonas054][])
* [#7723](https://github.com/rubocop-hq/rubocop/pull/7723): Enable `Migration/DepartmentName` cop by default. ([@koic][])

Expand Down
1 change: 1 addition & 0 deletions config/default.yml
Expand Up @@ -3090,6 +3090,7 @@ Style/ModuleFunction:
SupportedStyles:
- module_function
- extend_self
- forbidden
Autocorrect: false
SafeAutoCorrect: false

Expand Down
66 changes: 56 additions & 10 deletions lib/rubocop/cop/style/module_function.rb
Expand Up @@ -6,7 +6,7 @@ module Style
# This cop checks for use of `extend self` or `module_function` in a
# module.
#
# Supported styles are: module_function, extend_self.
# Supported styles are: module_function, extend_self, forbidden.
#
# @example EnforcedStyle: module_function (default)
# # bad
Expand Down Expand Up @@ -46,6 +46,29 @@ module Style
# # ...
# end
#
# The option `forbidden` prohibits the usage of both styles.
#
# @example EnforcedStyle: forbidden
# # bad
# module Test
# module_function
# # ...
# end
#
# # bad
# module Test
# extend self
# # ...
# end
#
# # bad
# module Test
# extend self
# # ...
# private
# # ...
# end
#
# These offenses are not safe to auto-correct since there are different
# implications to each approach.
class ModuleFunction < Cop
Expand All @@ -55,6 +78,8 @@ class ModuleFunction < Cop
'Use `module_function` instead of `extend self`.'
EXTEND_SELF_MSG =
'Use `extend self` instead of `module_function`.'
FORBIDDEN_MSG =
'Do not use `module_function` or `extend self`.'

def_node_matcher :module_function_node?, '(send nil? :module_function)'
def_node_matcher :extend_self_node?, '(send nil? :extend self)'
Expand All @@ -69,6 +94,8 @@ def on_module(node)
end

def autocorrect(node)
return if style == :forbidden

lambda do |corrector|
if extend_self_node?(node)
corrector.replace(node.source_range, 'module_function')
Expand All @@ -80,22 +107,41 @@ def autocorrect(node)

private

def each_wrong_style(nodes)
def each_wrong_style(nodes, &block)
case style
when :module_function
private_directive = nodes.any? { |node| private_directive?(node) }

nodes.each do |node|
yield node if extend_self_node?(node) && !private_directive
end
check_module_function(nodes, &block)
when :extend_self
nodes.each do |node|
yield node if module_function_node?(node)
end
check_extend_self(nodes, &block)
when :forbidden
check_forbidden(nodes, &block)
end
end

def check_module_function(nodes)
private_directive = nodes.any? { |node| private_directive?(node) }

nodes.each do |node|
yield node if extend_self_node?(node) && !private_directive
end
end

def check_extend_self(nodes)
nodes.each do |node|
yield node if module_function_node?(node)
end
end

def check_forbidden(nodes)
nodes.each do |node|
yield node if extend_self_node?(node)
yield node if module_function_node?(node)
end
end

def message(_node)
return FORBIDDEN_MSG if style == :forbidden

style == :module_function ? MODULE_FUNCTION_MSG : EXTEND_SELF_MSG
end
end
Expand Down
29 changes: 27 additions & 2 deletions manual/cops_style.md
Expand Up @@ -3734,11 +3734,13 @@ Enabled | Yes | Yes (Unsafe) | 0.11 | 0.65
This cop checks for use of `extend self` or `module_function` in a
module.
Supported styles are: module_function, extend_self.
Supported styles are: module_function, extend_self, forbidden.
In case there are private methods, the cop won't be activated.
Otherwise, it forces to change the flow of the default code.

The option `forbidden` prohibits the usage of both styles.

These offenses are not safe to auto-correct since there are different
implications to each approach.

Expand Down Expand Up @@ -3785,12 +3787,35 @@ module Test
# ...
end
```
#### EnforcedStyle: forbidden

```ruby
# bad
module Test
module_function
# ...
end
# bad
module Test
extend self
# ...
end
# bad
module Test
extend self
# ...
private
# ...
end
```

### Configurable attributes

Name | Default value | Configurable values
--- | --- | ---
EnforcedStyle | `module_function` | `module_function`, `extend_self`
EnforcedStyle | `module_function` | `module_function`, `extend_self`, `forbidden`
Autocorrect | `false` | Boolean

### References
Expand Down
65 changes: 65 additions & 0 deletions spec/rubocop/cop/style/module_function_spec.rb
Expand Up @@ -82,4 +82,69 @@ def test; end
RUBY
end
end

context 'when enforced style is `forbidden`' do
let(:cop_config) { { 'EnforcedStyle' => 'forbidden' } }

context 'registers an offense for `extend self`' do
it 'in a module' do
expect_offense(<<~RUBY)
module Test
extend self
^^^^^^^^^^^ Do not use `module_function` or `extend self`.
def test; end
end
RUBY

expect_no_corrections
end

it 'in a module with private methods' do
expect_offense(<<~RUBY)
module Test
extend self
^^^^^^^^^^^ Do not use `module_function` or `extend self`.
def test; end
private
def test_private;end
end
RUBY

expect_no_corrections
end

it 'in a module with declarative private' do
expect_offense(<<~RUBY)
module Test
extend self
^^^^^^^^^^^ Do not use `module_function` or `extend self`.
def test; end
private :test
end
RUBY

expect_no_corrections
end
end

it 'accepts `extend self` in a class' do
expect_no_offenses(<<~RUBY)
class Test
extend self
end
RUBY
end

it 'registers an offense for `module_function` without an argument' do
expect_offense(<<~RUBY)
module Test
module_function
^^^^^^^^^^^^^^^ Do not use `module_function` or `extend self`.
def test; end
end
RUBY

expect_no_corrections
end
end
end

0 comments on commit 13a2452

Please sign in to comment.