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 new Style/OptionalBooleanParameter cop #8412

Merged
merged 1 commit into from Jul 30, 2020
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 @@ -7,6 +7,7 @@
* [#8322](https://github.com/rubocop-hq/rubocop/pull/8322): Support autocorrect for `Style/CaseEquality` cop. ([@fatkodima][])
* [#8291](https://github.com/rubocop-hq/rubocop/pull/8291): Add new `Lint/SelfAssignment` cop. ([@fatkodima][])
* [#8389](https://github.com/rubocop-hq/rubocop/pull/8389): Add new `Lint/DuplicateRescueException` cop. ([@fatkodima][])
* [#8412](https://github.com/rubocop-hq/rubocop/pull/8412): Add new `Style/OptionalBooleanParameter` cop. ([@fatkodima][])
* [#8376](https://github.com/rubocop-hq/rubocop/pull/8376): Add new `Lint/MissingSuper` cop. ([@fatkodima][])
* [#8339](https://github.com/rubocop-hq/rubocop/pull/8339): Add `Config#for_badge` as an efficient way to get a cop's config merged with its department's. ([@marcandre][])
* [#5067](https://github.com/rubocop-hq/rubocop/issues/5067): Add new `Style/StringConcatenation` cop. ([@fatkodima][])
Expand Down
7 changes: 7 additions & 0 deletions config/default.yml
Expand Up @@ -3583,6 +3583,13 @@ Style/OptionalArguments:
VersionAdded: '0.33'
VersionChanged: '0.83'

Style/OptionalBooleanParameter:
Description: 'Use keyword arguments when defining method with boolean argument.'
StyleGuide: '#boolean-keyword-arguments'
Enabled: pending
Safe: false
VersionAdded: '0.89'

Style/OrAssignment:
Description: 'Recommend usage of double pipe equals (||=) where applicable.'
StyleGuide: '#double-pipe-for-uninit'
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -430,6 +430,7 @@ In the following section you find all available cops:
* xref:cops_style.adoc#styleonelineconditional[Style/OneLineConditional]
* xref:cops_style.adoc#styleoptionhash[Style/OptionHash]
* xref:cops_style.adoc#styleoptionalarguments[Style/OptionalArguments]
* xref:cops_style.adoc#styleoptionalbooleanparameter[Style/OptionalBooleanParameter]
* xref:cops_style.adoc#styleorassignment[Style/OrAssignment]
* xref:cops_style.adoc#styleparallelassignment[Style/ParallelAssignment]
* xref:cops_style.adoc#styleparenthesesaroundcondition[Style/ParenthesesAroundCondition]
Expand Down
40 changes: 40 additions & 0 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -6547,6 +6547,46 @@ end

* https://rubystyle.guide#optional-arguments

== Style/OptionalBooleanParameter

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| No
| No
| 0.89
| -
|===

This cop checks for places where keyword arguments can be used instead of
boolean arguments when defining methods.

=== Examples

[source,ruby]
----
# bad
def some_method(bar = false)
puts bar
end

# bad - common hack before keyword args were introduced
def some_method(options = {})
bar = options.fetch(:bar, false)
puts bar
end

# good
def some_method(bar: false)
puts bar
end
----

=== References

* https://rubystyle.guide#boolean-keyword-arguments

== Style/OrAssignment

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -480,6 +480,7 @@
require_relative 'rubocop/cop/style/or_assignment'
require_relative 'rubocop/cop/style/option_hash'
require_relative 'rubocop/cop/style/optional_arguments'
require_relative 'rubocop/cop/style/optional_boolean_parameter'
require_relative 'rubocop/cop/style/parallel_assignment'
require_relative 'rubocop/cop/style/parentheses_around_condition'
require_relative 'rubocop/cop/style/percent_literal_delimiters'
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cli/command/execute_runner.rb
Expand Up @@ -55,7 +55,7 @@ def display_error_summary(errors)
#{Gem.loaded_specs['rubocop'].metadata['bug_tracker_uri']}

Mention the following information in the issue report:
#{RuboCop::Version.version(true)}
#{RuboCop::Version.version(debug: true)}
WARNING
end

Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cli/command/version.rb
Expand Up @@ -8,8 +8,8 @@ class Version < Base
self.command_name = :version

def run
puts RuboCop::Version.version(false) if @options[:version]
puts RuboCop::Version.version(true) if @options[:verbose_version]
puts RuboCop::Version.version(debug: false) if @options[:version]
puts RuboCop::Version.version(debug: true) if @options[:verbose_version]
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/migration/department_name.rb
Expand Up @@ -51,7 +51,7 @@ def check_cop_name(name, comment, offset)

add_offense(range) do |corrector|
cop_name = range.source
qualified_cop_name = Cop.registry.qualified_cop_name(cop_name, nil, false)
qualified_cop_name = Cop.registry.qualified_cop_name(cop_name, nil, warn: false)

unless qualified_cop_name.include?('/')
qualified_cop_name = qualified_legacy_cop_name(cop_name)
Expand Down
6 changes: 3 additions & 3 deletions lib/rubocop/cop/registry.rb
Expand Up @@ -100,9 +100,9 @@ def contains_cop_matching?(names)
# @note Emits a warning if the provided name has an incorrect namespace
#
# @return [String] Qualified cop name
def qualified_cop_name(name, path, shall_warn = true)
def qualified_cop_name(name, path, warn: true)
badge = Badge.parse(name)
print_warning(name, path) if shall_warn && department_missing?(badge, name)
print_warning(name, path) if warn && department_missing?(badge, name)
return name if registered?(badge)

potential_badges = qualify_badge(badge)
Expand Down Expand Up @@ -149,7 +149,7 @@ def length
@registry.size
end

def enabled(config, only = [], only_safe = false)
def enabled(config, only = [], only_safe: false)
select do |cop|
only.include?(cop.cop_name) || enabled?(cop, config, only_safe)
end
Expand Down
8 changes: 4 additions & 4 deletions lib/rubocop/cop/style/documentation.rb
Expand Up @@ -112,17 +112,17 @@ def compact_namespace?(node)
# proceeds to check its ancestors for :nodoc: all.
# Note: How end-of-line comments are associated with code changed in
# parser-2.2.0.4.
def nodoc_comment?(node, require_all = false)
def nodoc_comment?(node, require_all: false)
return false unless node&.children&.first

nodoc = nodoc(node)

return true if same_line?(nodoc, node) && nodoc?(nodoc, require_all)
return true if same_line?(nodoc, node) && nodoc?(nodoc, require_all: require_all)

nodoc_comment?(node.parent, true)
nodoc_comment?(node.parent, require_all: true)
end

def nodoc?(comment, require_all = false)
def nodoc?(comment, require_all: false)
/^#\s*:nodoc:#{"\s+all\s*$" if require_all}/.match?(comment.text)
end

Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/style/guard_clause.rb
Expand Up @@ -87,7 +87,7 @@ def on_if(node)
private

def check_ending_if(node)
return if accepted_form?(node, true) || !min_body_length?(node)
return if accepted_form?(node, ending: true) || !min_body_length?(node)

register_offense(node, 'return', opposite_keyword(node))
end
Expand Down Expand Up @@ -125,7 +125,7 @@ def too_long_for_single_line?(node, example)
max && node.source_range.column + example.length > max
end

def accepted_form?(node, ending = false)
def accepted_form?(node, ending: false)
accepted_if?(node, ending) || node.condition.multiline? ||
node.parent&.assignment?
end
Expand Down
42 changes: 42 additions & 0 deletions lib/rubocop/cop/style/optional_boolean_parameter.rb
@@ -0,0 +1,42 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# This cop checks for places where keyword arguments can be used instead of
# boolean arguments when defining methods.
#
# @example
# # bad
# def some_method(bar = false)
# puts bar
# end
#
# # bad - common hack before keyword args were introduced
# def some_method(options = {})
# bar = options.fetch(:bar, false)
# puts bar
# end
#
# # good
# def some_method(bar: false)
# puts bar
# end
#
class OptionalBooleanParameter < Base
MSG = 'Use keyword arguments when defining method with boolean argument.'
BOOLEAN_TYPES = %i[true false].freeze

def on_def(node)
node.arguments.each do |arg|
next unless arg.optarg_type?

_name, value = *arg
add_offense(arg) if BOOLEAN_TYPES.include?(value.type)
end
end
alias on_defs on_def
end
end
end
end
2 changes: 1 addition & 1 deletion lib/rubocop/cop/team.rb
Expand Up @@ -43,7 +43,7 @@ def self.mobilize_cops(cop_classes, config, options = {})
cop_classes = Registry.new(cop_classes.to_a) unless cop_classes.is_a?(Registry)
only = options.fetch(:only, [])
safe = options.fetch(:safe, false)
cop_classes.enabled(config, only, safe).map do |cop_class|
cop_classes.enabled(config, only, only_safe: safe).map do |cop_class|
cop_class.new(config, options)
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/version.rb
Expand Up @@ -9,7 +9,7 @@ module Version
'rubocop-ast %<rubocop_ast_version>s, ' \
'running on %<ruby_engine>s %<ruby_version>s %<ruby_platform>s)'

def self.version(debug = false)
def self.version(debug: false)
if debug
format(MSG, version: STRING, parser_version: Parser::VERSION,
rubocop_ast_version: RuboCop::AST::Version::STRING,
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cop/registry_spec.rb
Expand Up @@ -196,7 +196,7 @@
end

it 'selects only safe cops if :safe passed' do
enabled_cops = registry.enabled(config, [], true)
enabled_cops = registry.enabled(config, [], only_safe: true)
expect(enabled_cops).not_to include(RuboCop::Cop::RSpec::Foo)
end

Expand Down
51 changes: 51 additions & 0 deletions spec/rubocop/cop/style/optional_boolean_parameter_spec.rb
@@ -0,0 +1,51 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Style::OptionalBooleanParameter do
subject(:cop) { described_class.new }

it 'registers an offense when defining method with optional boolean arg' do
expect_offense(<<~RUBY)
def some_method(bar = false)
^^^^^^^^^^^ Use keyword arguments when defining method with boolean argument.
end
RUBY
end

it 'registers an offense when defining class method with optional boolean arg' do
expect_offense(<<~RUBY)
def self.some_method(bar = false)
^^^^^^^^^^^ Use keyword arguments when defining method with boolean argument.
end
RUBY
end

it 'registers an offense when defining method with multiple optional boolean args' do
expect_offense(<<~RUBY)
def some_method(foo = true, bar = 1, baz = false, quux: true)
^^^^^^^^^^ Use keyword arguments when defining method with boolean argument.
^^^^^^^^^^^ Use keyword arguments when defining method with boolean argument.
end
RUBY
end

it 'does not register an offense when defining method with keyword boolean arg' do
expect_no_offenses(<<~RUBY)
def some_method(bar: false)
end
RUBY
end

it 'does not register an offense when defining method without args' do
expect_no_offenses(<<~RUBY)
def some_method
end
RUBY
end

it 'does not register an offense when defining method with optonal non-boolean arg' do
expect_no_offenses(<<~RUBY)
def some_method(bar = 'foo')
end
RUBY
end
end