Skip to content

Commit 2d3bc1b

Browse files
committed
Add Configurable option for Rails/TransactionExitStatement
1 parent 3bc0e2d commit 2d3bc1b

File tree

5 files changed

+64
-6
lines changed

5 files changed

+64
-6
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1072](https://github.com/rubocop/rubocop-rails/pull/1072): Add `AllowedMethods` and `AllowedPatterns` for `Rails/TransactionExitStatement`. ([@marocchino][])

config/default.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,6 +1083,8 @@ Rails/TransactionExitStatement:
10831083
- https://github.com/rails/rails/commit/15aa4200e083
10841084
Enabled: pending
10851085
VersionAdded: '2.14'
1086+
AllowedMethods: []
1087+
AllowedPatterns: []
10861088

10871089
Rails/UniqBeforePluck:
10881090
Description: 'Prefer the use of uniq or distinct before pluck.'

docs/modules/ROOT/pages/cops_rails.adoc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6190,6 +6190,20 @@ ApplicationRecord.transaction do
61906190
end
61916191
----
61926192

6193+
=== Configurable attributes
6194+
6195+
|===
6196+
| Name | Default value | Configurable values
6197+
6198+
| AllowedMethods
6199+
| `[]`
6200+
| Array
6201+
6202+
| AllowedPatterns
6203+
| `[]`
6204+
| Array
6205+
|===
6206+
61936207
=== References
61946208

61956209
* https://github.com/rails/rails/commit/15aa4200e083

lib/rubocop/cop/rails/transaction_exit_statement.rb

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,28 @@ module Rails
5050
# # Commit
5151
# next if user.active?
5252
# end
53+
#
54+
# @example AllowedMethods: ["custom_transaction"]
55+
# # bad
56+
# CustomModel.custom_transaction do
57+
# return if user.active?
58+
# end
59+
#
60+
# @example AllowedPatterns: ["_transaction$"]
61+
# # bad
62+
# CustomModel.other_transaction do
63+
# return if user.active?
64+
# end
65+
#
5366
class TransactionExitStatement < Base
67+
include AllowedMethods
68+
include AllowedPattern
69+
5470
MSG = <<~MSG.chomp
5571
Exit statement `%<statement>s` is not allowed. Use `raise` (rollback) or `next` (commit).
5672
MSG
5773

58-
RESTRICT_ON_SEND = %i[transaction with_lock].freeze
74+
TRANSACTION_METHODS = %i[transaction with_lock].freeze
5975

6076
def_node_search :exit_statements, <<~PATTERN
6177
({return | break | send nil? :throw} ...)
@@ -69,11 +85,14 @@ class TransactionExitStatement < Base
6985
)
7086
PATTERN
7187

88+
def_node_matcher :transaction_method?, <<~PATTERN
89+
(send _ {#allowed_method_name?} ...)
90+
PATTERN
91+
7292
def on_send(node)
73-
return unless (parent = node.parent)
74-
return unless parent.block_type? && parent.body
93+
return unless in_transaction_block?(node)
7594

76-
exit_statements(parent.body).each do |statement_node|
95+
exit_statements(node.parent.body).each do |statement_node|
7796
next if statement_node.break_type? && nested_block?(statement_node)
7897

7998
statement = statement(statement_node)
@@ -85,6 +104,13 @@ def on_send(node)
85104

86105
private
87106

107+
def in_transaction_block?(node)
108+
return false unless transaction_method?(node)
109+
return false unless (parent = node.parent)
110+
111+
parent.block_type? && parent.body
112+
end
113+
88114
def statement(statement_node)
89115
if statement_node.return_type?
90116
'return'
@@ -96,9 +122,12 @@ def statement(statement_node)
96122
end
97123

98124
def nested_block?(statement_node)
99-
block_node = statement_node.ancestors.find(&:block_type?)
125+
name = statement_node.ancestors.find(&:block_type?).children.first.method_name
126+
!allowed_method_name?(name)
127+
end
100128

101-
RESTRICT_ON_SEND.none? { |name| block_node.method?(name) }
129+
def allowed_method_name?(name)
130+
TRANSACTION_METHODS.include?(name) || allowed_method?(name) || matches_allowed_pattern?(name)
102131
end
103132
end
104133
end

spec/rubocop/cop/rails/transaction_exit_statement_spec.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,4 +108,16 @@
108108

109109
it_behaves_like 'flags transaction exit statements', :transaction
110110
it_behaves_like 'flags transaction exit statements', :with_lock
111+
112+
context 'when `AllowedMethods: [writable_transaction]`' do
113+
let(:cop_config) { { 'AllowedMethods' => %w[writable_transaction] } }
114+
115+
it_behaves_like 'flags transaction exit statements', :writable_transaction
116+
end
117+
118+
context 'when `AllowedPatterns: [transaction]`' do
119+
let(:cop_config) { { 'AllowedPatterns' => %w[transaction] } }
120+
121+
it_behaves_like 'flags transaction exit statements', :foo_transaction
122+
end
111123
end

0 commit comments

Comments
 (0)