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

Add new Lint/DuplicateElsifCondition cop #8293

Merged
merged 1 commit into from Jul 9, 2020
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -6,6 +6,7 @@

* [#7333](https://github.com/rubocop-hq/rubocop/issues/7333): Add new `Style/RedundantFileExtensionInRequire` cop. ([@fatkodima][])
* [#8242](https://github.com/rubocop-hq/rubocop/pull/8242): Internal profiling available with `bin/rubocop-profile` and rake tasks. ([@marcandre][])
* [#8293](https://github.com/rubocop-hq/rubocop/pull/8293): Add new `Lint/DuplicateElsifCondition` cop. ([@fatkodima][])
* [#7736](https://github.com/rubocop-hq/rubocop/issues/7736): Add new `Style/CaseLikeIf` cop. ([@fatkodima][])
* [#4286](https://github.com/rubocop-hq/rubocop/issues/4286): Add new `Style/HashAsLastArrayItem` cop. ([@fatkodima][])
* [#8247](https://github.com/rubocop-hq/rubocop/issues/8247): Add new `Style/HashLikeCase` cop. ([@fatkodima][])
Expand Down
5 changes: 5 additions & 0 deletions config/default.yml
Expand Up @@ -1403,6 +1403,11 @@ Lint/DuplicateCaseCondition:
Enabled: true
VersionAdded: '0.45'

Lint/DuplicateElsifCondition:
Description: 'Do not repeat conditions used in if `elsif`.'
Enabled: 'pending'
VersionAdded: '0.88'

Lint/DuplicateHashKey:
Description: 'Check for duplicate keys in hash literals.'
Enabled: true
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -193,6 +193,7 @@ In the following section you find all available cops:
* xref:cops_lint.adoc#lintdeprecatedopensslconstant[Lint/DeprecatedOpenSSLConstant]
* xref:cops_lint.adoc#lintdisjunctiveassignmentinconstructor[Lint/DisjunctiveAssignmentInConstructor]
* xref:cops_lint.adoc#lintduplicatecasecondition[Lint/DuplicateCaseCondition]
* xref:cops_lint.adoc#lintduplicateelsifcondition[Lint/DuplicateElsifCondition]
* xref:cops_lint.adoc#lintduplicatehashkey[Lint/DuplicateHashKey]
* xref:cops_lint.adoc#lintduplicatemethods[Lint/DuplicateMethods]
* xref:cops_lint.adoc#linteachwithobjectargument[Lint/EachWithObjectArgument]
Expand Down
33 changes: 33 additions & 0 deletions docs/modules/ROOT/pages/cops_lint.adoc
Expand Up @@ -609,6 +609,39 @@ when 'second'
end
----

== Lint/DuplicateElsifCondition

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| Yes
| No
| 0.88
| -
|===

This cop checks that there are no repeated conditions used in if 'elsif'.

=== Examples

[source,ruby]
----
# bad
if x == 1
do_something
elsif x == 1
do_something_else
end

# good
if x == 1
do_something
elsif x == 2
do_something_else
end
----

== Lint/DuplicateHashKey

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -251,6 +251,7 @@
require_relative 'rubocop/cop/lint/deprecated_open_ssl_constant'
require_relative 'rubocop/cop/lint/disjunctive_assignment_in_constructor'
require_relative 'rubocop/cop/lint/duplicate_case_condition'
require_relative 'rubocop/cop/lint/duplicate_elsif_condition'
require_relative 'rubocop/cop/lint/duplicate_hash_key'
require_relative 'rubocop/cop/lint/duplicate_methods'
require_relative 'rubocop/cop/lint/each_with_object_argument'
Expand Down
39 changes: 39 additions & 0 deletions lib/rubocop/cop/lint/duplicate_elsif_condition.rb
@@ -0,0 +1,39 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Lint
# This cop checks that there are no repeated conditions used in if 'elsif'.
#
# @example
# # bad
# if x == 1
# do_something
# elsif x == 1
# do_something_else
# end
#
# # good
# if x == 1
# do_something
# elsif x == 2
# do_something_else
# end
#
class DuplicateElsifCondition < Base
MSG = 'Duplicate `elsif` condition detected.'

def on_if(node)
previous = []
while node.if? || node.elsif?
condition = node.condition
add_offense(condition) if previous.include?(condition)
previous << condition
node = node.else_branch
break unless node&.if_type?
end
end
end
end
end
end
54 changes: 54 additions & 0 deletions spec/rubocop/cop/lint/duplicate_elsif_condition_spec.rb
@@ -0,0 +1,54 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Lint::DuplicateElsifCondition do
subject(:cop) { described_class.new }

it 'registers an offense for repeated elsif conditions' do
expect_offense(<<~RUBY)
if x == 1
elsif x == 2
elsif x == 1
^^^^^^ Duplicate `elsif` condition detected.
end
RUBY
end

it 'registers an offense for subsequent repeated elsif conditions' do
expect_offense(<<~RUBY)
if x == 1
elsif x == 2
elsif x == 2
^^^^^^ Duplicate `elsif` condition detected.
end
RUBY
end

it 'registers multiple offenses for multiple repeated elsif conditions' do
expect_offense(<<~RUBY)
if x == 1
elsif x == 2
elsif x == 1
^^^^^^ Duplicate `elsif` condition detected.
elsif x == 2
^^^^^^ Duplicate `elsif` condition detected.
end
RUBY
end

it 'does not register an offense for non-repeated elsif conditions' do
expect_no_offenses(<<~RUBY)
if x == 1
elsif x == 2
else
end
RUBY
end

it 'does not register an offense for partially repeated elsif conditions' do
expect_no_offenses(<<~RUBY)
if x == 1
elsif x == 1 && x == 2
end
RUBY
end
end