-
-
Notifications
You must be signed in to change notification settings - Fork 270
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add new
Rails/PrivateTransactionOption
cop
This PR adds a new cop called `Rails/PrivateTransactionOption`. This cop checks whether `ActiveRecord::Base.transaction(joinable: _)` is used. The `joinable` option is a private API and is not intended to be called from outside Active Record core. rails/rails#39912 (comment) rails/rails#46182 (comment) Passing `joinable: false` may cause unexpected behavior such as the `after_commit` callback not firing at the appropriate time.
- Loading branch information
Showing
5 changed files
with
68 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
* [#1236](https://github.com/rubocop/rubocop-rails/pull/1236): Add new `Rails/PrivateTransactionOption`. ([@wata727][]) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
# frozen_string_literal: true | ||
|
||
module RuboCop | ||
module Cop | ||
module Rails | ||
# Checks whether `ActiveRecord::Base.transaction(joinable: _)` is used. | ||
# | ||
# The `joinable` option is a private API and is not intended to be called | ||
# from outside Active Record core. | ||
# https://github.com/rails/rails/issues/39912#issuecomment-665483779 | ||
# https://github.com/rails/rails/issues/46182#issuecomment-1265966330 | ||
# | ||
# Passing `joinable: false` may cause unexpected behavior such as the | ||
# `after_commit` callback not firing at the appropriate time. | ||
# | ||
# @safety | ||
# This Cop is unsafe because it cannot accurately identify | ||
# the `ActiveRecord::Base.transaction` method call. | ||
# | ||
# @example | ||
# # bad | ||
# ActiveRecord::Base.transaction(requires_new: true, joinable: false) | ||
# | ||
# # good | ||
# ActiveRecord::Base.transaction(requires_new: true) | ||
# | ||
class PrivateTransactionOption < Base | ||
MSG = 'Do not use `ActiveRecord::Base.transaction(joinable: _)`.' | ||
RESTRICT_ON_SEND = %i[transaction].freeze | ||
|
||
# @!method match_transaction_with_joinable(node) | ||
def_node_matcher :match_transaction_with_joinable, <<~PATTERN | ||
(send _ :transaction (hash <$(pair (sym :joinable) {true false}) ...>)) | ||
PATTERN | ||
|
||
def on_send(node) | ||
match_transaction_with_joinable(node) do |option_node| | ||
add_offense(option_node) | ||
end | ||
end | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
# frozen_string_literal: true | ||
|
||
RSpec.describe RuboCop::Cop::Rails::PrivateTransactionOption, :config do | ||
it 'registers an offense when using `joinable: false`' do | ||
expect_offense(<<~RUBY) | ||
ActiveRecord::Base.transaction(requires_new: true, joinable: false) | ||
^^^^^^^^^^^^^^^ Do not use `ActiveRecord::Base.transaction(joinable: _)`. | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using only `requires_new: true`' do | ||
expect_no_offenses(<<~RUBY) | ||
ActiveRecord::Base.transaction(requires_new: true) | ||
RUBY | ||
end | ||
end |