From 812221918bd44d1480e0eed2fef2e21d492d3009 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Fri, 16 Sep 2022 20:52:39 +0900 Subject: [PATCH] [Fix #11006] Allow multiple `elsif` for `Style/IfWithBooleanLiteralBranches` Fixes #11006. This PR allows multiple `elsif` for `Style/IfWithBooleanLiteralBranches`. I think it's good to allow when multiple `elsif`s are used instead of a new config option. OTOH, if there is only single `elsif`, `elsif` will not exist due to autocorrection. So I think the current behavior can be kept. --- ..._style_if_with_boolean_literal_branches.md | 1 + .../style/if_with_boolean_literal_branches.rb | 22 +++++++- .../if_with_boolean_literal_branches_spec.rb | 56 +++++++++++++++++++ 3 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 changelog/change_allow_multiple_elsif_for_style_if_with_boolean_literal_branches.md diff --git a/changelog/change_allow_multiple_elsif_for_style_if_with_boolean_literal_branches.md b/changelog/change_allow_multiple_elsif_for_style_if_with_boolean_literal_branches.md new file mode 100644 index 00000000000..5b60cd562ac --- /dev/null +++ b/changelog/change_allow_multiple_elsif_for_style_if_with_boolean_literal_branches.md @@ -0,0 +1 @@ +* [#11006](https://github.com/rubocop/rubocop/issues/11006): Allow multiple `elsif` for `Style/IfWithBooleanLiteralBranches`. ([@koic][]) diff --git a/lib/rubocop/cop/style/if_with_boolean_literal_branches.rb b/lib/rubocop/cop/style/if_with_boolean_literal_branches.rb index 0f3db5e42df..7a464fbc591 100644 --- a/lib/rubocop/cop/style/if_with_boolean_literal_branches.rb +++ b/lib/rubocop/cop/style/if_with_boolean_literal_branches.rb @@ -8,6 +8,20 @@ module Style # The conditions to be checked are comparison methods, predicate methods, and double negative. # `nonzero?` method is allowed by default. # These are customizable with `AllowedMethods` option. + # This cop targets only `if`s with a single `elsif` or `else` branch. + # + # [source,ruby] + # ---- + # if foo + # true + # elsif bar > baz + # true + # elsif qux > quux # Single `elsif` is warned, but two or more `elsif`s are not. + # true + # else + # false + # end + # ---- # # @safety # Autocorrection is unsafe because there is no guarantee that all predicate methods @@ -57,7 +71,7 @@ class IfWithBooleanLiteralBranches < Base def_node_matcher :double_negative?, '(send (send _ :!) :!)' def on_if(node) - return unless if_with_boolean_literal_branches?(node) + return if !if_with_boolean_literal_branches?(node) || multiple_elsif?(node) condition = node.condition range, keyword = offense_range_with_keyword(node, condition) @@ -76,6 +90,12 @@ def on_if(node) private + def multiple_elsif?(node) + return false unless (parent = node.parent) + + parent.if_type? && parent.elsif? + end + def offense_range_with_keyword(node, condition) if node.ternary? range = condition.source_range.end.join(node.source_range.end) diff --git a/spec/rubocop/cop/style/if_with_boolean_literal_branches_spec.rb b/spec/rubocop/cop/style/if_with_boolean_literal_branches_spec.rb index 802e570ec14..c3df9d76cac 100644 --- a/spec/rubocop/cop/style/if_with_boolean_literal_branches_spec.rb +++ b/spec/rubocop/cop/style/if_with_boolean_literal_branches_spec.rb @@ -85,6 +85,25 @@ RUBY end + it 'registers and corrects an offense when using `if` with boolean literal branches directly under `def`' do + expect_offense(<<~RUBY, comparison_operator: comparison_operator) + def foo + if bar > baz + ^^ Remove redundant `if` with boolean literal branches. + true + else + false + end + end + RUBY + + expect_correction(<<~RUBY) + def foo + bar > baz + end + RUBY + end + it 'does not register an offense when using a branch that is not boolean literal' do expect_no_offenses(<<~RUBY) if foo #{comparison_operator} bar @@ -555,6 +574,43 @@ end end + context 'when using `elsif` with boolean literal branches' do + it 'registers and corrects an offense when using single `elsif` with boolean literal branches' do + expect_offense(<<~RUBY) + if foo + true + elsif bar > baz + ^^^^^ Use `else` instead of redundant `elsif` with boolean literal branches. + true + else + false + end + RUBY + + expect_correction(<<~RUBY) + if foo + true + else + bar > baz + end + RUBY + end + + it 'does not register an offense when using multiple `elsif` with boolean literal branches' do + expect_no_offenses(<<~RUBY) + if foo + true + elsif bar > baz + true + elsif qux > quux + true + else + false + end + RUBY + end + end + context 'when `AllowedMethods: nonzero?`' do let(:cop_config) { { 'AllowedMethods' => ['nonzero?'] } }