Skip to content

Commit

Permalink
Add AllowKeywordBlockArguments option to UnderscorePrefixedVariableName
Browse files Browse the repository at this point in the history
  • Loading branch information
dduugg authored and bbatsov committed Apr 23, 2019
1 parent e002ac2 commit cd1d1a7
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -29,6 +29,7 @@
* [#6903](https://github.com/rubocop-hq/rubocop/issues/6903): Handle variables prefixed with `_` in `Naming/RescuedExceptionsVariableName` cop. ([@anthony-robin][])
* [#6917](https://github.com/rubocop-hq/rubocop/issues/6917): Bump Bundler dependency to >= 1.15.0. ([@koic][])
* Add `--auto-gen-only-exclude` to the command outputted in `rubocop_todo.yml` if the option is specified. ([@dvandersluis][])
* [#6887](https://github.com/rubocop-hq/rubocop/pull/6887): Allow `Lint/UnderscorePrefixedVariableName` cop to be configured to allow use of block keyword args. ([@dduugg][])

## 0.67.2 (2019-04-05)

Expand Down
1 change: 1 addition & 0 deletions config/default.yml
Expand Up @@ -1564,6 +1564,7 @@ Lint/UnderscorePrefixedVariableName:
Description: 'Do not use prefix `_` for a variable that is used.'
Enabled: true
VersionAdded: '0.21'
AllowKeywordBlockArguments: false

Lint/UnifiedInteger:
Description: 'Use Integer instead of Fixnum or Bignum'
Expand Down
30 changes: 25 additions & 5 deletions lib/rubocop/cop/lint/underscore_prefixed_variable_name.rb
Expand Up @@ -6,29 +6,40 @@ module Lint
# This cop checks for underscore-prefixed variables that are actually
# used.
#
# @example
# Since block keyword arguments cannot be arbitrarily named at call
# sites, the `AllowKeywordBlockArguments` will allow use of underscore-
# prefixed block keyword arguments.
#
# @example AllowKeywordBlockArguments: false (default)
#
# # bad
#
# [1, 2, 3].each do |_num|
# do_something(_num)
# end
#
# @example
# query(:sales) do |_id:, revenue:, cost:|
# {_id: _id, profit: revenue - cost}
# end
#
# # good
#
# [1, 2, 3].each do |num|
# do_something(num)
# end
#
# @example
# [1, 2, 3].each do |_num|
# do_something # not using `_num`
# end
#
# @example AllowKeywordBlockArguments: true
#
# # good
#
# [1, 2, 3].each do |_num|
# do_something # not using `_num`
# query(:sales) do |_id:, revenue:, cost:|
# {_id: _id, profit: revenue - cost}
# end
#
class UnderscorePrefixedVariableName < Cop
MSG = 'Do not use prefix `_` for a variable that is used.'.freeze

Expand All @@ -45,6 +56,7 @@ def after_leaving_scope(scope, _variable_table)
def check_variable(variable)
return unless variable.should_be_unused?
return if variable.references.none?(&:explicit?)
return if allowed_keyword_block_argument?(variable)

node = variable.declaration_node

Expand All @@ -56,6 +68,14 @@ def check_variable(variable)

add_offense(nil, location: location)
end

private

def allowed_keyword_block_argument?(variable)
variable.block_argument? &&
variable.keyword_argument? &&
cop_config['AllowKeywordBlockArguments']
end
end
end
end
Expand Down
29 changes: 25 additions & 4 deletions manual/cops_lint.md
Expand Up @@ -2077,30 +2077,51 @@ Enabled | Yes | No | 0.21 | -
This cop checks for underscore-prefixed variables that are actually
used.

Since block keyword arguments cannot be arbitrarily named at call
sites, the `AllowKeywordBlockArguments` will allow use of underscore-
prefixed block keyword arguments.

### Examples

#### AllowKeywordBlockArguments: false (default)

```ruby
# bad

[1, 2, 3].each do |_num|
do_something(_num)
end
```
```ruby

query(:sales) do |_id:, revenue:, cost:|
{_id: _id, profit: revenue - cost}
end

# good

[1, 2, 3].each do |num|
do_something(num)
end

[1, 2, 3].each do |_num|
do_something # not using `_num`
end
```
#### AllowKeywordBlockArguments: true

```ruby
# good

[1, 2, 3].each do |_num|
do_something # not using `_num`
query(:sales) do |_id:, revenue:, cost:|
{_id: _id, profit: revenue - cost}
end
```

### Configurable attributes

Name | Default value | Configurable values
--- | --- | ---
AllowKeywordBlockArguments | `false` | Boolean

## Lint/UnifiedInteger

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
Expand Down
37 changes: 33 additions & 4 deletions spec/rubocop/cop/lint/underscore_prefixed_variable_name_spec.rb
@@ -1,7 +1,9 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Lint::UnderscorePrefixedVariableName do
subject(:cop) { described_class.new }
RSpec.describe RuboCop::Cop::Lint::UnderscorePrefixedVariableName, :config do
subject(:cop) { described_class.new(config) }

let(:cop_config) { { 'AllowKeywordBlockArguments' => false } }

context 'when an underscore-prefixed variable is used' do
it 'registers an offense' do
Expand Down Expand Up @@ -49,14 +51,41 @@ def some_method(_foo)
end

context 'when an underscore-prefixed block argument is used' do
[true, false].each do |config|
let(:cop_config) { { 'AllowKeywordBlockArguments' => config } }

it 'registers an offense' do
expect_offense(<<-RUBY.strip_indent)
1.times do |_foo|
^^^^ Do not use prefix `_` for a variable that is used.
puts _foo
end
RUBY
end
end
end

context 'when an underscore-prefixed keyword block argument is used' do
it 'registers an offense' do
expect_offense(<<-RUBY.strip_indent)
1.times do |_foo|
^^^^ Do not use prefix `_` for a variable that is used.
define_method(:foo) do |_foo: 'default'|
^^^^ Do not use prefix `_` for a variable that is used.
puts _foo
end
RUBY
end

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

it 'does not register an offense' do
expect_no_offenses(<<-RUBY.strip_indent)
define_method(:foo) do |_foo: 'default'|
puts _foo
end
RUBY
end
end
end

context 'when an underscore-prefixed variable in top-level scope is used' do
Expand Down

0 comments on commit cd1d1a7

Please sign in to comment.