Skip to content

Commit

Permalink
Add new Workit/RedundantBooleanConditional cop
Browse files Browse the repository at this point in the history
  • Loading branch information
ydah committed Sep 12, 2023
1 parent ec4c2cf commit 738aee5
Show file tree
Hide file tree
Showing 5 changed files with 172 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unreleased

- Add new `Workit/RedundantBooleanConditional` cop. ([@ydah])

## 0.5.0 - 2022-12-27

- Deprecate `Workit/ComitteeAssertSchemaConfirm`. ([@ydah])
Expand Down
4 changes: 4 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ Workit/NoopRescue:
Description: "Check for suppress or ignore checked exception."
Enabled: false

Workit/RedundantBooleanConditional:
Description: "Checks for redundant boolean conditions."
Enabled: false

Workit/RestrictOnSend:
Description: |
Check for `RESTRICT_ON_SEND` is defined if `on_send` or `after_send` are defined.
Expand Down
66 changes: 66 additions & 0 deletions lib/rubocop/cop/workit/redundant_boolean_conditional.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Workit
# Checks for redundant boolean conditions.
#
# @example
# # bad
# true if x == y
#
# # good
# x == y
#
class RedundantBooleanConditional < Base
extend AutoCorrector

MSG = "This conditional expression can just be replaced by `%<replaced>s`."

# @!method true_or_false?(node)
def_node_matcher :true_or_false?, <<~RUBY
({:true :false})
RUBY

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

add_offense(node, message: offense_message(node)) do |corrector|
corrector.replace(node, replacement_condition(node))
end
end

private

def offense_message(node)
replacement = replacement_condition(node)
replaced = node.elsif? ? "\n#{replacement}" : replacement

format(MSG, replaced: replaced)
end

def redundant?(node)
return false if node.else? || node.elsif? || node.elsif_conditional? || node.ternary?

node.if_branch.true_type? || node.if_branch.false_type?
end

def replacement_condition(node)
condition = node.condition.source
expression = invert_expression?(node) ? "!(#{condition})" : condition

node.elsif? ? indented_else_node(expression, node) : expression
end

def invert_expression?(node)
(!node.elsif_conditional? && (node.if? && node.if_branch.false_type?)) ||
(node.unless? && node.if_branch.true_type?)
end

def indented_else_node(expression, node)
"else\n#{indentation(node)}#{expression}"
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/workitcop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
require_relative "rubocop/cop/workit/action_args"
require_relative "rubocop/cop/workit/committee_expected_response_status_code"
require_relative "rubocop/cop/workit/noop_rescue"
require_relative "rubocop/cop/workit/redundant_boolean_conditional"
require_relative "rubocop/cop/workit/restrict_on_send"
require_relative "rubocop/cop/workit/rspec_capybara_match_style"
require_relative "rubocop/cop/workit/rspec_capybara_predicate_matcher"
Expand Down
99 changes: 99 additions & 0 deletions spec/rubocop/cop/workit/redundant_boolean_conditional_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Workit::RedundantBooleanConditional, :config do
it "registers an offense for if with true results" do
expect_offense(<<~RUBY)
true if x == y
^^^^^^^^^^^^^^ This conditional expression can just be replaced by `x == y`.
RUBY

expect_correction(<<~RUBY)
x == y
RUBY
end

it "registers an offense for if with false results" do
expect_offense(<<~RUBY)
false if x == y
^^^^^^^^^^^^^^^ This conditional expression can just be replaced by `!(x == y)`.
RUBY

expect_correction(<<~RUBY)
!(x == y)
RUBY
end

it "registers an offense for unless with true results" do
expect_offense(<<~RUBY)
true unless x == y
^^^^^^^^^^^^^^^^^^ This conditional expression can just be replaced by `!(x == y)`.
RUBY

expect_correction(<<~RUBY)
!(x == y)
RUBY
end

it "registers an offense for unless with false results" do
expect_offense(<<~RUBY)
false unless x == y
^^^^^^^^^^^^^^^^^^^ This conditional expression can just be replaced by `x == y`.
RUBY

expect_correction(<<~RUBY)
x == y
RUBY
end

it "registers an offense for if block with true results" do
expect_offense(<<~RUBY)
if x == y
^^^^^^^^^ This conditional expression can just be replaced by `x == y`.
true
end
RUBY

expect_correction(<<~RUBY)
x == y
RUBY
end

it "registers an offense for if block with false results" do
expect_offense(<<~RUBY)
if x == y
^^^^^^^^^ This conditional expression can just be replaced by `!(x == y)`.
false
end
RUBY

expect_correction(<<~RUBY)
!(x == y)
RUBY
end

it "registers an offense for unless block with true results" do
expect_offense(<<~RUBY)
unless x == y
^^^^^^^^^^^^^ This conditional expression can just be replaced by `!(x == y)`.
true
end
RUBY

expect_correction(<<~RUBY)
!(x == y)
RUBY
end

it "registers an offense for unless block with false results" do
expect_offense(<<~RUBY)
unless x == y
^^^^^^^^^^^^^ This conditional expression can just be replaced by `x == y`.
false
end
RUBY

expect_correction(<<~RUBY)
x == y
RUBY
end
end

0 comments on commit 738aee5

Please sign in to comment.