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

Support autocorrection for Lint/EnsureReturn #7934

Merged
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 @@ -14,6 +14,7 @@
* [#3696](https://github.com/rubocop-hq/rubocop/issues/3696): Add `AllowComments` option to `Lint/EmptyWhen` cop. ([@koic][])
* [#7910](https://github.com/rubocop-hq/rubocop/pull/7910): Support autocorrection for `Lint/ParenthesesAsGroupedExpression`. ([@koic][])
* [#7925](https://github.com/rubocop-hq/rubocop/pull/7925): Support autocorrection for `Layout/ConditionPosition`. ([@koic][])
* [#7934](https://github.com/rubocop-hq/rubocop/pull/7934): Support autocorrection for `Lint/EnsureReturn`. ([@koic][])

### Bug fixes

Expand Down
1 change: 1 addition & 0 deletions config/default.yml
Expand Up @@ -1425,6 +1425,7 @@ Lint/EnsureReturn:
StyleGuide: '#no-return-ensure'
Enabled: true
VersionAdded: '0.9'
VersionChanged: '0.83'

Lint/ErbNewArguments:
Description: 'Use `:trim_mode` and `:eoutvar` keyword arguments to `ERB.new`.'
Expand Down
19 changes: 18 additions & 1 deletion lib/rubocop/cop/lint/ensure_return.rb
Expand Up @@ -29,6 +29,8 @@ module Lint
# do_something_else
# end
class EnsureReturn < Cop
include RangeHelp

MSG = 'Do not return from an `ensure` block.'

def on_ensure(node)
Expand All @@ -37,7 +39,22 @@ def on_ensure(node)
return unless ensure_body

ensure_body.each_node(:return) do |return_node|
add_offense(return_node)
next if return_node.arguments.size >= 2

add_offense(return_node, location: :keyword)
end
end

def autocorrect(node)
lambda do |corrector|
if node.arguments?
corrector.replace(node, node.source.gsub(/return\s*/, ''))
else
range = range_by_whole_lines(
node.loc.expression, include_final_newline: true
)
corrector.remove(range)
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion manual/cops_lint.md
Expand Up @@ -651,7 +651,7 @@ AllowComments | `true` | Boolean

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | No | 0.9 | -
Enabled | Yes | Yes | 0.9 | 0.83

This cop checks for *return* from an *ensure* block.
Explicit return from an ensure block alters the control flow
Expand Down
41 changes: 40 additions & 1 deletion spec/rubocop/cop/lint/ensure_return_spec.rb
Expand Up @@ -3,7 +3,7 @@
RSpec.describe RuboCop::Cop::Lint::EnsureReturn do
subject(:cop) { described_class.new }

it 'registers an offense for return in ensure' do
it 'registers an offense and corrects for return in ensure' do
expect_offense(<<~RUBY)
begin
something
Expand All @@ -13,6 +13,35 @@
^^^^^^ Do not return from an `ensure` block.
end
RUBY

expect_correction(<<~RUBY)
begin
something
ensure
file.close
end
RUBY
end

it 'registers an offense and corrects for return with argument in ensure' do
expect_offense(<<~RUBY)
begin
foo
ensure
bar
return baz
^^^^^^ Do not return from an `ensure` block.
end
RUBY

expect_correction(<<~RUBY)
begin
foo
ensure
bar
baz
end
RUBY
end

it 'does not register an offense for return outside ensure' do
Expand All @@ -26,6 +55,16 @@
RUBY
end

it "doesn't register an offense when returning multiple values in `ensure`" do
expect_no_offenses(<<~RUBY)
begin
something
ensure
return foo, bar
end
RUBY
end

it 'does not check when ensure block has no body' do
expect_no_offenses(<<~RUBY)
begin
Expand Down