Skip to content

Commit

Permalink
Add new Lint/DuplicateBranch cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima authored and bbatsov committed Nov 6, 2020
1 parent cf96730 commit b9b5853
Show file tree
Hide file tree
Showing 12 changed files with 429 additions and 17 deletions.
1 change: 1 addition & 0 deletions changelog/new_duplicate_branch_cop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#8404](https://github.com/rubocop-hq/rubocop/pull/8404): Add new `Lint/DuplicateBranch` cop. ([@fatkodima][])
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1449,6 +1449,11 @@ Lint/DisjunctiveAssignmentInConstructor:
VersionAdded: '0.62'
VersionChanged: '0.88'

Lint/DuplicateBranch:
Description: Checks that there are no repeated bodies within `if/unless`, `case-when` and `rescue` constructs.
Enabled: pending
VersionAdded: '1.3'

Lint/DuplicateCaseCondition:
Description: 'Do not repeat values in case conditionals.'
Enabled: true
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ In the following section you find all available cops:
* xref:cops_lint.adoc#lintdeprecatedclassmethods[Lint/DeprecatedClassMethods]
* xref:cops_lint.adoc#lintdeprecatedopensslconstant[Lint/DeprecatedOpenSSLConstant]
* xref:cops_lint.adoc#lintdisjunctiveassignmentinconstructor[Lint/DisjunctiveAssignmentInConstructor]
* xref:cops_lint.adoc#lintduplicatebranch[Lint/DuplicateBranch]
* xref:cops_lint.adoc#lintduplicatecasecondition[Lint/DuplicateCaseCondition]
* xref:cops_lint.adoc#lintduplicateelsifcondition[Lint/DuplicateElsifCondition]
* xref:cops_lint.adoc#lintduplicatehashkey[Lint/DuplicateHashKey]
Expand Down
69 changes: 69 additions & 0 deletions docs/modules/ROOT/pages/cops_lint.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,75 @@ def initialize
end
----

== Lint/DuplicateBranch

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

| Pending
| Yes
| No
| 1.3
| -
|===

This cop checks that there are no repeated bodies
within `if/unless`, `case-when` and `rescue` constructs.

=== Examples

[source,ruby]
----
# bad
if foo
do_foo
do_something_else
elsif bar
do_foo
do_something_else
end
# good
if foo || bar
do_foo
do_something_else
end
# bad
case x
when foo
do_foo
when bar
do_foo
else
do_something_else
end
# good
case x
when foo, bar
do_foo
else
do_something_else
end
# bad
begin
do_something
rescue FooError
handle_error
rescue BarError
handle_error
end
# good
begin
do_something
rescue FooError, BarError
handle_error
end
----

== Lint/DuplicateCaseCondition

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@
require_relative 'rubocop/cop/lint/deprecated_class_methods'
require_relative 'rubocop/cop/lint/deprecated_open_ssl_constant'
require_relative 'rubocop/cop/lint/disjunctive_assignment_in_constructor'
require_relative 'rubocop/cop/lint/duplicate_branch'
require_relative 'rubocop/cop/lint/duplicate_case_condition'
require_relative 'rubocop/cop/lint/duplicate_elsif_condition'
require_relative 'rubocop/cop/lint/duplicate_hash_key'
Expand Down
7 changes: 3 additions & 4 deletions lib/rubocop/cop/layout/block_alignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,9 @@ def loc_to_source_line_column(loc)
end

def alt_start_msg(start_loc, source_line_column)
if style != :either
''
elsif start_loc.line == source_line_column[:line] &&
start_loc.column == source_line_column[:column]
if style != :either ||
(start_loc.line == source_line_column[:line] &&
start_loc.column == source_line_column[:column])
''
else
" or #{format_source_line_column(source_line_column)}"
Expand Down
93 changes: 93 additions & 0 deletions lib/rubocop/cop/lint/duplicate_branch.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Lint
# This cop checks that there are no repeated bodies
# within `if/unless`, `case-when` and `rescue` constructs.
#
# @example
# # bad
# if foo
# do_foo
# do_something_else
# elsif bar
# do_foo
# do_something_else
# end
#
# # good
# if foo || bar
# do_foo
# do_something_else
# end
#
# # bad
# case x
# when foo
# do_foo
# when bar
# do_foo
# else
# do_something_else
# end
#
# # good
# case x
# when foo, bar
# do_foo
# else
# do_something_else
# end
#
# # bad
# begin
# do_something
# rescue FooError
# handle_error
# rescue BarError
# handle_error
# end
#
# # good
# begin
# do_something
# rescue FooError, BarError
# handle_error
# end
#
class DuplicateBranch < Base
include RescueNode

MSG = 'Duplicate branch body detected.'

def on_branching_statement(node)
branches = node.branches.compact
branches.each_with_object(Set.new) do |branch, previous|
add_offense(offense_range(branch)) unless previous.add?(branch)
end
end
alias on_if on_branching_statement
alias on_case on_branching_statement
alias on_rescue on_branching_statement

private

def offense_range(duplicate_branch)
parent = duplicate_branch.parent

if parent.respond_to?(:else_branch) &&
parent.else_branch.equal?(duplicate_branch)
if parent.if_type? && parent.ternary?
duplicate_branch.source_range
else
parent.loc.else
end
else
parent.source_range
end
end
end
end
end
end
9 changes: 4 additions & 5 deletions lib/rubocop/cop/lint/shadowed_exception.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,10 @@ def sorted?(rescued_groups)
rescued_groups.each_cons(2).all? do |x, y|
if x.include?(Exception)
false
elsif y.include?(Exception)
true
elsif x.none? || y.none?
# consider sorted if a group is empty or only contains
# `nil`s
elsif y.include?(Exception) ||
# consider sorted if a group is empty or only contains
# `nil`s
x.none? || y.none?
true
else
(x <=> y || 0) <= 0
Expand Down
6 changes: 2 additions & 4 deletions lib/rubocop/cop/lint/useless_method_definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,9 @@ def optional_args?(node)
end

def delegating?(node, def_node)
if node.nil?
false
elsif node.zsuper_type?
if node&.zsuper_type?
true
elsif node.super_type?
elsif node&.super_type?
node.arguments.map(&:source) == def_node.arguments.map(&:source)
else
false
Expand Down
4 changes: 1 addition & 3 deletions lib/rubocop/cop/style/and_or.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,7 @@ def process_logical_operator(node)
node.each_child_node do |expr|
if expr.send_type?
correct_send(expr, corrector)
elsif expr.return_type?
correct_other(expr, corrector)
elsif expr.assignment?
elsif expr.return_type? || expr.assignment?
correct_other(expr, corrector)
end
end
Expand Down
2 changes: 1 addition & 1 deletion rubocop.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ Gem::Specification.new do |s|
s.add_runtime_dependency('rainbow', '>= 2.2.2', '< 4.0')
s.add_runtime_dependency('regexp_parser', '>= 1.8')
s.add_runtime_dependency('rexml')
s.add_runtime_dependency('rubocop-ast', '>= 1.0.1')
s.add_runtime_dependency('rubocop-ast', '>= 1.1.1')
s.add_runtime_dependency('ruby-progressbar', '~> 1.7')
s.add_runtime_dependency('unicode-display_width', '>= 1.4.0', '< 2.0')

Expand Down

0 comments on commit b9b5853

Please sign in to comment.