Skip to content

Commit

Permalink
[Fix rubocop#2903] RedundantReturn minds conditionals
Browse files Browse the repository at this point in the history
`Style/RedundantReturn` looks into conditional branches in order to
find redundant `return`. E.g. the following will be flagged:

    def func
      if x
        return 1
      else
        return 2
      end
    end
  • Loading branch information
lumeet authored and Neodelf committed Oct 15, 2016
1 parent 916be93 commit 64155e8
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
* [#3503](https://github.com/bbatsov/rubocop/issues/3503): Change misleading message of `Style/EmptyLinesAroundAccessModifier`. ([@bquorning][])
* [#3407](https://github.com/bbatsov/rubocop/issues/3407): Turn off autocorrect for unsafe rules by default. ([@ptarjan][])
* [#3521](https://github.com/bbatsov/rubocop/issues/3521): Turn off autocorrect for `Security/JSONLoad` by default. ([@savef][])
* [#2903](https://github.com/bbatsov/rubocop/issues/2903): `Style/RedundantReturn` looks for redundant `return` inside conditional branches. ([@lumeet][])

## 0.43.0 (2016-09-19)

Expand Down
6 changes: 2 additions & 4 deletions lib/rubocop/cop/style/numeric_literal_prefix.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,11 @@ def literal_type(node)
end

def octal_literal_type(literal)
# rubocop:disable Style/GuardClause
if literal =~ OCTAL_ZERO_ONLY_REGEX && octal_zero_only?
return :octal_zero_only
:octal_zero_only
elsif literal =~ OCTAL_REGEX && !octal_zero_only?
return :octal
:octal
end
# rubocop:enable Style/GuardClause
end

def hex_bin_dec_literal_type(literal)
Expand Down
45 changes: 37 additions & 8 deletions lib/rubocop/cop/style/redundant_return.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ module Style
# or a case expression with a default branch.
class RedundantReturn < Cop
include OnMethodDef
include IfNode

MSG = 'Redundant `return` detected.'.freeze

Expand Down Expand Up @@ -71,15 +72,15 @@ def arguments?(args)
def on_method_def(_node, _method_name, _args, body)
return unless body

if body.return_type?
check_return_node(body)
elsif body.begin_type?
expressions = *body
last_expr = expressions.last

return unless last_expr && last_expr.return_type?
check_branch(body)
end

check_return_node(last_expr)
def check_branch(node)
case node.type
when :return then check_return_node(node)
when :case then check_case_node(node)
when :if then check_if_node(node)
when :begin then check_begin_node(node)
end
end

Expand All @@ -89,6 +90,34 @@ def check_return_node(node)

add_offense(node, :keyword)
end

def check_case_node(node)
_cond, *when_nodes, else_node = *node
when_nodes.each { |when_node| check_when_node(when_node) }
check_branch(else_node) if else_node
end

def check_when_node(node)
_cond, body = *node
check_branch(body)
end

def check_if_node(node)
return if modifier_if?(node) || ternary?(node)

_cond, if_node, else_node = if_node_parts(node)
check_branch(if_node)
check_branch(else_node) if else_node
end

def check_begin_node(node)
expressions = *node
last_expr = expressions.last

return unless last_expr && last_expr.return_type?

check_return_node(last_expr)
end
end
end
end
Expand Down
62 changes: 62 additions & 0 deletions spec/rubocop/cop/style/redundant_return_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -221,4 +221,66 @@
expect(new_source).to eq(src)
end
end

context 'when return is inside an if-branch' do
let(:src) do
['def func',
' if x',
' return 1',
' elsif y',
' return 2',
' else',
' return 3',
' end',
'end']
end

it 'registers an offense' do
inspect_source(cop, src)
expect(cop.offenses.size).to eq 3
end

it 'auto-corrects' do
corrected = autocorrect_source(cop, src)
expect(corrected).to eq ['def func',
' if x',
' 1',
' elsif y',
' 2',
' else',
' 3',
' end',
'end'].join("\n")
end
end

context 'when return is inside a when-branch' do
let(:src) do
['def func',
' case x',
' when y then return 1',
' when z then return 2',
' else',
' return 3',
' end',
'end']
end

it 'registers an offense' do
inspect_source(cop, src)
expect(cop.offenses.size).to eq 3
end

it 'auto-corrects' do
corrected = autocorrect_source(cop, src)
expect(corrected).to eq ['def func',
' case x',
' when y then 1',
' when z then 2',
' else',
' 3',
' end',
'end'].join("\n")
end
end
end

0 comments on commit 64155e8

Please sign in to comment.