Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix #5388] Add new Style/UnlessMultipleConditions cop #5400

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions .rubocop.yml
Expand Up @@ -29,6 +29,12 @@ Style/FormatStringToken:
Exclude:
- spec/**/*

Style/UnlessMultipleConditions:
# Because RuboCop currently supports Ruby 2.1.
# In Ruby 2.1 not implementing `Hash#dig`, the following code is useful.
# `unless hash[:a] && hash[:a][:b]`
Enabled: false

Layout/EndOfLine:
EnforcedStyle: lf

Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -13,6 +13,7 @@
* [#5319](https://github.com/bbatsov/rubocop/pull/5319): Add new `Security/Open` cop. ([@mame][])
* Add `EnforcedStyleForEmptyBrackets` configuration to `Layout/SpaceInsideReferenceBrackets`.([@garettarrowood][])
* [#5358](https://github.com/bbatsov/rubocop/pull/5358): `--no-auto-gen-timestamp` CLI option suppresses the inclusion of the date and time it was generated in auto-generated config. ([@dominicsayers][])
* [#5388](https://github.com/bbatsov/rubocop/issues/5388): Add new `Style/UnlessMultipleConditions` cop. ([@wata727][])

### Bug fixes

Expand Down
4 changes: 4 additions & 0 deletions config/enabled.yml
Expand Up @@ -1969,6 +1969,10 @@ Style/UnlessElse:
StyleGuide: '#no-else-with-unless'
Enabled: true

Style/UnlessMultipleConditions:
Description: 'Do not use `unless` with multiple conditions.'
Enabled: true

Style/UnneededCapitalW:
Description: 'Checks for %W when interpolation is not needed.'
Enabled: true
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -508,6 +508,7 @@
require_relative 'rubocop/cop/style/trailing_underscore_variable'
require_relative 'rubocop/cop/style/trivial_accessors'
require_relative 'rubocop/cop/style/unless_else'
require_relative 'rubocop/cop/style/unless_multiple_conditions'
require_relative 'rubocop/cop/style/unneeded_capital_w'
require_relative 'rubocop/cop/style/unneeded_interpolation'
require_relative 'rubocop/cop/style/unneeded_percent_q'
Expand Down
41 changes: 41 additions & 0 deletions lib/rubocop/cop/style/unless_multiple_conditions.rb
@@ -0,0 +1,41 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# This cop checks that `unless` is not used with multiple conditions.
# In general, using multiple conditions with `unless` reduces readability.
#
# @example
# # bad
# unless foo && bar
# something
# end
#
# # bad
# unless foo || bar
# something
# end
#
# # good
# if !foo || !bar
# something
# end
#
# # good
# if !foo && !bar
# something
# end
class UnlessMultipleConditions < Cop
MSG = 'Avoid using `unless` with multiple conditions.'.freeze

def on_if(node)
return unless node.unless?

add_offense(node.condition) if node.condition.and_type? ||
node.condition.or_type?
end
end
end
end
end
1 change: 1 addition & 0 deletions manual/cops.md
Expand Up @@ -511,6 +511,7 @@ In the following section you find all available cops:
* [Style/TrailingUnderscoreVariable](cops_style.md#styletrailingunderscorevariable)
* [Style/TrivialAccessors](cops_style.md#styletrivialaccessors)
* [Style/UnlessElse](cops_style.md#styleunlesselse)
* [Style/UnlessMultipleConditions](cops_style.md#styleunlessmultipleconditions)
* [Style/UnneededCapitalW](cops_style.md#styleunneededcapitalw)
* [Style/UnneededInterpolation](cops_style.md#styleunneededinterpolation)
* [Style/UnneededPercentQ](cops_style.md#styleunneededpercentq)
Expand Down
33 changes: 33 additions & 0 deletions manual/cops_style.md
Expand Up @@ -5659,6 +5659,39 @@ end

* [https://github.com/bbatsov/ruby-style-guide#no-else-with-unless](https://github.com/bbatsov/ruby-style-guide#no-else-with-unless)

## Style/UnlessMultipleConditions

Enabled by default | Supports autocorrection
--- | ---
Enabled | No

This cop checks that `unless` is not used with multiple conditions.
In general, using multiple conditions with `unless` reduces readability.

### Examples

```ruby
# bad
unless foo && bar
something
end

# bad
unless foo || bar
something
end

# good
if !foo || !bar
something
end

# good
if !foo && !bar
something
end
```

## Style/UnneededCapitalW

Enabled by default | Supports autocorrection
Expand Down
52 changes: 52 additions & 0 deletions spec/rubocop/cop/style/unless_multiple_conditions_spec.rb
@@ -0,0 +1,52 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Style::UnlessMultipleConditions do
subject(:cop) { described_class.new(config) }

let(:config) { RuboCop::Config.new }

it 'registers an offense when using `unless`' \
'with multiple `and` conditions' do
expect_offense(<<-RUBY.strip_indent)
unless foo && bar
^^^^^^^^^^ Avoid using `unless` with multiple conditions.
something
end
RUBY
end

it 'registers an offense when using `unless` with multiple `or` conditions' do
expect_offense(<<-RUBY.strip_indent)
unless foo || bar
^^^^^^^^^^ Avoid using `unless` with multiple conditions.
something
end
RUBY
end

it 'does not register an offense when using `if`' \
'with multiple `and` conditions' do
expect_no_offenses(<<-RUBY.strip_indent)
if !foo && !bar
something
end
RUBY
end

it 'does not register an offense when using `if`' \
'with multiple `or` conditions' do
expect_no_offenses(<<-RUBY.strip_indent)
if !foo || !bar
something
end
RUBY
end

it 'does not register an offense when using `unless` with single condition' do
expect_no_offenses(<<-RUBY.strip_indent)
unless foo
something
end
RUBY
end
end