Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add AllowKeywordBlockArguments option to UnderscorePrefixedVariableName #6887

Merged
merged 1 commit into from Apr 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -26,6 +26,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