Skip to content

Commit

Permalink
[Fix #7886] Look for comments between rescue and end
Browse files Browse the repository at this point in the history
To avoid false negatives for single line rescue statements with a
comment on the next line, we look for comments, not on the line
after rescue, but one lines between rescue and the corresponding
end.
  • Loading branch information
jonas054 authored and bbatsov committed May 17, 2020
1 parent a6bf640 commit 8376d3d
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -14,6 +14,7 @@
* [#7962](https://github.com/rubocop-hq/rubocop/issues/7962): Fix a false positive for `Lint/ParenthesesAsGroupedExpression` when heredoc has a space between the same string as the method name and `(`. ([@koic][])
* [#7967](https://github.com/rubocop-hq/rubocop/pull/7967): `Style/SlicingWithRange` cop now supports any expression as its first index. ([@zverok][])
* [#7972](https://github.com/rubocop-hq/rubocop/issues/7972): Fix an incorrect autocrrect for `Style/HashSyntax` when using a return value uses `return`. ([@koic][])
* [#7886](https://github.com/rubocop-hq/rubocop/issues/7886): Fix a bug in `AllowComments` logic in `Lint/SuppressedException`. ([@jonas054][])

### Changes

Expand Down
13 changes: 12 additions & 1 deletion lib/rubocop/cop/lint/suppressed_exception.rb
Expand Up @@ -69,10 +69,21 @@ class SuppressedException < Cop

def on_resbody(node)
return if node.body
return if cop_config['AllowComments'] && comment_lines?(node)
return if cop_config['AllowComments'] && comment_between_rescue_and_end?(node)

add_offense(node)
end

private

def comment_between_rescue_and_end?(node)
end_line = nil
node.each_ancestor(:kwbegin) do |ancestor|
end_line = ancestor.loc.end.line
break
end
processed_source[node.first_line...end_line].any? { |line| comment_line?(line) }
end
end
end
end
Expand Down
13 changes: 13 additions & 0 deletions spec/rubocop/cop/lint/suppressed_exception_spec.rb
Expand Up @@ -38,5 +38,18 @@
end
RUBY
end

it 'registers an offense for empty rescue on single line with a comment after it' do
expect_offense(<<~RUBY)
RSpec.describe Dummy do
it 'dummy spec' do
# This rescue is here to ensure the test does not fail because of the `raise`
expect { begin subject; rescue ActiveRecord::Rollback; end }.not_to(change(Post, :count))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not suppress exceptions.
# Done
end
end
RUBY
end
end
end

0 comments on commit 8376d3d

Please sign in to comment.