Skip to content

Commit

Permalink
Fix a false negative for Rails/TransactionExitStatement
Browse files Browse the repository at this point in the history
Follow up rubocop#674 (comment)

Revert "[Fix rubocop#669] Fix a false positive for `Rails/TransactionExitStatement`"

This reverts commit d9ec02d and add an example, tweak a spec.

So this PR fixes a false negative for `Rails/TransactionExitStatement` when
`return` is used in `rescue`.
  • Loading branch information
koic committed Apr 27, 2022
1 parent 0f9dfd2 commit 6866080
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#696](https://github.com/rubocop/rubocop-rails/pull/696): Fix a false negative for `Rails/TransactionExitStatement` when `return` is used in `rescue`. ([@koic][])
8 changes: 1 addition & 7 deletions lib/rubocop/cop/rails/transaction_exit_statement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def on_send(node)
return unless parent.block_type? && parent.body

exit_statements(parent.body).each do |statement_node|
next if in_rescue?(statement_node) || nested_block?(statement_node)
next if statement_node.break_type? && nested_block?(statement_node)

statement = statement(statement_node)
message = format(MSG, statement: statement)
Expand All @@ -87,13 +87,7 @@ def statement(statement_node)
end
end

def in_rescue?(statement_node)
statement_node.ancestors.any? { |n| rescue_body_return_node?(n) }
end

def nested_block?(statement_node)
return false unless statement_node.break_type?

!statement_node.ancestors.find(&:block_type?).method?(:transaction)
end
end
Expand Down
5 changes: 3 additions & 2 deletions spec/rubocop/cop/rails/transaction_exit_statement_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,12 @@
RUBY
end

it 'does not register an offense when `return` is used in `rescue`' do
expect_no_offenses(<<~RUBY)
it 'registers an offense when `return` is used in `rescue`' do
expect_offense(<<~RUBY)
ApplicationRecord.transaction do
rescue
return do_something
^^^^^^^^^^^^^^^^^^^ Exit statement `return` is not allowed. Use `raise` (rollback) or `next` (commit).
end
RUBY
end
Expand Down

0 comments on commit 6866080

Please sign in to comment.