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 #674 (comment)

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

This reverts commit d9ec02d and 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 23, 2022
1 parent 6382db9 commit f229951
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 15 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 @@ -58,7 +58,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 @@ -79,13 +79,7 @@ def statement(statement_node)
end
end

def in_rescue?(statement_node)
statement_node.ancestors.find(&:rescue_type?)
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
17 changes: 9 additions & 8 deletions spec/rubocop/cop/rails/transaction_exit_statement_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,21 +66,22 @@
RUBY
end

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

it 'does not register an offense when `return` is used in `rescue`' do
it 'does not register an offense when `break` is used in `loop` in transactions' do
expect_no_offenses(<<~RUBY)
ApplicationRecord.transaction do
rescue
return do_something
loop do
break if condition
end
end
RUBY
end
Expand Down

0 comments on commit f229951

Please sign in to comment.