Skip to content

Commit

Permalink
Add new Style/IfWithBooleanLiteralBranches cop
Browse files Browse the repository at this point in the history
This PR adds new `Style/IfWithBooleanLiteralBranches` cop that checks
for redundant `if` with boolean literal branches.

It checks only conditions to return boolean value (`true` or `false`)
for safe detection.
The conditions to be checked are comparison methods, predicate methods,
and double negative.

```ruby
# bad
if foo == bar
  true
else
  false
end

# bad
foo == bar ? true : false

# good
foo == bar
```

And this PR auto-corrected the following offense by the new cop.

```console
% bundle exec rake
(snip)

Offenses:

lib/rubocop/cop/style/ternary_parentheses.rb:123:47: C: [Correctable]
Style/IfWithBooleanLiteralBranches: Remove redundant ternary operator
with boolean literal branches.
            non_complex_expression?(condition) ? false : true
                                              ^^^^^^^^^^^^^^^

1246 files inspected, 1 offense detected, 1 offense auto-correctable
RuboCop failed!
```
  • Loading branch information
koic authored and bbatsov committed Jan 26, 2021
1 parent 6f6efd0 commit fdf25f2
Show file tree
Hide file tree
Showing 6 changed files with 615 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#9396](https://github.com/rubocop-hq/rubocop/pull/9396): Add new `Style/IfWithBooleanLiteralBranches` cop. ([@koic][])
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3517,6 +3517,11 @@ Style/IfUnlessModifierOfIfUnless:
VersionAdded: '0.39'
VersionChanged: '0.87'

Style/IfWithBooleanLiteralBranches:
Description: 'Checks for redundant `if` with boolean literal branches.'
Enabled: pending
VersionAdded: '<<next>>'

Style/IfWithSemicolon:
Description: 'Do not use if x; .... Use the ternary operator instead.'
StyleGuide: '#no-semicolon-ifs'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,7 @@
require_relative 'rubocop/cop/style/if_inside_else'
require_relative 'rubocop/cop/style/if_unless_modifier'
require_relative 'rubocop/cop/style/if_unless_modifier_of_if_unless'
require_relative 'rubocop/cop/style/if_with_boolean_literal_branches'
require_relative 'rubocop/cop/style/if_with_semicolon'
require_relative 'rubocop/cop/style/implicit_runtime_error'
require_relative 'rubocop/cop/style/infinite_loop'
Expand Down
96 changes: 96 additions & 0 deletions lib/rubocop/cop/style/if_with_boolean_literal_branches.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# This cop checks for redundant `if` with boolean literal branches.
# It checks only conditions to return boolean value (`true` or `false`) for safe detection.
# The conditions to be checked are comparison methods, predicate methods, and double negative.
#
# @example
# # bad
# if foo == bar
# true
# else
# false
# end
#
# # bad
# foo == bar ? true : false
#
# # good
# foo == bar
#
class IfWithBooleanLiteralBranches < Base
extend AutoCorrector

MSG = 'Remove redundant %<keyword>s with boolean literal branches.'

def_node_matcher :if_with_boolean_literal_branches?, <<~PATTERN
(if #return_boolean_value? {(true) (false) | (false) (true)})
PATTERN
def_node_matcher :double_negative?, '(send (send _ :!) :!)'

def on_if(node)
return unless if_with_boolean_literal_branches?(node)

condition = node.condition
range, keyword = if node.ternary?
range = condition.source_range.end.join(node.source_range.end)

[range, 'ternary operator']
else
keyword = node.loc.keyword

[keyword, "`#{keyword.source}`"]
end

add_offense(range, message: format(MSG, keyword: keyword)) do |corrector|
replacement = replacement_condition(node, condition)

corrector.replace(node, replacement)
end
end

private

def return_boolean_value?(condition)
if condition.begin_type?
return_boolean_value?(condition.children.first)
elsif condition.or_type?
return_boolean_value?(condition.lhs) && return_boolean_value?(condition.rhs)
elsif condition.and_type?
return_boolean_value?(condition.rhs)
else
assume_boolean_value?(condition)
end
end

def assume_boolean_value?(condition)
return false unless condition.send_type?

condition.comparison_method? || condition.predicate_method? || double_negative?(condition)
end

def replacement_condition(node, condition)
bang = '!' if opposite_condition?(node)

if bang && require_parentheses?(condition)
"#{bang}(#{condition.source})"
else
"#{bang}#{condition.source}"
end
end

def opposite_condition?(node)
!node.unless? && node.if_branch.false_type? || node.unless? && node.if_branch.true_type?
end

def require_parentheses?(condition)
condition.and_type? || condition.or_type? ||
condition.send_type? && condition.comparison_method?
end
end
end
end
end
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/ternary_parentheses.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def complex_condition?(condition)
if condition.begin_type?
condition.to_a.any? { |x| complex_condition?(x) }
else
non_complex_expression?(condition) ? false : true
!non_complex_expression?(condition)
end
end

Expand Down

0 comments on commit fdf25f2

Please sign in to comment.