New cop: use bang modifiers for ActiveRecord methods in transactions

[shanecav84]

Transactions in Rails:

Transactions reset the state of records through a process called a rollback. In Rails, rollbacks are only triggered by an exception. This is a crucial point to understand; I saw several transaction blocks that would never rollback because the containing code could not throw an exception.

  # bad
  ::ActiveRecord::Base.transaction do
    @record = ::Record.find_by(id: 1)
    @record.update(attr: attr)
    @record.save
    @record.destroy
  end

  # good
  ::ActiveRecord::Base.transaction do
    @record = ::Record.find(1) # or ::Record.find_by!(id: 1)
    @record.update!(attr: attr)
    @record.save!
    @record.destroy!
  end

I can put a PR together if I get the go-ahead.

[andyw8]

If a model was using strict validations, save can raise ActiveModel::StrictValidationFailed which would trigger a rollback.

https://guides.rubyonrails.org/active_record_validations.html#strict-validations

[shanecav84]

That's new to me. Rails always has something up its sleeve.

A configurable boolean attribute StrictValidations that would ignore non-bang persistence methods could be useful.

[ibrahima]

Hmm, is there any opposition to adding such a cop for those who are not using strict validations? While that seems like a nice option, it would require more code changes, and in the meantime this cop would probably help a lot of people if it was available.

With strict validations a simple version of this cop would have false positives, but someone using strict validations could just disable it? Not sure what level of static analysis could be done to figure out whether the class in question is using strict validations or not.

[tejasbubane]

Sorry for digging up on this old issue. But I think it still makes sense to have such a cop. I may not want to make all my validations strict, just the ones in an explicit transaction.