Merge pull request #673 from Tietew/fix_a_false_negative_for_rails_transaction_exit_statement

Fix a false negative for `Rails/TransactionExitStatement`

Author: koic

changelog/fix_false_negatives_for_rails_transaction_exit_statement.md

@@ -0,0 +1 @@
+* [#673](https://github.com/rubocop/rubocop-rails/pull/673): Fix a false negative for `Rails/TransactionExitStatement` when `return` or `throw` is used in a block in transactions. ([@Tietew][])

lib/rubocop/cop/rails/transaction_exit_statement.rb

@@ -58,7 +58,7 @@ def on_send(node)
           return unless parent.block_type? && parent.body
 
           exit_statements(parent.body).each do |statement_node|
-            next unless statement_node.ancestors.find(&:block_type?).method?(:transaction)
+            next if statement_node.break_type? && nested_block?(statement_node)
 
             statement = statement(statement_node)
             message = format(MSG, statement: statement)
@@ -78,6 +78,10 @@ def statement(statement_node)
             statement_node.method_name
           end
         end
+
+        def nested_block?(statement_node)
+          !statement_node.ancestors.find(&:block_type?).method?(:transaction)
+        end
       end
     end
   end

spec/rubocop/cop/rails/transaction_exit_statement_spec.rb

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