Skip to content

Commit

Permalink
Add new Style/InvertibleUnlessCondition cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima committed Jan 15, 2023
1 parent b48ebef commit fafee9c
Show file tree
Hide file tree
Showing 5 changed files with 239 additions and 0 deletions.
1 change: 1 addition & 0 deletions changelog/new_add_new_invertible_unless_condition_cop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#11410](https://github.com/rubocop/rubocop/issues/11410): Add new `Style/InvertibleUnlessCondition` cop. ([@fatkodima][])
29 changes: 29 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4036,6 +4036,35 @@ Style/InverseMethods:
:select: :reject
:select!: :reject!

Style/InvertibleUnlessCondition:
Description: 'Favor `if` with inverted condition over `unless`.'
Enabled: false
VersionAdded: '<<next>>'
# `InverseMethods` are methods that can be inverted in a `unless` condition.
# The relationship of inverse methods needs to be defined in both directions.
# Keys and values both need to be defined as symbols.
InverseMethods:
:!=: :==
:>: :<=
:<=: :>
:<: :>=
:>=: :<
:!~: :=~
:zero?: :nonzero?
:nonzero?: :zero?
:any?: :none?
:none?: :any?
:even?: :odd?
:odd?: :even?
# `ActiveSupport` defines some common inverse methods. They are listed below,
# and not enabled by default.
# :present?: :blank?
# :blank?: :present?
# :include?: :exclude?
# :exclude?: :include?
# :one?: :many?
# :many?: :one?

Style/IpAddresses:
Description: "Don't include literal IP addresses in code."
Enabled: false
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,7 @@
require_relative 'rubocop/cop/style/infinite_loop'
require_relative 'rubocop/cop/style/inverse_methods'
require_relative 'rubocop/cop/style/inline_comment'
require_relative 'rubocop/cop/style/invertible_unless_condition'
require_relative 'rubocop/cop/style/ip_addresses'
require_relative 'rubocop/cop/style/keyword_parameters_order'
require_relative 'rubocop/cop/style/lambda'
Expand Down
114 changes: 114 additions & 0 deletions lib/rubocop/cop/style/invertible_unless_condition.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# Checks for usages of `unless` which can be replaced by `if` with inverted condition.
# Code without `unless` is easier to read, but that is subjective, so this cop
# is disabled by default.
#
# Methods that can be inverted should be defined in `InverseMethods`. Note that
# the relationship of inverse methods needs to be defined in both directions.
# For example,
# InverseMethods:
# :!=: :==
# :even?: :odd?
# :odd?: :even?
#
# will suggest both `even?` and `odd?` to be inverted, but only `!=` (and not `==`).
#
# @safety
# This cop is unsafe because it cannot be guaranteed that the method
# and its inverse method are both defined on receiver, and also are
# actually inverse of each other.
#
# @example
# # bad (simple condition)
# foo unless !bar
# foo unless x != y
# foo unless x >= 10
# foo unless x.even?
#
# # good
# foo if bar
# foo if x == y
# foo if x < 10
# foo if x.odd?
#
# # bad (complex condition)
# foo unless x != y || x.even?
#
# # good
# foo if x == y && x.odd?
#
# # good (if)
# foo if !condition
#
class InvertibleUnlessCondition < Base
extend AutoCorrector

MSG = 'Favor `if` with inverted condition over `unless`.'

def on_if(node)
return unless node.unless?

condition = node.condition
return unless invertible?(condition)

add_offense(node) do |corrector|
corrector.replace(node.loc.keyword, node.inverse_keyword)
autocorrect(corrector, condition)
end
end

private

def invertible?(node)
case node.type
when :begin
invertible?(node.children.first)
when :send
return if inheritance_check?(node)

node.method?(:!) || inverse_methods.key?(node.method_name)
when :or, :and
invertible?(node.lhs) && invertible?(node.rhs)
else
false
end
end

def inheritance_check?(node)
argument = node.first_argument
node.method?(:<) &&
(argument.const_type? && argument.short_name.to_s.upcase != argument.short_name.to_s)
end

def autocorrect(corrector, node)
case node.type
when :begin
autocorrect(corrector, node.children.first)
when :send
autocorrect_send_node(corrector, node)
when :or, :and
corrector.replace(node.loc.operator, node.inverse_operator)
autocorrect(corrector, node.lhs)
autocorrect(corrector, node.rhs)
end
end

def autocorrect_send_node(corrector, node)
if node.method?(:!)
corrector.remove(node.loc.selector)
else
corrector.replace(node.loc.selector, inverse_methods[node.method_name])
end
end

def inverse_methods
@inverse_methods ||= cop_config['InverseMethods']
end
end
end
end
end
94 changes: 94 additions & 0 deletions spec/rubocop/cop/style/invertible_unless_condition_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Style::InvertibleUnlessCondition, :config do
let(:cop_config) do
{
'InverseMethods' => {
:!= => :==,
:even? => :odd?,
:odd? => :even?,
:>= => :<,
:< => :>=,
:zero? => :nonzero?
}
}
end

context 'when invertible `unless`' do
it 'registers an offense and corrects when using `!` negation' do
expect_offense(<<~RUBY)
foo unless !x
^^^^^^^^^^^^^ Favor `if` with inverted condition over `unless`.
foo unless !!x
^^^^^^^^^^^^^^ Favor `if` with inverted condition over `unless`.
RUBY

expect_correction(<<~RUBY)
foo if x
foo if !x
RUBY
end

it 'registers an offense and corrects when using simple operator condition' do
expect_offense(<<~RUBY)
foo unless x != y
^^^^^^^^^^^^^^^^^ Favor `if` with inverted condition over `unless`.
RUBY

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

it 'registers an offense and corrects when using simple method condition' do
expect_offense(<<~RUBY)
foo unless x.odd?
^^^^^^^^^^^^^^^^^ Favor `if` with inverted condition over `unless`.
RUBY

expect_correction(<<~RUBY)
foo if x.even?
RUBY
end

it 'registers an offense and corrects when using simple bracketed condition' do
expect_offense(<<~RUBY)
foo unless ((x != y))
^^^^^^^^^^^^^^^^^^^^^ Favor `if` with inverted condition over `unless`.
RUBY

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

it 'registers an offense and corrects when using complex condition' do
expect_offense(<<~RUBY)
foo unless x != y && (((x.odd?) || (((y >= 5)))) || z.zero?)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor `if` with inverted condition over `unless`.
RUBY

expect_correction(<<~RUBY)
foo if x == y || (((x.even?) && (((y < 5)))) && z.nonzero?)
RUBY
end
end

it 'does not register an offense when using non invertible `unless`' do
expect_no_offenses(<<~RUBY)
foo unless x != y || x.awesome?
RUBY
end

it 'does not register an offense when checking for inheritance' do
expect_no_offenses(<<~RUBY)
foo unless x < Foo
RUBY
end

it 'does not register an offense when using invertible `if`' do
expect_no_offenses(<<~RUBY)
foo if !condition
RUBY
end
end

0 comments on commit fafee9c

Please sign in to comment.