From df70484dc92f7202782f1b069c5001c8e7c33424 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Wed, 29 Jul 2020 18:43:15 +0300 Subject: [PATCH] Add new `Style/OptionalBooleanParameter` cop --- CHANGELOG.md | 1 + config/default.yml | 7 +++ docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_style.adoc | 40 +++++++++++++++ lib/rubocop.rb | 1 + lib/rubocop/cli/command/execute_runner.rb | 2 +- lib/rubocop/cli/command/version.rb | 4 +- lib/rubocop/cop/migration/department_name.rb | 2 +- lib/rubocop/cop/registry.rb | 6 +-- lib/rubocop/cop/style/documentation.rb | 8 +-- lib/rubocop/cop/style/guard_clause.rb | 4 +- .../cop/style/optional_boolean_parameter.rb | 42 +++++++++++++++ lib/rubocop/cop/team.rb | 2 +- lib/rubocop/version.rb | 2 +- spec/rubocop/cop/registry_spec.rb | 2 +- .../style/optional_boolean_parameter_spec.rb | 51 +++++++++++++++++++ 16 files changed, 159 insertions(+), 16 deletions(-) create mode 100644 lib/rubocop/cop/style/optional_boolean_parameter.rb create mode 100644 spec/rubocop/cop/style/optional_boolean_parameter_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index cef3f0b8d48..f50a3218d9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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][]) diff --git a/config/default.yml b/config/default.yml index fe609f251e4..b284d4b002d 100644 --- a/config/default.yml +++ b/config/default.yml @@ -3589,6 +3589,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' diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 3a481041de7..c8b52a383ce 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -431,6 +431,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] diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc index 55099ba14b3..2d3fa76d42b 100644 --- a/docs/modules/ROOT/pages/cops_style.adoc +++ b/docs/modules/ROOT/pages/cops_style.adoc @@ -6591,6 +6591,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 |=== diff --git a/lib/rubocop.rb b/lib/rubocop.rb index e84497070ea..901bdfbd269 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -481,6 +481,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' diff --git a/lib/rubocop/cli/command/execute_runner.rb b/lib/rubocop/cli/command/execute_runner.rb index d3c5f0f3d69..f4dff2ffd10 100644 --- a/lib/rubocop/cli/command/execute_runner.rb +++ b/lib/rubocop/cli/command/execute_runner.rb @@ -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 diff --git a/lib/rubocop/cli/command/version.rb b/lib/rubocop/cli/command/version.rb index 34ece180b79..81190286afc 100644 --- a/lib/rubocop/cli/command/version.rb +++ b/lib/rubocop/cli/command/version.rb @@ -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 diff --git a/lib/rubocop/cop/migration/department_name.rb b/lib/rubocop/cop/migration/department_name.rb index 3ad5b92b6a1..5105ee1bcd2 100644 --- a/lib/rubocop/cop/migration/department_name.rb +++ b/lib/rubocop/cop/migration/department_name.rb @@ -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) diff --git a/lib/rubocop/cop/registry.rb b/lib/rubocop/cop/registry.rb index 38144d6f6c7..4acba701e01 100644 --- a/lib/rubocop/cop/registry.rb +++ b/lib/rubocop/cop/registry.rb @@ -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) @@ -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 diff --git a/lib/rubocop/cop/style/documentation.rb b/lib/rubocop/cop/style/documentation.rb index 240b875408b..6fb47f64dbe 100644 --- a/lib/rubocop/cop/style/documentation.rb +++ b/lib/rubocop/cop/style/documentation.rb @@ -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 diff --git a/lib/rubocop/cop/style/guard_clause.rb b/lib/rubocop/cop/style/guard_clause.rb index 55060d6bb2c..24086c9f779 100644 --- a/lib/rubocop/cop/style/guard_clause.rb +++ b/lib/rubocop/cop/style/guard_clause.rb @@ -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 @@ -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 diff --git a/lib/rubocop/cop/style/optional_boolean_parameter.rb b/lib/rubocop/cop/style/optional_boolean_parameter.rb new file mode 100644 index 00000000000..58573956ea8 --- /dev/null +++ b/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 diff --git a/lib/rubocop/cop/team.rb b/lib/rubocop/cop/team.rb index 254d48b8546..e4d1e62b060 100644 --- a/lib/rubocop/cop/team.rb +++ b/lib/rubocop/cop/team.rb @@ -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 diff --git a/lib/rubocop/version.rb b/lib/rubocop/version.rb index 152df14849b..4e0575ffc92 100644 --- a/lib/rubocop/version.rb +++ b/lib/rubocop/version.rb @@ -9,7 +9,7 @@ module Version 'rubocop-ast %s, ' \ 'running on %s %s %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, diff --git a/spec/rubocop/cop/registry_spec.rb b/spec/rubocop/cop/registry_spec.rb index 56d6f6e2763..fe73c800bc3 100644 --- a/spec/rubocop/cop/registry_spec.rb +++ b/spec/rubocop/cop/registry_spec.rb @@ -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 diff --git a/spec/rubocop/cop/style/optional_boolean_parameter_spec.rb b/spec/rubocop/cop/style/optional_boolean_parameter_spec.rb new file mode 100644 index 00000000000..ff0bba53320 --- /dev/null +++ b/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