Merge pull request #1088 from koic/rename_allowed_methods_to_transaction_methods

Rename `AllowedMethods` to `TransactionMethods` for `Rails/TransactionExitStatement`

Author: koic

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 `TransactionMethods` config for `Rails/TransactionExitStatement` to detect custom transaction methods. ([@marocchino][])

changelog/new_add_transaction_methods_to_transaction_exit_statement.md

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

config/default.yml

@@ -13,6 +13,8 @@ module Rails
       # error when rollback is desired, and to use `next` when commit is
       # desired.
       #
+      # If you are defining custom transaction methods, you can configure it with `TransactionMethods`.
+      #
       # @example
       #   # bad
       #   ApplicationRecord.transaction do
@@ -51,27 +53,15 @@ module Rails
       #     next if user.active?
       #   end
       #
-      # @example AllowedMethods: ["custom_transaction"]
+      # @example TransactionMethods: ["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
-
-        TRANSACTION_METHODS = %i[transaction with_lock].freeze
+        MSG = 'Exit statement `%<statement>s` is not allowed. Use `raise` (rollback) or `next` (commit).'
+        BUILT_IN_TRANSACTION_METHODS = %i[transaction with_lock].freeze
 
         def_node_search :exit_statements, <<~PATTERN
           ({return | break | send nil? :throw} ...)
@@ -85,10 +75,6 @@ class TransactionExitStatement < Base
           )
         PATTERN
 
-        def_node_matcher :transaction_method?, <<~PATTERN
-          (send _ {#allowed_method_name?} ...)
-        PATTERN
-
         def on_send(node)
           return unless in_transaction_block?(node)
 
@@ -105,7 +91,7 @@ def on_send(node)
         private
 
         def in_transaction_block?(node)
-          return false unless transaction_method?(node)
+          return false unless transaction_method_name?(node.method_name)
           return false unless (parent = node.parent)
 
           parent.block_type? && parent.body
@@ -123,11 +109,15 @@ def statement(statement_node)
 
         def nested_block?(statement_node)
           name = statement_node.ancestors.find(&:block_type?).children.first.method_name
-          !allowed_method_name?(name)
+          !transaction_method_name?(name)
+        end
+
+        def transaction_method_name?(method_name)
+          BUILT_IN_TRANSACTION_METHODS.include?(method_name) || transaction_method?(method_name)
         end
 
-        def allowed_method_name?(name)
-          TRANSACTION_METHODS.include?(name) || allowed_method?(name) || matches_allowed_pattern?(name)
+        def transaction_method?(method_name)
+          cop_config.fetch('TransactionMethods', []).include?(method_name.to_s)
         end
       end
     end

lib/rubocop/cop/rails/transaction_exit_statement.rb

@@ -109,15 +109,9 @@
   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] } }
+  context 'when `TransactionMethods: [writable_transaction]`' do
+    let(:cop_config) { { 'TransactionMethods' => %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