Skip to content

Commit

Permalink
[Fix rubocop#11006] Allow multiple elsif for `Style/IfWithBooleanLi…
Browse files Browse the repository at this point in the history
…teralBranches`

Fixes rubocop#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.
  • Loading branch information
koic committed Sep 16, 2022
1 parent 1e35c57 commit 8122219
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 1 deletion.
@@ -0,0 +1 @@
* [#11006](https://github.com/rubocop/rubocop/issues/11006): Allow multiple `elsif` for `Style/IfWithBooleanLiteralBranches`. ([@koic][])
22 changes: 21 additions & 1 deletion lib/rubocop/cop/style/if_with_boolean_literal_branches.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
56 changes: 56 additions & 0 deletions spec/rubocop/cop/style/if_with_boolean_literal_branches_spec.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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?'] } }

Expand Down

0 comments on commit 8122219

Please sign in to comment.