Merge pull request #1072 from marocchino/add-configurable-option-to-t…

…ransaction-exit-statement

Add Configurable option for `Rails/TransactionExitStatement`

changelog/new_add_allow_methods_and_allow_patterns_to_transaction_exit_statement.md

@@ -0,0 +1 @@
+* [#1072](https://github.com/rubocop/rubocop-rails/pull/1072): Add `AllowedMethods` and `AllowedPatterns` for `Rails/TransactionExitStatement`. ([@marocchino][])

config/default.yml

@@ -1104,6 +1104,8 @@ Rails/TransactionExitStatement:
     - https://github.com/rails/rails/commit/15aa4200e083
   Enabled: pending
   VersionAdded: '2.14'
+  AllowedMethods: []
+  AllowedPatterns: []
 
 Rails/UniqBeforePluck:
   Description: 'Prefer the use of uniq or distinct before pluck.'

docs/modules/ROOT/pages/cops_rails.adoc

@@ -6190,6 +6190,20 @@ ApplicationRecord.transaction do
 end
 ----
 
+=== Configurable attributes
+
+|===
+| Name | Default value | Configurable values
+
+| AllowedMethods
+| `[]`
+| Array
+
+| AllowedPatterns
+| `[]`
+| Array
+|===
+
 === References
 
 * https://github.com/rails/rails/commit/15aa4200e083

lib/rubocop/cop/rails/transaction_exit_statement.rb

@@ -50,12 +50,28 @@ module Rails
       #     # Commit
       #     next if user.active?
       #   end
+      #
+      # @example AllowedMethods: ["custom_transaction"]
+      #   # bad
+      #   CustomModel.custom_transaction do
+      #     return if user.active?
+      #   end
+      #
+      # @example AllowedPatterns: ["_transaction$"]
+      #   # bad
+      #   CustomModel.other_transaction do
+      #     return if user.active?
+      #   end
+      #
       class TransactionExitStatement < Base
+        include AllowedMethods
+        include AllowedPattern
+
         MSG = <<~MSG.chomp
           Exit statement `%<statement>s` is not allowed. Use `raise` (rollback) or `next` (commit).
         MSG
 
-        RESTRICT_ON_SEND = %i[transaction with_lock].freeze
+        TRANSACTION_METHODS = %i[transaction with_lock].freeze
 
         def_node_search :exit_statements, <<~PATTERN
           ({return | break | send nil? :throw} ...)
@@ -69,11 +85,14 @@ class TransactionExitStatement < Base
           )
         PATTERN
 
+        def_node_matcher :transaction_method?, <<~PATTERN
+          (send _ {#allowed_method_name?} ...)
+        PATTERN
+
         def on_send(node)
-          return unless (parent = node.parent)
-          return unless parent.block_type? && parent.body
+          return unless in_transaction_block?(node)
 
-          exit_statements(parent.body).each do |statement_node|
+          exit_statements(node.parent.body).each do |statement_node|
             next if statement_node.break_type? && nested_block?(statement_node)
 
             statement = statement(statement_node)
@@ -85,6 +104,13 @@ def on_send(node)
 
         private
 
+        def in_transaction_block?(node)
+          return false unless transaction_method?(node)
+          return false unless (parent = node.parent)
+
+          parent.block_type? && parent.body
+        end
+
         def statement(statement_node)
           if statement_node.return_type?
             'return'
@@ -96,9 +122,12 @@ def statement(statement_node)
         end
 
         def nested_block?(statement_node)
-          block_node = statement_node.ancestors.find(&:block_type?)
+          name = statement_node.ancestors.find(&:block_type?).children.first.method_name
+          !allowed_method_name?(name)
+        end
 
-          RESTRICT_ON_SEND.none? { |name| block_node.method?(name) }
+        def allowed_method_name?(name)
+          TRANSACTION_METHODS.include?(name) || allowed_method?(name) || matches_allowed_pattern?(name)
         end
       end
     end

spec/rubocop/cop/rails/transaction_exit_statement_spec.rb

@@ -108,4 +108,16 @@
 
   it_behaves_like 'flags transaction exit statements', :transaction
   it_behaves_like 'flags transaction exit statements', :with_lock
+
+  context 'when `AllowedMethods: [writable_transaction]`' do
+    let(:cop_config) { { 'AllowedMethods' => %w[writable_transaction] } }
+
+    it_behaves_like 'flags transaction exit statements', :writable_transaction
+  end
+
+  context 'when `AllowedPatterns: [transaction]`' do
+    let(:cop_config) { { 'AllowedPatterns' => %w[transaction] } }
+
+    it_behaves_like 'flags transaction exit statements', :foo_transaction
+  end
 end