Skip to content

Commit d45d052

Browse files
koickoic
authored
Merge pull request #1005 from splattael/rails-transaction-exit-statement-with-lock-fix
Flag `break` in `with_lock` for `Rails/TransactionExitStatement`
2 parents facb068 + 673d52e commit d45d052

File tree

3 files changed

+101
-97
lines changed

3 files changed

+101
-97
lines changed

‎changelog/fix_flag_break_in_with_lock_for.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1005](https://github.com/rubocop/rubocop-rails/pull/1005): Flag `break` in `with_lock` for `Rails/TransactionExitStatement`. ([@splattael][])

‎lib/rubocop/cop/rails/transaction_exit_statement.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ module Rails
3434
# throw if user.active?
3535
# end
3636
#
37+
# # bad, as `with_lock` implicitly opens a transaction too
38+
# ApplicationRecord.with_lock do
39+
# break if user.active?
40+
# end
41+
#
3742
# # good
3843
# ApplicationRecord.transaction do
3944
# # Rollback
@@ -91,7 +96,9 @@ def statement(statement_node)
9196
end
9297

9398
def nested_block?(statement_node)
94-
!statement_node.ancestors.find(&:block_type?).method?(:transaction)
99+
block_node = statement_node.ancestors.find(&:block_type?)
100+
101+
RESTRICT_ON_SEND.none? { |name| block_node.method?(name) }
95102
end
96103
end
97104
end

‎spec/rubocop/cop/rails/transaction_exit_statement_spec.rb

Lines changed: 92 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -1,115 +1,111 @@
11
# frozen_string_literal: true
22

33
RSpec.describe RuboCop::Cop::Rails::TransactionExitStatement, :config do
4-
it 'registers an offense when `return` is used in transactions' do
5-
expect_offense(<<~RUBY)
6-
ApplicationRecord.transaction do
7-
return if user.active?
8-
^^^^^^ Exit statement `return` is not allowed. Use `raise` (rollback) or `next` (commit).
9-
end
10-
RUBY
11-
end
4+
shared_examples 'flags transaction exit statements' do |method|
5+
it 'registers an offense when `return` is used in transactions' do
6+
expect_offense(<<~RUBY, method: method)
7+
ApplicationRecord.%{method} do
8+
return if user.active?
9+
^^^^^^ Exit statement `return` is not allowed. Use `raise` (rollback) or `next` (commit).
10+
end
11+
RUBY
12+
end
1213

13-
it 'registers an offense when `break` is used in transactions' do
14-
expect_offense(<<~RUBY)
15-
ApplicationRecord.transaction do
16-
break if user.active?
17-
^^^^^ Exit statement `break` is not allowed. Use `raise` (rollback) or `next` (commit).
18-
end
19-
RUBY
20-
end
14+
it 'registers an offense when `break` is used in transactions' do
15+
expect_offense(<<~RUBY, method: method)
16+
ApplicationRecord.%{method} do
17+
break if user.active?
18+
^^^^^ Exit statement `break` is not allowed. Use `raise` (rollback) or `next` (commit).
19+
end
20+
RUBY
21+
end
2122

22-
it 'registers an offense when `throw` is used in transactions' do
23-
expect_offense(<<~RUBY)
24-
ApplicationRecord.transaction do
25-
throw if user.active?
26-
^^^^^ Exit statement `throw` is not allowed. Use `raise` (rollback) or `next` (commit).
27-
end
28-
RUBY
29-
end
23+
it 'registers an offense when `throw` is used in transactions' do
24+
expect_offense(<<~RUBY, method: method)
25+
ApplicationRecord.%{method} do
26+
throw if user.active?
27+
^^^^^ Exit statement `throw` is not allowed. Use `raise` (rollback) or `next` (commit).
28+
end
29+
RUBY
30+
end
3031

31-
it 'registers an offense when `return` is used in `with_lock` transactions' do
32-
expect_offense(<<~RUBY)
33-
user.with_lock do
34-
return if user.active?
35-
^^^^^^ Exit statement `return` is not allowed. Use `raise` (rollback) or `next` (commit).
36-
end
37-
RUBY
38-
end
32+
it 'does not register an offense when `next` is used in transactions' do
33+
expect_no_offenses(<<~RUBY)
34+
ApplicationRecord.#{method} do
35+
next if user.active?
36+
end
37+
RUBY
38+
end
3939

40-
it 'does not register an offense when `next` is used in transactions' do
41-
expect_no_offenses(<<~RUBY)
42-
ApplicationRecord.transaction do
43-
next if user.active?
44-
end
45-
RUBY
46-
end
40+
it 'does not register an offense when `raise` is used in transactions' do
41+
expect_no_offenses(<<~RUBY)
42+
ApplicationRecord.#{method} do
43+
raise if user.active?
44+
end
45+
RUBY
46+
end
4747

48-
it 'does not register an offense when `raise` is used in transactions' do
49-
expect_no_offenses(<<~RUBY)
50-
ApplicationRecord.transaction do
51-
raise if user.active?
52-
end
53-
RUBY
54-
end
48+
it 'registers an offense when `return` is used in `loop` in transactions' do
49+
expect_offense(<<~RUBY, method: method)
50+
ApplicationRecord.%{method} do
51+
loop do
52+
return if condition
53+
^^^^^^ Exit statement `return` is not allowed. Use `raise` (rollback) or `next` (commit).
54+
end
55+
end
56+
RUBY
57+
end
5558

56-
it 'registers an offense when `return` is used in `loop` in transactions' do
57-
expect_offense(<<~RUBY)
58-
ApplicationRecord.transaction do
59-
loop do
60-
return if condition
61-
^^^^^^ Exit statement `return` is not allowed. Use `raise` (rollback) or `next` (commit).
59+
it 'registers an offense when `throw` is used in `loop` in transactions' do
60+
expect_offense(<<~RUBY, method: method)
61+
ApplicationRecord.%{method} do
62+
loop do
63+
throw if condition
64+
^^^^^ Exit statement `throw` is not allowed. Use `raise` (rollback) or `next` (commit).
65+
end
6266
end
63-
end
64-
RUBY
65-
end
67+
RUBY
68+
end
6669

67-
it 'registers an offense when `throw` is used in `loop` in transactions' do
68-
expect_offense(<<~RUBY)
69-
ApplicationRecord.transaction do
70-
loop do
71-
throw if condition
72-
^^^^^ Exit statement `throw` is not allowed. Use `raise` (rollback) or `next` (commit).
70+
it 'does not register an offense when `break` is used in `loop` in transactions' do
71+
expect_no_offenses(<<~RUBY)
72+
ApplicationRecord.#{method} do
73+
loop do
74+
break if condition
75+
end
7376
end
74-
end
75-
RUBY
76-
end
77+
RUBY
78+
end
7779

78-
it 'does not register an offense when `break` is used in `loop` in transactions' do
79-
expect_no_offenses(<<~RUBY)
80-
ApplicationRecord.transaction do
81-
loop do
82-
break if condition
80+
it 'registers an offense when `return` is used in `rescue`' do
81+
expect_offense(<<~RUBY, method: method)
82+
ApplicationRecord.%{method} do
83+
rescue
84+
return do_something
85+
^^^^^^^^^^^^^^^^^^^ Exit statement `return` is not allowed. Use `raise` (rollback) or `next` (commit).
8386
end
84-
end
85-
RUBY
86-
end
87+
RUBY
88+
end
8789

88-
it 'registers an offense when `return` is used in `rescue`' do
89-
expect_offense(<<~RUBY)
90-
ApplicationRecord.transaction do
91-
rescue
92-
return do_something
93-
^^^^^^^^^^^^^^^^^^^ Exit statement `return` is not allowed. Use `raise` (rollback) or `next` (commit).
94-
end
95-
RUBY
96-
end
90+
it 'registers an offense when `return` is used outside of a `rescue`' do
91+
expect_offense(<<~RUBY, method: method)
92+
ApplicationRecord.%{method} do
93+
return if user.active?
94+
^^^^^^ Exit statement `return` is not allowed. Use `raise` (rollback) or `next` (commit).
95+
rescue
96+
pass
97+
end
98+
RUBY
99+
end
97100

98-
it 'registers an offense when `return` is used outside of a `rescue`' do
99-
expect_offense(<<~RUBY)
100-
ApplicationRecord.transaction do
101-
return if user.active?
102-
^^^^^^ Exit statement `return` is not allowed. Use `raise` (rollback) or `next` (commit).
103-
rescue
104-
pass
105-
end
106-
RUBY
101+
it 'does not register an offense when transaction block is empty' do
102+
expect_no_offenses(<<~RUBY)
103+
ApplicationRecord.#{method} do
104+
end
105+
RUBY
106+
end
107107
end
108108

109-
it 'does not register an offense when transaction block is empty' do
110-
expect_no_offenses(<<~RUBY)
111-
ApplicationRecord.transaction do
112-
end
113-
RUBY
114-
end
109+
it_behaves_like 'flags transaction exit statements', :transaction
110+
it_behaves_like 'flags transaction exit statements', :with_lock
115111
end

0 commit comments

Comments
 (0)