Skip to content

Commit

Permalink
[Fix #7739] Allow methods raising NotImplementedError in `Lint/Unused…
Browse files Browse the repository at this point in the history
…MethodArgument`

Closes #7739
  • Loading branch information
tejasbubane authored and bbatsov committed Mar 22, 2020
1 parent 4393c90 commit 27d35b3
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -14,6 +14,7 @@
* [#7331](https://github.com/rubocop-hq/rubocop/issues/7331): Add `forbidden` option to `Style/ModuleFunction` cop. ([@weh][])
* [#7699](https://github.com/rubocop-hq/rubocop/pull/7699): Add new `Lint/StructNewOverride` cop. ([@ybiquitous][])
* [#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][])

### Bug fixes

Expand Down
3 changes: 2 additions & 1 deletion config/default.yml
Expand Up @@ -1768,9 +1768,10 @@ Lint/UnusedMethodArgument:
StyleGuide: '#underscore-unused-vars'
Enabled: true
VersionAdded: '0.21'
VersionChanged: '0.35'
VersionChanged: '0.81'
AllowUnusedKeywordArguments: false
IgnoreEmptyMethods: true
IgnoreNotImplementedMethods: true

Lint/UriEscapeUnescape:
Description: >-
Expand Down
38 changes: 32 additions & 6 deletions lib/rubocop/cop/lint/unused_method_argument.rb
Expand Up @@ -38,9 +38,34 @@ module Lint
# def do_something(unused)
# end
#
# @example IgnoreNotImplementedMethods: true (default)
# # good
# def do_something(unused)
# raise NotImplementedError
# end
#
# def do_something_else(unused)
# fail "TODO"
# end
#
# @example IgnoreNotImplementedMethods: false
# # bad
# def do_something(unused)
# raise NotImplementedError
# end
#
# def do_something_else(unused)
# fail "TODO"
# end
#
class UnusedMethodArgument < Cop
include UnusedArgument

def_node_matcher :not_implemented?, <<~PATTERN
{(send nil? :raise (const nil? :NotImplementedError))
(send nil? :fail ...)}
PATTERN

def autocorrect(node)
UnusedArgCorrector.correct(processed_source, node)
end
Expand All @@ -51,16 +76,17 @@ def check_argument(variable)
return unless variable.method_argument?
return if variable.keyword_argument? &&
cop_config['AllowUnusedKeywordArguments']

if cop_config['IgnoreEmptyMethods']
body = variable.scope.node.body

return if body.nil?
end
return if ignored_method?(variable.scope.node.body)

super
end

def ignored_method?(body)
cop_config['IgnoreEmptyMethods'] && body.nil? ||
cop_config['IgnoreNotImplementedMethods'] &&
not_implemented?(body)
end

def message(variable)
message = +"Unused method argument - `#{variable.name}`."

Expand Down
27 changes: 26 additions & 1 deletion manual/cops_lint.md
Expand Up @@ -2636,7 +2636,7 @@ AllowUnusedKeywordArguments | `false` | Boolean

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | Yes | 0.21 | 0.35
Enabled | Yes | Yes | 0.21 | 0.81

This cop checks for unused method arguments.

Expand Down Expand Up @@ -2683,13 +2683,38 @@ end
def do_something(unused)
end
```
#### IgnoreNotImplementedMethods: true (default)

```ruby
# good
def do_something(unused)
raise NotImplementedError
end

def do_something_else(unused)
fail "TODO"
end
```
#### IgnoreNotImplementedMethods: false

```ruby
# bad
def do_something(unused)
raise NotImplementedError
end

def do_something_else(unused)
fail "TODO"
end
```

### Configurable attributes

Name | Default value | Configurable values
--- | --- | ---
AllowUnusedKeywordArguments | `false` | Boolean
IgnoreEmptyMethods | `true` | Boolean
IgnoreNotImplementedMethods | `true` | Boolean

### References

Expand Down
89 changes: 88 additions & 1 deletion spec/rubocop/cop/lint/unused_method_argument_spec.rb
Expand Up @@ -4,7 +4,11 @@
subject(:cop) { described_class.new(config) }

let(:cop_config) do
{ 'AllowUnusedKeywordArguments' => false, 'IgnoreEmptyMethods' => false }
{
'AllowUnusedKeywordArguments' => false,
'IgnoreEmptyMethods' => false,
'IgnoreNotImplementedMethods' => false
}
end

describe 'inspection' do
Expand Down Expand Up @@ -434,4 +438,87 @@ def method(a, b, *others)
RUBY
end
end

context 'when IgnoreNotImplementedMethods config parameter is set' do
let(:cop_config) { { 'IgnoreNotImplementedMethods' => true } }

it 'accepts a method with a single unused parameter & '\
'raises NotImplementedError' do
expect_no_offenses(<<~RUBY)
def method(arg)
raise NotImplementedError
end
RUBY
end

it 'accepts a method with a single unused parameter & '\
'fails with message' do
expect_no_offenses(<<~RUBY)
def method(arg)
fail "TODO"
end
RUBY
end

it 'accepts a method with a single unused parameter & '\
'fails without message' do
expect_no_offenses(<<~RUBY)
def method(arg)
fail
end
RUBY
end

it 'accepts an empty singleton method with a single unused parameter &'\
'raise NotImplementedError' do
expect_no_offenses(<<~RUBY)
def self.method(unused)
raise NotImplementedError
end
RUBY
end

it 'registers an offense for a non-empty method with a single unused ' \
'parameter' do
message = "Unused method argument - `arg`. If it's necessary, use " \
'`_` or `_arg` as an argument name to indicate that it ' \
"won't be used. You can also write as `method(*)` if you " \
"want the method to accept any arguments but don't care " \
'about them.'

expect_offense(<<~RUBY)
def method(arg)
^^^ #{message}
1
end
RUBY
end

it 'accepts an empty method with multiple unused parameters' do
expect_no_offenses(<<~RUBY)
def method(a, b, *others)
raise NotImplementedError
end
RUBY
end

it 'registers an offense for a non-empty method with multiple unused ' \
'parameters' do
(a_message, b_message, others_message) = %w[a b others].map do |arg|
"Unused method argument - `#{arg}`. If it's necessary, use `_` or " \
"`_#{arg}` as an argument name to indicate that it won't be used. " \
'You can also write as `method(*)` if you want the method ' \
"to accept any arguments but don't care about them."
end

expect_offense(<<~RUBY)
def method(a, b, *others)
^^^^^^ #{others_message}
^ #{b_message}
^ #{a_message}
1
end
RUBY
end
end
end

0 comments on commit 27d35b3

Please sign in to comment.