Fix #400 Add Rails/TransactionRequiresNew cop

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Changes
 
+* [#410](https://github.com/rubocop-hq/rubocop-rails/pull/410): Add new`Rails/TransactionRequiresNew` cop. ([@the-wendell][])
 * [#409](https://github.com/rubocop-hq/rubocop-rails/pull/409): Deconstruct "table.column" in `Rails/WhereNot`. ([@mobilutz][])
 
 ## 2.9.1 (2020-12-16)
@@ -329,3 +330,4 @@
 [@Tietew]: https://github.com/Tietew
 [@cilim]: https://github.com/cilim
 [@flanger001]: https://github.com/flanger001
+[@the-wendell]: https://github.com/the-wendell

config/default.yml

@@ -1,5 +1,4 @@
 # Common configuration.
-
 inherit_mode:
   merge:
     - Exclude
@@ -695,6 +694,11 @@ Rails/TimeZone:
     - strict
     - flexible
 
+Rails/TransactionRequiresNew:
+  Description: 'When using a transaction requires that the argument `requires_new: true` is passed.'
+  Enabled: true
+  VersionAdded: '2.10'
+
 Rails/UniqBeforePluck:
   Description: 'Prefer the use of uniq or distinct before pluck.'
   Enabled: true

docs/modules/ROOT/pages/cops.adoc

@@ -93,6 +93,7 @@ based on the https://rails.rubystyle.guide/[Rails Style Guide].
 * xref:cops_rails.adoc#railsskipsmodelvalidations[Rails/SkipsModelValidations]
 * xref:cops_rails.adoc#railssquishedsqlheredocs[Rails/SquishedSQLHeredocs]
 * xref:cops_rails.adoc#railstimezone[Rails/TimeZone]
+* xref:cops_rails.adoc#railstransactionrequiresnew[Rails/TransactionRequiresNew]
 * xref:cops_rails.adoc#railsuniqbeforepluck[Rails/UniqBeforePluck]
 * xref:cops_rails.adoc#railsuniquevalidationwithoutindex[Rails/UniqueValidationWithoutIndex]
 * xref:cops_rails.adoc#railsunknownenv[Rails/UnknownEnv]

docs/modules/ROOT/pages/cops_rails.adoc

@@ -4202,6 +4202,37 @@ Time.at(timestamp).in_time_zone
 * https://rails.rubystyle.guide#time
 * http://danilenko.org/2012/7/6/rails_timezones
 
+== Rails/TransactionRequiresNew
+
+|===
+| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+
+| Enabled
+| Yes
+| Yes
+| 2.10
+| -
+|===
+
+
+
+=== Examples
+
+==== Enabled (default)
+
+[source,ruby]
+----
+# bad
+ActiveRecord::Base.transaction do
+  some_database_stuff
+end
+
+# good
+ActiveRecord::Base.transaction(requires_new: true) do
+  some_database_stuff
+end
+----
+
 == Rails/UniqBeforePluck
 
 |===

lib/rubocop/cop/rails/transaction_requires_new.rb

@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # @example Enabled (default)
+      #   # bad
+      #   ActiveRecord::Base.transaction do
+      #     some_database_stuff
+      #   end
+      #
+      #   # good
+      #   ActiveRecord::Base.transaction(requires_new: true) do
+      #     some_database_stuff
+      #   end
+      #
+      class TransactionRequiresNew < Base
+        extend AutoCorrector
+        MSG = 'Always pass "requires_new: true" to transactions.'
+
+        def_node_matcher :transaction?, <<~PATTERN
+          (block
+            (send _ :transaction ...)
+            (args) {_})
+        PATTERN
+
+        def_node_matcher :requires_new_argument_passed?, <<~PATTERN
+          (block
+            (send _ :transaction (hash ... (pair (sym :requires_new) (true) ) ))
+            (args) {_})
+        PATTERN
+
+        def_node_matcher :arguments_present?, <<~PATTERN
+          (block
+            (send _ :transaction _+ )
+            (args) {_}
+          )
+        PATTERN
+
+        def on_block(node)
+          return unless transaction?(node) && !requires_new_argument_passed?(node)
+
+          if arguments_present?(node)
+            add_offense(node)
+          else
+            add_offense(node) do |corrector|
+              corrector.replace(node, node.source.gsub('.transaction', '.transaction(requires_new: true)'))
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -82,6 +82,7 @@
 require_relative 'rails/skips_model_validations'
 require_relative 'rails/squished_sql_heredocs'
 require_relative 'rails/time_zone'
+require_relative 'rails/transaction_requires_new'
 require_relative 'rails/uniq_before_pluck'
 require_relative 'rails/unique_validation_without_index'
 require_relative 'rails/unknown_env'

spec/rubocop/cop/rails/transaction_requires_new_spec.rb

@@ -0,0 +1,81 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::TransactionRequiresNew do
+  subject(:cop) { described_class.new(config) }
+
+  let(:config) { RuboCop::Config.new }
+
+  context 'when no arguments are passed' do
+    it 'registers an offense when using `ActiveRecord::Base#transaction`' do
+      expect_offense(<<~RUBY)
+        ActiveRecord::Base.transaction do
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Always pass "requires_new: true" to transactions.
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        ActiveRecord::Base.transaction(requires_new: true) do
+        end
+      RUBY
+    end
+
+    it 'registers an offense when using a Model transaction' do
+      expect_offense(<<~RUBY)
+        User.transaction do
+        ^^^^^^^^^^^^^^^^^^^ Always pass "requires_new: true" to transactions.
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        User.transaction(requires_new: true) do
+        end
+      RUBY
+    end
+  end
+
+  context 'when arguments other than `requires_new: true` are passsed' do
+    it 'registers an offense when using `ActiveRecord::Base#transaction`' do
+      expect_offense(<<~RUBY)
+        ActiveRecord::Base.transaction(some_option: false) do
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Always pass "requires_new: true" to transactions.
+        end
+      RUBY
+
+      # Does NOT auto-correct when arugments are present
+      expect_correction(<<~RUBY)
+        ActiveRecord::Base.transaction(some_option: false) do
+        end
+      RUBY
+    end
+
+    it 'registers an offense when using a Model transaction' do
+      expect_offense(<<~RUBY)
+        User.transaction(some_option: true) do
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Always pass "requires_new: true" to transactions.
+        end
+      RUBY
+
+      # does NOT auto-correct when arguments are present
+      expect_correction(<<~RUBY)
+        User.transaction(some_option: true) do
+        end
+      RUBY
+    end
+  end
+
+  context 'when `requires_new: true` is passed' do
+    it 'does not register an offense when using `ActiveRecord::Base#transaction`' do
+      expect_no_offenses(<<~RUBY)
+        ActiveRecord::Base.transaction(requires_new: true) do
+        end
+      RUBY
+    end
+
+    it 'does not register an offense when using a Model transaction' do
+      expect_no_offenses(<<~RUBY)
+        User.transaction(requires_new: true) do
+        end
+      RUBY
+    end
+  end
+end