From fc8ae803721b467e821dbc5ddbbe3794c3533835 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 23 Apr 2022 15:12:57 +0900 Subject: [PATCH] Fix a false negative for `Rails/TransactionExitStatement` Follow up https://github.com/rubocop/rubocop-rails/pull/674#issuecomment-1107330877 Revert "[Fix #669] Fix a false positive for `Rails/TransactionExitStatement`" This reverts commit d9ec02de75d95337e7dc2a8f876ba8a4874276c1. So this PR fixes a false negative for `Rails/TransactionExitStatement` when `return` is used in `rescue`. --- ...alse_negative_for_rails_transaction_exit_statement.md | 1 + lib/rubocop/cop/rails/transaction_exit_statement.rb | 8 +------- .../rubocop/cop/rails/transaction_exit_statement_spec.rb | 9 --------- 3 files changed, 2 insertions(+), 16 deletions(-) create mode 100644 changelog/fix_a_false_negative_for_rails_transaction_exit_statement.md diff --git a/changelog/fix_a_false_negative_for_rails_transaction_exit_statement.md b/changelog/fix_a_false_negative_for_rails_transaction_exit_statement.md new file mode 100644 index 0000000000..b9e1bec01b --- /dev/null +++ b/changelog/fix_a_false_negative_for_rails_transaction_exit_statement.md @@ -0,0 +1 @@ +* [#696](https://github.com/rubocop/rubocop-rails/pull/696): Fix a false negative for `Rails/TransactionExitStatement` when `return` is used in `rescue`. ([@koic][]) diff --git a/lib/rubocop/cop/rails/transaction_exit_statement.rb b/lib/rubocop/cop/rails/transaction_exit_statement.rb index 373d9403d9..d2ec6c6ed0 100644 --- a/lib/rubocop/cop/rails/transaction_exit_statement.rb +++ b/lib/rubocop/cop/rails/transaction_exit_statement.rb @@ -58,7 +58,7 @@ def on_send(node) return unless parent.block_type? && parent.body exit_statements(parent.body).each do |statement_node| - next if in_rescue?(statement_node) || nested_block?(statement_node) + next if statement_node.break_type? && nested_block?(statement_node) statement = statement(statement_node) message = format(MSG, statement: statement) @@ -79,13 +79,7 @@ def statement(statement_node) end end - def in_rescue?(statement_node) - statement_node.ancestors.find(&:rescue_type?) - end - def nested_block?(statement_node) - return false unless statement_node.break_type? - !statement_node.ancestors.find(&:block_type?).method?(:transaction) end end diff --git a/spec/rubocop/cop/rails/transaction_exit_statement_spec.rb b/spec/rubocop/cop/rails/transaction_exit_statement_spec.rb index 953e19a2d2..c9212f2d4e 100644 --- a/spec/rubocop/cop/rails/transaction_exit_statement_spec.rb +++ b/spec/rubocop/cop/rails/transaction_exit_statement_spec.rb @@ -76,15 +76,6 @@ RUBY end - it 'does not register an offense when `return` is used in `rescue`' do - expect_no_offenses(<<~RUBY) - ApplicationRecord.transaction do - rescue - return do_something - end - RUBY - end - it 'does not register an offense when transaction block is empty' do expect_no_offenses(<<~RUBY) ApplicationRecord.transaction do