Skip to content

Commit

Permalink
[Fix rubocop#5401] Fix to trigger when begin-end, rescue, and ensure …
Browse files Browse the repository at this point in the history
…blocks present.
  • Loading branch information
asherkach committed Jan 12, 2018
1 parent 87f453d commit eefd205
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
* [#5348](https://github.com/bbatsov/rubocop/issues/5348): Fix false positive for `Style/SafeNavigation` when safe guarding more comparison methods. ([@rrosenblum][])
* [#4889](https://github.com/bbatsov/rubocop/issues/4889): Auto-correcting `Style/SafeNavigation` will add safe navigation to all methods in a method chain. ([@rrosenblum][])
* [#5287](https://github.com/bbatsov/rubocop/issues/5287): Do not register an offense in `Style/SafeNavigation` if there is an unsafe method used in a method chain. ([@rrosenblum][])
* [#5401](https://github.com/bbatsov/rubocop/issues/5401): Fix `Style/RedundantReturn` to trigger when begin-end, rescue, and ensure blocks present. ([@asherkach][])

### Changes

Expand Down
10 changes: 5 additions & 5 deletions lib/rubocop/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,19 @@ def run(args = ARGV)
execute_runners(paths)
rescue RuboCop::ConfigNotFoundError => e
warn e.message
return e.status
e.status
rescue RuboCop::Error => e
warn Rainbow("Error: #{e.message}").red
return STATUS_ERROR
STATUS_ERROR
rescue Finished
return STATUS_SUCCESS
STATUS_SUCCESS
rescue IncorrectCopNameError => e
warn e.message
return STATUS_ERROR
STATUS_ERROR
rescue StandardError, SyntaxError, LoadError => e
warn e.message
warn e.backtrace
return STATUS_ERROR
STATUS_ERROR
end
# rubocop:enable Metrics/MethodLength, Metrics/AbcSize

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/config_loader_resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def transform(config)

def gem_config_path(gem_name, relative_config_path)
spec = Gem::Specification.find_by_name(gem_name)
return File.join(spec.gem_dir, relative_config_path)
File.join(spec.gem_dir, relative_config_path)
rescue Gem::LoadError => e
raise Gem::LoadError,
"Unable to find gem #{gem_name}; is the gem installed? #{e}"
Expand Down
19 changes: 18 additions & 1 deletion lib/rubocop/cop/style/redundant_return.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,20 @@ def arguments?(args)
!args.first.begin_type? || !args.first.children.empty?
end

# rubocop:disable Metrics/CyclomaticComplexity
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)
when :rescue, :resbody
check_rescue_node(node)
when :ensure then check_ensure_node(node)
when :begin, :kwbegin
check_begin_node(node)
end
end
# rubocop:enable Metrics/CyclomaticComplexity

def check_return_node(node)
return if cop_config['AllowMultipleReturnValues'] &&
Expand Down Expand Up @@ -110,6 +116,17 @@ def check_if_node(node)
check_branch(else_node) if else_node
end

def check_rescue_node(node)
node.child_nodes.each do |child_node|
check_branch(child_node)
end
end

def check_ensure_node(node)
rescue_node = node.node_parts[0]
check_branch(rescue_node)
end

def check_begin_node(node)
expressions = *node
last_expr = expressions.last
Expand Down
93 changes: 93 additions & 0 deletions spec/rubocop/cop/style/redundant_return_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
src = <<-RUBY.strip_indent
def func
return something
ensure
2
end
RUBY
inspect_source(src)
Expand Down Expand Up @@ -282,6 +284,97 @@ def func
end
end

context 'when return is inside begin-end body' do
let(:src) do
<<-RUBY.strip_indent
def func
begin
return 1
end
end
RUBY
end

it 'registers an offense' do
expect_offense(<<-RUBY.strip_indent)
def func
begin
return 1
^^^^^^ Redundant `return` detected.
end
end
RUBY
end

it 'auto-corrects' do
corrected = autocorrect_source(src)
expect(corrected).to eq <<-RUBY.strip_indent
def func
begin
1
end
end
RUBY
end
end

context 'when rescue and return blocks present' do
let(:src) do
<<-RUBY.strip_indent
def func
1
2
return 3
rescue SomeException
4
return 5
rescue AnotherException
return 6
ensure
return 7
end
RUBY
end

it 'does register an offense when inside function or rescue block' do
expect_offense(<<-RUBY.strip_indent)
def func
1
2
return 3
^^^^^^ Redundant `return` detected.
rescue SomeException
4
return 5
^^^^^^ Redundant `return` detected.
rescue AnotherException
return 6
^^^^^^ Redundant `return` detected.
ensure
return 7
end
RUBY
end

it 'auto-corrects' do
corrected = autocorrect_source(src)
expect(corrected).to eq <<-RUBY.strip_indent
def func
1
2
3
rescue SomeException
4
5
rescue AnotherException
6
ensure
return 7
end
RUBY
end
end

context 'when return is inside an if-branch' do
let(:src) do
<<-RUBY.strip_indent
Expand Down

0 comments on commit eefd205

Please sign in to comment.