diff --git a/changelog/new_add_rails_private_transaction_option_cop.md b/changelog/new_add_rails_private_transaction_option_cop.md new file mode 100644 index 0000000000..50d5a5f0af --- /dev/null +++ b/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][]) diff --git a/config/default.yml b/config/default.yml index 4ad7a0d053..a674f6a0ce 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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: '<>' + Rails/RakeEnvironment: Description: 'Include `:environment` as a dependency for all Rake tasks.' Enabled: true diff --git a/lib/rubocop/cop/rails/private_transaction_option.rb b/lib/rubocop/cop/rails/private_transaction_option.rb new file mode 100644 index 0000000000..f6e36bd4a0 --- /dev/null +++ b/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 diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index 0d92ff4ca5..e04ae1994a 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -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' diff --git a/spec/rubocop/cop/rails/private_transaction_option_spec.rb b/spec/rubocop/cop/rails/private_transaction_option_spec.rb new file mode 100644 index 0000000000..eb4ef78ad5 --- /dev/null +++ b/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