Skip to content

Commit

Permalink
Fix bug extend_self with private methods
Browse files Browse the repository at this point in the history
This PR fixes rubocop#6370. Cop extend self to turn into module_function should
not be invoked in case there are private methods. Because of that, the
flow of the code gets broken.

This is a very first draft PR. Once the code is good, I will proceed for
the examples + documentation.

There is one test scenario that I don't know how to make it. For
declarative private methods like:
```
def test
 'test'
end

private :test
```

the output for the node is:
```
s(:send, nil, :private,
    s(:sym, :test)))
```
Is there a way to do a `def_node_matcher` saying from the 3rd position
(after private) I don't care about all the arguments that are there?

Code reviews for a better code are more than welcome, since I am not an
expert with the nodes system (maybe there's a better way to do my
purpose)

Looking forward to hearing from you, and hope we can manage the
sceneario of declarative private methods

best!
  • Loading branch information
Ruben Suet authored and bbatsov committed Dec 29, 2018
1 parent 3a8a552 commit 351bc70
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
* [#6598](https://github.com/rubocop-hq/rubocop/pull/6598): Fix a false positive for `Style/MethodCallWithArgsParentheses` `omit_parentheses` enforced style for argument calls with braced blocks. ([@gsamokovarov][])
* [#6594](https://github.com/rubocop-hq/rubocop/pull/6594): Fix a false positive for `Rails/OutputSafety` when the receiver is a non-interpolated string literal. ([@amatsuda][])
* [#6574](https://github.com/rubocop-hq/rubocop/pull/6574): Fix `Style/AccessModifierIndentation` not handling arbitrary blocks. ([@deivid-rodriguez][])
[#6370](https://github.com/rubocop-hq/rubocop/issues/6370): Fix the enforcing style from `extend self` into `module_function` when there are private methods

### Changes

Expand Down Expand Up @@ -3720,3 +3721,4 @@
[@nadiyaka]: https://github.com/nadiyaka
[@amatsuda]: https://github.com/amatsuda
[@Intrepidd]: https://github.com/Intrepidd
[@ruffeng]: https://github.com/Ruffeng
17 changes: 16 additions & 1 deletion lib/rubocop/cop/style/module_function.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ module Style
# # ...
# end
#
# In case there are private methods, the cop won't be activated.
# Otherwise, it forces to change the flow of the default code.
#
# @example EnforcedStyle: module_function (default)
# # good
# module Test
# extend self
# # ...
# private
# # ...
# end
#
# @example EnforcedStyle: extend_self
# # bad
# module Test
Expand All @@ -46,6 +58,7 @@ class ModuleFunction < Cop

def_node_matcher :module_function_node?, '(send nil? :module_function)'
def_node_matcher :extend_self_node?, '(send nil? :extend self)'
def_node_matcher :private_directive?, '(send nil? :private ...)'

def on_module(node)
_name, body = *node
Expand All @@ -71,8 +84,10 @@ def autocorrect(node)
def each_wrong_style(nodes)
case style
when :module_function
private_directive = nodes.any? { |node| private_directive?(node) }

nodes.each do |node|
yield node if extend_self_node?(node)
yield node if extend_self_node?(node) && !private_directive
end
when :extend_self
nodes.each do |node|
Expand Down
14 changes: 14 additions & 0 deletions manual/cops_style.md
Original file line number Diff line number Diff line change
Expand Up @@ -3370,6 +3370,9 @@ module.
Supported styles are: module_function, extend_self.
In case there are private methods, the cop won't be activated.
Otherwise, it forces to change the flow of the default code.

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

Expand All @@ -3390,6 +3393,17 @@ module Test
# ...
end
```
#### EnforcedStyle: module_function (default)

```ruby
# good
module Test
extend self
# ...
private
# ...
end
```
#### EnforcedStyle: extend_self

```ruby
Expand Down
21 changes: 21 additions & 0 deletions spec/rubocop/cop/style/module_function_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,27 @@ def test; end
RUBY
end

it 'accepts for `extend self` in a module with private methods' do
expect_no_offenses(<<-RUBY.strip_indent)
module Test
extend self
def test; end
private
def test_private;end
end
RUBY
end

it 'accepts for `extend self` in a module with declarative private' do
expect_no_offenses(<<-RUBY.strip_indent)
module Test
extend self
def test; end
private :test
end
RUBY
end

it 'accepts `extend self` in a class' do
expect_no_offenses(<<-RUBY.strip_indent)
class Test
Expand Down

0 comments on commit 351bc70

Please sign in to comment.