Class: RuboCop::Cop::DevDoc::Rails::AvoidBypassingValidation

Inherits:
Base
  • Object
show all
Defined in:
lib/rubocop/cop/dev_doc/rails/avoid_bypassing_validation.rb

Overview

Avoid methods that bypass validations and callbacks.

Rationale

Avoid bypassing validation unless absolutely necessary. Methods like update_column, update_all, insert_all, upsert_all, delete_all, and save(validate: false) skip validations and callbacks, which hides data integrity issues rather than surfacing them.

Even in migrations, check the code to see if there is any blatant reason why existing records may be invalid. If there is, fix those records first rather than bypassing validation.

Relationship with Rails/SkipsModelValidations

This cop is a strict SUPERSET of the stock Rails/SkipsModelValidations (it covers every method that cop flags, plus delete_all and save(validate: false)), with per-method actionable messages instead of a generic one. Consumer projects should enable THIS cop and disable Rails/SkipsModelValidations — running both forces every disable comment to name two cops for the same offense. Methods that skip validations BY DESIGN in your domain (e.g. touch, counter-cache maintenance) can be permitted wholesale via AllowedMethods rather than disabled per site.

 Bypasses validation  hides data integrity issues
Faq.where(purpose: nil).update_all(purpose: :intro)

✔️ Runs validation  surfaces problems early
Faq.where(purpose: nil).find_each do |faq|
faq.purpose = :intro
faq.save!
end

Examples:

# bad
Faq.where(purpose: nil).update_all(purpose: :intro)

# bad
user.update_column(:status, 'active')

# bad
user.save(validate: false)

# bad
User.insert_all(rows)

# good
Faq.where(purpose: nil).find_each do |faq|
  faq.purpose = :intro
  faq.save!
end

Constant Summary collapse

MESSAGES =
{
  update_column: 'Avoid `update_column`; it bypasses validations. Use `save!` instead.',
  update_columns: 'Avoid `update_columns`; it bypasses validations. Use `save!` instead.',
  update_attribute: 'Avoid `update_attribute`; it bypasses validations (the singular is not ' \
                    'a validating `update`). Use `update!` instead.',
  update_all: 'Avoid `update_all`; it bypasses validations. Use `save!` in a loop instead.',
  insert: 'Avoid `insert`; it bypasses validations. Use `create!` instead, ' \
          'or `# rubocop:disable` with a reason if a raw insert is intentional.',
  insert!: 'Avoid `insert!`; it bypasses validations (the bang is about duplicate keys, ' \
           'not validity). Use `create!` instead.',
  insert_all: 'Avoid `insert_all`; it bypasses validations. Use `create!` in a loop, ' \
              'or `# rubocop:disable` with a reason if bulk-insert is intentional.',
  upsert: 'Avoid `upsert`; it bypasses validations. Use `create!`/`update!` instead, ' \
          'or `# rubocop:disable` with a reason if a raw upsert is intentional.',
  upsert_all: 'Avoid `upsert_all`; it bypasses validations. Use `create!`/`update!` in a loop, ' \
              'or `# rubocop:disable` with a reason if bulk-upsert is intentional.',
  toggle!: 'Avoid `toggle!`; it saves without validations. Use `toggle(...).save!` instead.',
  increment!: 'Avoid `increment!`; it writes without validations. Use `increment(...).save!` instead.',
  decrement!: 'Avoid `decrement!`; it writes without validations. Use `decrement(...).save!` instead.',
  increment_counter: 'Avoid `increment_counter`; it issues a direct UPDATE that bypasses validations ' \
                     'and callbacks. Use the record with `save!`, or `AllowedMethods` for deliberate ' \
                     'counter-cache maintenance.',
  decrement_counter: 'Avoid `decrement_counter`; it issues a direct UPDATE that bypasses validations ' \
                     'and callbacks. Use the record with `save!`, or `AllowedMethods` for deliberate ' \
                     'counter-cache maintenance.',
  update_counters: 'Avoid `update_counters`; it issues a direct UPDATE that bypasses validations ' \
                   'and callbacks. Use the record with `save!`, or `AllowedMethods` for deliberate ' \
                   'counter-cache maintenance.',
  touch: 'Avoid `touch`; it writes timestamps without validations or save callbacks. Use `save!` ' \
         '(`updated_at` updates automatically), or `AllowedMethods` if bare timestamp bumps are ' \
         'accepted in this project.',
  touch_all: 'Avoid `touch_all`; it issues a bulk UPDATE that bypasses validations and callbacks. ' \
             'Iterate with `save!`, or `# rubocop:disable` with a reason if a bulk timestamp bump ' \
             'is intentional.',
  insert_all!: 'Avoid `insert_all!`; it bypasses validations (the bang is about duplicate keys, ' \
               'not validity). Use `create!` in a loop, or `# rubocop:disable` with a reason if ' \
               'bulk-insert is intentional.',
  delete_all: 'Avoid `delete_all`; it bypasses callbacks. Use `destroy_all` to run callbacks, ' \
              'or `# rubocop:disable` with a reason if bulk-delete is intentional.'
}.freeze
SAVE_MSG =
'Avoid `save(validate: false)`; it bypasses validations. Use `save!` instead.'.freeze
RESTRICT_ON_SEND =
(MESSAGES.keys + %i[save]).freeze
CONST_RECEIVER_ONLY =

insert/insert!/upsert are AR class-level APIs (User.insert(...)), while insert on a non-const receiver is everyday Array#insert — flag only the const-receiver form to avoid false positives.

%i[insert insert! upsert].freeze
NON_CONST_RECEIVER_ONLY =

The inverse for touch: the AR form is instance-level (record.touch), while a const receiver is FileUtils.touch or similar — not an ActiveRecord write at all.

%i[touch].freeze

Instance Method Summary collapse

Instance Method Details

#on_send(node) ⇒ Object



111
112
113
114
115
116
117
118
119
120
121
122
123
# File 'lib/rubocop/cop/dev_doc/rails/avoid_bypassing_validation.rb', line 111

def on_send(node)
  return if allowed_methods.include?(node.method_name.to_s)

  if node.method?(:save)
    return unless save_with_validate_false?(node)

    add_offense(node.loc.selector, message: SAVE_MSG)
  else
    return unless receiver_shape_matches?(node)

    add_offense(node.loc.selector, message: MESSAGES[node.method_name])
  end
end