Skip to content

Commit

Permalink
Add new Rails/PrivateTransactionOption cop
Browse files Browse the repository at this point in the history
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
wata727 committed Feb 10, 2024
1 parent 8293b58 commit 14f8e13
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 0 deletions.
1 change: 1 addition & 0 deletions changelog/new_add_rails_private_transaction_option_cop.md
@@ -0,0 +1 @@
* [#1236](https://github.com/rubocop/rubocop-rails/pull/1236): Add new `Rails/PrivateTransactionOption` ([@wata727][])
6 changes: 6 additions & 0 deletions config/default.yml
Expand Up @@ -782,6 +782,12 @@ Rails/Present:
# Convert usages of `unless blank?` to `if present?`
UnlessBlank: true

Rails/PrivateTransactionOption:
Description: 'Avoid use of `ActiveRecord::Base.transaction(joinable: _)`'
Enabled: pending
Safe: false
VersionAdded: '<<next>>'

Rails/RakeEnvironment:
Description: 'Include `:environment` as a dependency for all Rake tasks.'
Enabled: true
Expand Down
44 changes: 44 additions & 0 deletions lib/rubocop/cop/rails/private_transaction_option.rb
@@ -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
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails_cops.rb
Expand Up @@ -88,6 +88,7 @@
require_relative 'rails/pluralization_grammar'
require_relative 'rails/presence'
require_relative 'rails/present'
require_relative 'rails/private_transaction_option'
require_relative 'rails/rake_environment'
require_relative 'rails/read_write_attribute'
require_relative 'rails/redundant_active_record_all_method'
Expand Down
16 changes: 16 additions & 0 deletions spec/rubocop/cop/rails/private_transaction_option_spec.rb
@@ -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

0 comments on commit 14f8e13

Please sign in to comment.