rubocop-rails icon indicating copy to clipboard operation
rubocop-rails copied to clipboard

Rails/TransactionExitStatement - not applicable anymore

Open FunnyHector opened this issue 3 years ago • 2 comments

As @javierjulio has noticed in this PR, rails/rails/pull/39453 was committed which seems to make this cop condition not applicable. Rails won't emit such deprecation if there is no write to database, whereas Rails/TransactionExitStatement doesn't consider that.

I'm not sure if there is a way for Rubocop to detect write to database. I don't think so given that Rubocop is just a static analysis tool.

Maybe we should decommission this cop now?

cc: @javierjulio


Expected behavior

User.transaction do
  return if some_crtieria_met?

  # do work that write to database
end

This code won't emit deprecation, because there hasn't been any write into database. This is a new behaviour since rails/rails/pull/39453

Actual behavior

Rails/TransactionExitStatement will flag it.

Steps to reproduce the problem

Just run rubocop with this cop on this code.

RuboCop version

Since the version where Rails/TransactionExitStatement is introduced.

My local version:

➜  bundle exec  rubocop -V
1.25.1 (using Parser 3.1.2.0, rubocop-ast 1.17.0, running on ruby 3.0.4 arm64-darwin21)
  - rubocop-performance 1.13.2
  - rubocop-rails 2.14.2
  - rubocop-rspec 2.10.0

FunnyHector avatar Jun 21 '22 23:06 FunnyHector

Any updates on this one? I propose to remove this cop, if no solution can be found.

FunnyHector avatar Aug 08 '22 03:08 FunnyHector

It's worth mentioning that, with Rails 7.1, the behavior of return inside a transaction depends on config.active_record.commit_transaction_on_non_local_return. It seems that the current plan is for non-local returns to commit transactions on 7.2.

See https://github.com/rails/rails/pull/48600 and related info linked from that PR.

aprescott avatar Jan 08 '24 20:01 aprescott