Skip to content

Commit

Permalink
Inspect ActiveRecord::Locking::Pessimistic#with_lock as well
Browse files Browse the repository at this point in the history
`ActiveRecord::Locking::Pessimistic#with_lock` implicitly opens a
transaction, so we should inspect this method as well as
`ActiveRecord::Transactions#transaction`.
  • Loading branch information
Hector Zhao committed Jun 2, 2022
1 parent fbba9b2 commit 74c608a
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#710](https://github.com/rubocop/rubocop-rails/pull/710): Rails/TransactionExitStatement - Inspect `ActiveRecord::Locking::Pessimistic#with_lock` too, as `#with_lock` opens a transaction. ([@FunnyHector][https://github.com/FunnyHector])
7 changes: 6 additions & 1 deletion lib/rubocop/cop/rails/transaction_exit_statement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ module Rails
# throw if user.active?
# end
#
# # bad, as `with_lock` implicitly opens a transaction too
# user.with_lock do
# throw if user.active?
# end
#
# # good
# ApplicationRecord.transaction do
# # Rollback
Expand All @@ -47,7 +52,7 @@ class TransactionExitStatement < Base
Exit statement `%<statement>s` is not allowed. Use `raise` (rollback) or `next` (commit).
MSG

RESTRICT_ON_SEND = %i[transaction].freeze
RESTRICT_ON_SEND = %i[transaction with_lock].freeze

def_node_search :exit_statements, <<~PATTERN
({return | break | send nil? :throw} ...)
Expand Down
9 changes: 9 additions & 0 deletions spec/rubocop/cop/rails/transaction_exit_statement_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@
RUBY
end

it 'registers an offense when `return` is used in `with_lock` transactions' do
expect_offense(<<~RUBY)
user.with_lock do
return if user.active?
^^^^^^ Exit statement `return` is not allowed. Use `raise` (rollback) or `next` (commit).
end
RUBY
end

it 'does not register an offense when `next` is used in transactions' do
expect_no_offenses(<<~RUBY)
ApplicationRecord.transaction do
Expand Down

0 comments on commit 74c608a

Please sign in to comment.