Skip to content

Commit

Permalink
Add Configurable option for Rails/TransactionExitStatement
Browse files Browse the repository at this point in the history
  • Loading branch information
marocchino committed Aug 22, 2023
1 parent 3bc0e2d commit 9616883
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#1072](https://github.com/rubocop/rubocop-rails/pull/1072): Add `AllowedMethods` and `AllowedPatterns` for `Rails/TransactionExitStatement`. ([@marocchino][])
2 changes: 2 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1083,6 +1083,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.'
Expand Down
14 changes: 14 additions & 0 deletions docs/modules/ROOT/pages/cops_rails.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 35 additions & 6 deletions lib/rubocop/cop/rails/transaction_exit_statement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,28 @@ module Rails
# # Commit
# next if user.active?
# end
#
# @example AllowedMethods: ["custom_transaction"]
# # bad
# ApplicationRecord.custom_transaction do
# return if user.active?
# end
#
# @example AllowedPatterns: ["_transaction$"]
# # bad
# ApplicationRecord.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} ...)
Expand All @@ -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)
Expand All @@ -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'
Expand All @@ -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
Expand Down
12 changes: 12 additions & 0 deletions spec/rubocop/cop/rails/transaction_exit_statement_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 9616883

Please sign in to comment.