Class: RuboCop::Cop::DevDoc::Rails::SoftFailureInBangMethod

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

Overview

Flag a !-named model method that signals failure softly — errors on the record and a falsy return instead of a raise. Soft failure is save semantics and belongs to a non-bang name.

Rationale

By convention a ! method signals failure by raising, the way save!/create! do (DevDoc/Style/RedundantGuardAfterBang polices call sites under the same assumption). Rails' non-bang save is the opposite contract: validation errors land on the record, the call returns false, and the caller — typically a controller rendering record.errors — checks the result. A domain-action method that wears the ! while failing softly mixes the two contracts: callers cannot tell whether to rescue or to check the return value, call sites accrete rescue blocks and comments explaining what the name should have said, and the naming convention other code relies on stops being trustworthy.

Controller-invoked domain actions (finalize, publish, approve, ...) should normally take the soft shape under a non-bang name: assign attributes, add any precondition errors, return save's boolean — the controller then renders the errors exactly as it does for a plain failed save, with no rescue. Reserve the ! for methods that raise on failure (delegating to save!/update! is the common case), for callers — jobs, migrations, internal invariants — that want the exception.

❌ soft failure under a bang name — the `!` lies
def publish!
if archived?
  errors.add(:base, 'Archived posts cannot be published')
  return
end
self.published_at = Time.current
save
end

❌ return-gated non-bang save under a bang name — same lie
def archive!
self.archived_at = Time.current
transaction do
  next unless save
  items.each { |item| item.update!(archived: true) }
end
end

✔️ same behavior, honest name — `save` semantics, non-bang
def publish
if archived?
  errors.add(:base, 'Archived posts cannot be published')
  return false
end
self.published_at = Time.current
save
end

✔️ raising bang — the `!` is earned
def publish!
raise ArgumentError, 'already archived' if archived?

update!(published_at: Time.current)
end

A bang-named custom validator that adds errors by design should simply drop its ! — validators are the canonical soft-failure shape.

Relationship with DevDoc/Rails/NoManualRecordInvalid

The two cops close the two spellings of the same confusion. This cop catches soft failure hiding under a raising name; that one catches a hand-raised ActiveRecord::RecordInvalid simulating a validation failure. Together they funnel controller-invoked domain actions to the soft non-bang shape, while leaving genuinely raising bang methods untouched.

NOTE: Only a literal raise/fail (Kernel.-qualified included) in the method's own body counts as raise semantics — a nested def is a separate method whose raises and soft signals both stay its own. Delegating to save! on the happy path does NOT excuse an errors.add-and-return on a precondition path — that mixed contract is exactly what this cop exists to catch. Blind spots reviewers must cover: soft failure hidden entirely in a callee (the method returns a callee's false without touching errors), a persistence boolean stashed in a variable before branching, methods defined via define_method/DSL, and failure signalled through a custom exception swallowed internally.

Constant Summary collapse

MSG = 
'Bang method `%<method>s` %<signal>s — a `!` name promises raise-on-failure. ' \
'Drop the `!` (soft `save` semantics), or raise on the failing path.'.freeze
SOFT_PERSISTENCE =

create is omitted: receiverless create in an instance method is not a persistence call on self.

%i[save update destroy].freeze

Instance Method Summary collapse

Instance Method Details

#on_def(node) ⇒ Object Also known as: on_defs 



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

def on_def(node)
  return unless bang_name?(node.method_name)
  return if node.body.nil?
  return if contains_raise?(node)

  signal = soft_signal(node)
  return unless signal

  add_offense(node.loc.name, message: format(MSG, method: node.method_name, signal: signal))
end