Rails/TransactionExitStatement - Inspect #with_lock method too

[FunnyHector]

ActiveRecord::Locking::Pessimistic#with_lock implicitly opens a transaction, so we should inspect this method as well as ActiveRecord::Transactions#transaction.

---

Before submitting the PR make sure the following are checked:

• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
• Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.
• If this is a new cop, consider making a corresponding update to the Rails Style Guide.

[koic]

Thanks!

[javierjulio]

Having dug around the Rails discussions, I noticed this change rails/rails#39453 was committed which seems to make this cop condition not applicable. I'm using with_lock and doing an early return if a condition isn't meant just like the following rails/rails#29333 (comment) but with the followup I don't think a warning would be made. Are we sure this cop should be reporting for with_lock?

[FunnyHector]

Having dug around the Rails discussions, I noticed this change rails/rails#39453 was committed which seems to make this cop condition not applicable. I'm using with_lock and doing an early return if a condition isn't meant just like the following rails/rails#29333 (comment) but with the followup I don't think a warning would be made. Are we sure this cop should be reporting for with_lock?

@javierjulio I see, I didn't dig into how this deprecation was implemented in Rails, and wasn't aware of "no warning if not written" (rails/rails#39453).

If I'm understanding rails/rails#39453 correctly, neither should with_lock or transaction be flagged by this cop, right? At least not without some updates to detect whether there is any data written.

[javierjulio]

@FunnyHector yes, I think you're right. My main concern was just with with_lock since I have 2 instances in the same class that were reported by Rubocop after upgrading but neither instance performs a write if returned. My understanding is the same as yours, I think this cop wouldn't apply now unless we can detect that data is being written in either with_lock or transaction now.

[FunnyHector]

@javierjulio I'll raise an issue for this. Thanks for this opportunity to learn more about Rails 👍

[javierjulio]

@FunnyHector sounds good, thank you! ❤️