Skip to content

[bot] Fix Rails/SaveBang#196

Closed
6[bot] wants to merge 1 commit intomainfrom
fix/rails-save_bang-23565726858
Closed

[bot] Fix Rails/SaveBang#196
6[bot] wants to merge 1 commit intomainfrom
fix/rails-save_bang-23565726858

Conversation

@6
Copy link
Copy Markdown
Contributor

@6 6 bot commented Mar 25, 2026

Status: Agent is working on this fix...

Cop: Rails/SaveBang | Backend: codex / hard | Model: gpt-5.4 (xhigh) | Mode: fix
Code bugs: 9 | Run: https://github.com/6/nitrocop/actions/runs/23565726858

Refs #172

Task prompt (9881 tokens)

Fix Rails/SaveBang — 0 FP, 22 FN

Instructions

You are fixing ONE cop in nitrocop, a Rust Ruby linter that uses Prism for parsing.

Current state: 74,272 matches, 0 false positives, 22 false negatives.
Focus on: FN (RuboCop flags code nitrocop misses).

⚠ 74,272 existing matches must not regress. Validate with check_cop.py before committing.

Workflow

  1. Read the Pre-diagnostic Results and Corpus FP/FN Examples sections below first
  2. Verify with RuboCop first (for FP fixes): before writing any code, confirm RuboCop's
    behavior on BOTH the specific FP case AND the general pattern:
    echo '<specific FP case>' > /tmp/test.rb && rubocop --only Rails/SaveBang /tmp/test.rb
    echo '<general pattern>' > /tmp/test.rb && rubocop --only Rails/SaveBang /tmp/test.rb
    If RuboCop flags the general pattern, your fix must be narrow enough to not suppress it.
  3. Add a test case FIRST:
    • FN fix: add the missed pattern to tests/fixtures/cops/rails/save_bang/offense.rb with ^ annotation
    • FP fix: add the false-positive pattern to tests/fixtures/cops/rails/save_bang/no_offense.rb
  4. Verify test fails: cargo test --lib -- cop::rails::save_bang
  5. Fix src/cop/rails/save_bang.rs
  6. Verify test passes: cargo test --lib -- cop::rails::save_bang
  7. Validate against corpus (REQUIRED before committing):
    python3 scripts/check_cop.py Rails/SaveBang --rerun --clone --sample 15
    If this reports FP or FN regression, your fix is too broad — narrow it down.
  8. Add a /// doc comment on the cop struct documenting what you found and fixed
  9. Commit only your cop's files

Fixture Format

Mark offenses with ^ markers on the line AFTER the offending source line.
The ^ characters must align with the offending columns. The message format is Rails/SaveBang: <message text>.
See the Current Fixture sections below for real examples from this cop.

If your test passes immediately

If you add a test case and it passes without code changes, the corpus mismatch is
caused by config/context differences, not a detection bug.
Do NOT loop trying to make the test fail. Instead:

  1. Investigate config resolution (Include/Exclude, cop enablement, disable comments)
  2. The fix is likely in src/config/ or the cop's config handling, not detection logic
  3. If you cannot determine the root cause within 5 minutes, document your findings as
    a /// comment on the cop struct and commit

CRITICAL: Avoid regressions in the opposite direction

When fixing FPs, your change MUST NOT suppress legitimate detections. When fixing FNs,
your change MUST NOT flag code that RuboCop accepts. A fix that eliminates a few issues
in one direction but introduces hundreds in the other is a catastrophic regression.

Before exempting a category of patterns, verify with RuboCop that the general case
is still an offense:

rubocop --only Rails/SaveBang /tmp/test.rb

If RuboCop flags the general pattern but not your specific case, the difference is in
a narrow context (e.g., enclosing structure, receiver type, argument count) — your fix
must target that specific context, not the broad category.

Rule of thumb: if your fix adds an early return or continue that skips a whole
node type, operator class, or naming pattern, it's probably too broad. Prefer adding a
condition that matches the SPECIFIC differentiating context.

Rules

  • Only modify src/cop/rails/save_bang.rs and tests/fixtures/cops/rails/save_bang/
  • Run cargo test --lib -- cop::rails::save_bang to verify your fix (do NOT run the full test suite)
  • Run python3 scripts/check_cop.py Rails/SaveBang --rerun --clone --sample 15 before committing to catch regressions
  • Do NOT touch unrelated files
  • Do NOT use git stash

Prism Notes

  • hash splits into HashNode (literal {}) and KeywordHashNode (keyword args foo(a: 1)). If you handle one, check if you need the other.
  • const splits into ConstantReadNode (simple Foo) and ConstantPathNode (qualified Foo::Bar). If you handle one, check if you need the other.

Current Fixture: offense.rb

tests/fixtures/cops/rails/save_bang/offense.rb

def process
  object.save
         ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
  object.save(name: 'Tom', age: 20)
         ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
  object.update(name: 'Tom', age: 20)
         ^^^^^^ Rails/SaveBang: Use `update!` instead of `update` if the return value is not checked.
  save
  ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
  nil
end

# CREATE methods in local variable assignments should be flagged (return value not checked with persisted?)
def create_examples
  x = object.create
             ^^^^^^ Rails/SaveBang: Use `create!` instead of `create` if the return value is not checked. Or check `persisted?` on model returned from `create`.
  y = object.find_or_create_by(name: 'Tom')
             ^^^^^^^^^^^^^^^^^^ Rails/SaveBang: Use `find_or_create_by!` instead of `find_or_create_by` if the return value is not checked. Or check `persisted?` on model returned from `find_or_create_by`.
  nil
end

# CREATE methods in conditions should get conditional message
if object.create
          ^^^^^^ Rails/SaveBang: `create` returns a model which is always truthy.
  puts "created"
end

unless object.create
              ^^^^^^ Rails/SaveBang: `create` returns a model which is always truthy.
  puts "not created"
end

# CREATE method in boolean expression
object.create && notify_user
       ^^^^^^ Rails/SaveBang: `create` returns a model which is always truthy.
object.create || raise("failed")
       ^^^^^^ Rails/SaveBang: `create` returns a model which is always truthy.

# Persist call in body of modifier-if (void context, not the condition)
object.save if false
       ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.

# Persist call in else branch
if condition
  puts "true"
else
  object.save
         ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
end

# Safe navigation calls
object&.save
        ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
object&.update(name: 'Tom')
        ^^^^^^ Rails/SaveBang: Use `update!` instead of `update` if the return value is not checked.

# Variable arguments
object.save(variable)
       ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
object.save(*variable)
       ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
object.save(**variable)
       ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.

# CREATE in case statement condition
case object.create
            ^^^^^^ Rails/SaveBang: `create` returns a model which is always truthy.
when true
  puts "true"
end

# Persist calls inside blocks (void context within block body)
records.map do |r|
  r.update(name: 'Tom')
    ^^^^^^ Rails/SaveBang: Use `update!` instead of `update` if the return value is not checked.
  nil
end

# Persist calls inside nested blocks
items.each do |i|
  i.records.each do |r|
    r.save
      ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
    nil
  end
end

# CREATE in condition inside a block
items.each do |i|
  if User.create
          ^^^^^^ Rails/SaveBang: `create` returns a model which is always truthy.
    puts "yes"
  end
end

# CREATE in assignment inside a block (not followed by persisted?)
items.each do |i|
  x = User.create
           ^^^^^^ Rails/SaveBang: Use `create!` instead of `create` if the return value is not checked. Or check `persisted?` on model returned from `create`.
  nil
end

# Persist call chained as receiver of non-persisted? method (return value not meaningfully checked)
def process
  object.save.to_s
         ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
  object.update(name: 'Tom').inspect
         ^^^^^^ Rails/SaveBang: Use `update!` instead of `update` if the return value is not checked.
  nil
end

# Persist call as receiver of method chain inside argument context
# (outer expression is an argument, but the persist call itself is a receiver — not exempt)
log(object.save.to_s)
           ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
result = object.update(name: 'Tom').inspect
                ^^^^^^ Rails/SaveBang: Use `update!` instead of `update` if the return value is not checked.

# Multi-statement method: last statement is NOT implicit return
# (RuboCop only exempts single-statement method/block bodies)
def multi_stmt_method
  setup_things
  object.save
         ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
end

# Multi-statement block: last statement is NOT implicit return
items.each do |item|
  log(item)
  item.save
       ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
end

# Multi-statement brace block
items.each { |item| log(item); item.update(name: 'Tom') }
                                    ^^^^^^ Rails/SaveBang: Use `update!` instead of `update` if the return value is not checked.

# Persist call in string interpolation (return value not checked)
def process
  "result: #{object.save}"
                    ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
  nil
end

# Persist call in array literal in void context (NOT exempt)
def process_array
  [object.save]
          ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
  nil
end

# Persist call in hash literal in void context (NOT exempt)
def process_hash
  {key: object.save}
               ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
  nil
end

# Singleton method: implicit return does NOT apply (RuboCop only exempts def, not def self.x)
def self.create_default
  create(name: 'test')
  ^^^^^^ Rails/SaveBang: Use `create!` instead of `create` if the return value is not checked.
end

# Block-wrapped create in argument context: create { block } as array element inside method arg
# In RuboCop, `create { }` becomes Block(Send, Args, Body) — argument? on the Send walks
# Send→Block→array, and Block.parent is array, not send_type?, so argument? returns false.
# RuboCop flags this.
def schedule_with_state
  Subscription.new([Item.create { setup }, Subscription.create { cleanup }])
                         ^^^^^^ Rails/SaveBang: Use `create!` instead of `create` if the return value is not checked.
                                                        ^^^^^^ Rails/SaveBang: Use `create!` instead of `create` if the return value is not checked.
end

# CREATE inside || or && (compound_boolean? in RuboCop) — always flagged as conditional
# regardless of enclosing context (assignment, argument, implicit return)
Tag.find_by_name("foo") || Tag.create(name: "foo")
                               ^^^^^^ Rails/SaveBang: `create` returns a model which is always truthy.
Setting.first || Setting.create(name: "bar")
                         ^^^^^^ Rails/SaveBang: `create` returns a model which is always truthy.
x = Foo.first || Foo.create(name: "baz")
                     ^^^^^^ Rails/SaveBang: `create` returns a model which is always truthy.
log(Thing.find || Thing.create(name: "qux"))
                        ^^^^^^ Rails/SaveBang: `create` returns a model which is always truthy.

# rescue modifier breaks implicit return and assignment chains
def teardown
  @post.destroy rescue nil
        ^^^^^^^ Rails/SaveBang: Use `destroy!` instead of `destroy` if the return value is not checked.
end
exception = (around.save rescue $!)
                    ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.

# yield arguments are NOT in argument context (RuboCop's argument? only checks send/csend parents)
items.each {|p| yield(Node.create(p)) }
                           ^^^^^^ Rails/SaveBang: Use `create!` instead of `create` if the return value is not checked.

# Splat breaks argument context chain
execute *builder.create
                 ^^^^^^ Rails/SaveBang: Use `create!` instead of `create` if the return value is not checked.

# yield with modify persist call (yield is NOT argument context per RuboCop)
def process_yield
  yield object.save
               ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
  nil
end

# super with modify persist call (super is NOT argument context per RuboCop)
def process_super
  super(object.save)
               ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
  nil
end

# yield/super even in implicit return position — yield/super break the chain
def process_yield_implicit
  yield object.save
               ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
end
def process_super_implicit
  super(object.save)
               ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.
end

# Create in || inside setter assignment — compound_boolean should flag
self.parent_tag = Tag.find_by_name("x") || Tag.create(name: "x")
                                               ^^^^^^ Rails/SaveBang: `create` returns a model which is always truthy.

# Hash#update on hash literal — flagged as persist method
{ruby_method_type: :class}.update(kwargs)
                           ^^^^^^ Rails/SaveBang: Use `update!` instead of `update` if the return value is not checked.

# Create in || assigned to local (no persisted? check) — compound_boolean
x = AdminSetting.first || AdminSetting.create(last_updated_by: Admin.first)
                                       ^^^^^^ Rails/SaveBang: `create` returns a model which is always truthy.

# Create in || with memoization operator — compound_boolean
@current ||= current_user.presence || User.create(email: "x")
                                           ^^^^^^ Rails/SaveBang: `create` returns a model which is always truthy.

# Create chained: Student.create.lessons — create return value used as receiver chain
Student.create.lessons
        ^^^^^^ Rails/SaveBang: Use `create!` instead of `create` if the return value is not checked.

# Create on LEFT side of `or`/`||` in block implicit return — NOT exempt
# (RuboCop's implicit_return? only exempts the right side via sibling_index math)
items.map { |v| Gem::Version.create(v) or raise }
                             ^^^^^^ Rails/SaveBang: `create` returns a model which is always truthy.

# Create in || inside instance variable assignment — compound_boolean takes priority
# (RuboCop's return_value_assigned? doesn't walk through or nodes)
@directory = connection.directories.get(key) || connection.directories.create(key: key)
                                                                       ^^^^^^ Rails/SaveBang: `create` returns a model which is always truthy.

# CREATE with csend persisted? — RuboCop's call_to_persisted? only matches send_type?, not csend
# So `s&.persisted?` does NOT count as a persisted? check
s = DomainSetup.create(domain: "x")
                ^^^^^^ Rails/SaveBang: Use `create!` instead of `create` if the return value is not checked. Or check `persisted?` on model returned from `create`.
s if s&.persisted?

# CREATE as receiver of call_operator_write — receiver is void context
Student.create.lessons += [science]
        ^^^^^^ Rails/SaveBang: Use `create!` instead of `create` if the return value is not checked.

# CREATE in || inside explicit return — compound_boolean takes priority
# RuboCop's explicit_return? uses assignable_node which doesn't walk through or nodes
def self.get_or_create(**opts)
  record = find_by(opts)
  return record || create(opts)
                   ^^^^^^ Rails/SaveBang: `create` returns a model which is always truthy.
end

# CREATE in local assignment at top-level (no persisted? check)
field = Chargify::SubscriptionMetafield.create name: 'internal info'
                                        ^^^^^^ Rails/SaveBang: Use `create!` instead of `create` if the return value is not checked. Or check `persisted?` on model returned from `create`.

# Parenthesized persist call in argument — parens break argument? check
# RuboCop's argument? checks node.parent.send_type? and begin (parens) is not send_type?
@accounts << (@account.users.create name: "Daniel")
                             ^^^^^^ Rails/SaveBang: Use `create!` instead of `create` if the return value is not checked.

# Parenthesized save in argument — same as above
assert ( cnpj_valido.save ), "CNPJ valido nao foi salvo."
                     ^^^^ Rails/SaveBang: Use `save!` instead of `save` if the return value is not checked.

# update/save inside array inside && — compound_boolean/Condition should not leak through arrays
# RuboCop's in_condition_or_compound_boolean? checks first non-begin ancestor, which is array (not &&)
@database_version ||= (version = raw_connection.oracle_server_version) &&
  [version.major, version.minor, version.update, version.patch]
                                         ^^^^^^ Rails/SaveBang: Use `update!` instead of `update` if the return value is not checked.

# CREATE reassigned to same variable after persisted? check — second create has no persisted? check
# RuboCop's VariableForce tracks each assignment separately; persisted? on an earlier assignment
# does NOT suppress a later re-assignment to the same variable.
def metafield_example
  mf = Chargify::CustomerMetafield.create name: 'test'
  mf.persisted?
  mf = Chargify::SubscriptionMetafield.create name: 'internal info'
                                       ^^^^^^ Rails/SaveBang: Use `create!` instead of `create` if the return value is not checked. Or check `persisted?` on model returned from `create`.
  nil
end

Current Fixture: no_offense.rb

tests/fixtures/cops/rails/save_bang/no_offense.rb

object.save!
object.update!
object.destroy('Tom')
Model.save(1, name: 'Tom')
object.save('Tom')
object.create!

# MODIFY method: return value assigned (exempt)
result = object.save
x = object.update(name: 'Tom')
@saved = object.save
@@flag = object.save
$global = object.save

# MODIFY method: return value in condition (exempt)
if object.save
  puts "saved"
end

# Parenthesized condition
if(object.save)
  puts "saved"
end

unless object.save
  puts "not saved"
end

object.save ? notify : rollback

# MODIFY method: return value in boolean expression (exempt)
object.save && notify_user
object.save || raise("failed")
object.save and log_success
object.save or raise("failed")

# Explicit return
def foo
  return object.save
end

# Implicit return (last expression in method, AllowImplicitReturn: true by default)
def bar
  object.save
end

# Implicit return in block body
items.each do |item|
  item.save
end

# Used as argument
handle_result(object.save)
log(object.update(name: 'Tom'))
handle_save(object.save, true)

# Used in array/hash literal that is assigned (return value used)
result = [object.save, object.update(name: 'Tom')]

# Hash value assigned
result = { success: object.save }

# Return with hash/array (argument context)
return [{ success: object.save }, true]

# Keyword argument
handle_save(success: object.save)

# Explicit early return from block (next)
objects.each do |object|
  next object.save if do_the_save
  do_something_else
end

# Explicit final return from block (next)
objects.each do |object|
  next foo.save
end

# CREATE method with persisted? check immediately
return unless object.create.persisted?

# ENV receiver is always exempt
ENV.update("DISABLE_SPRING" => "1")

# save/update with 2 arguments
Model.save(1, name: 'Tom')

# destroy with arguments is not a persistence method
object.destroy(param)

# CREATE with || in implicit return
def find_or_create(**opts)
  find(**opts) || create(**opts)
end

# CREATE with persisted? check on next line (local variable)
user = User.create
if user.persisted?
  foo
end


# CREATE with persisted? check on next line (instance variable)
@user = User.create
if @user.persisted?
  foo
end

# CREATE with persisted? check directly on result
return unless object.create.persisted?

# CREATE with persisted? in same if condition (parenthesized assignment)
if (user = User.create).persisted?
  foo(user)
end

# CREATE with persisted? after conditional assignment
user ||= User.create
if user.persisted?
  foo
end

# CREATE with persisted? checked non-immediately (skip intervening statements)
# RuboCop uses VariableForce to track all references in scope, not just next stmt.
def create_user
  user = User.create(user_params)
  logger.info("Attempting to create user")
  do_something_else
  if user.persisted?
    redirect_to user
  end
end

# CREATE with persisted? used in same expression (non-adjacent)
def create_and_render
  @user = User.create(user_params)
  render json: @user, status: @user.persisted? ? :created : :unprocessable_entity
end

# CREATE with persisted? in nested expression after other code
def process
  record = Record.find_or_create_by(name: params[:name])
  log_event("Processing record #{record.id}")
  raise ActiveRecord::RecordInvalid unless record.persisted?
end

# Persist call inside brace block — last expression (implicit return)
items.each { |i| i.save }

# Negation: ! / not on persist call (single_negative? in RuboCop — condition context)
!object.save
not object.save

# (Yield/super with persist call moved to offense.rb — RuboCop's
# argument? and implicit_return? don't treat yield/super as exempt)

# CREATE assigned to instance/class/global variable (RuboCop's VariableForce only tracks locals)
@record = object.first_or_create
@@record = User.create(name: 'Joe')
$record = User.create(name: 'Joe')

# CREATE assigned to instance variable without persisted? check (exempt — not tracked by VariableForce)
@user = User.create(name: 'Joe')
foo(@user)

# CREATE in ||= assignment (RuboCop's VariableForce doesn't flag or_asgn create-in-assignment)
label ||= Project.create(title: params[:title])

# CREATE in &&= assignment (same as ||=)
record &&= User.create(name: 'Joe')

# Persist call with block argument: create(hash, &block) has 2 args → not expected_signature
Model.create(name: 'Joe', &block)

# Setter receiver: persist call used as receiver of attribute write (assignment context)
# RuboCop treats setter methods (ending with =) as assignments.
def setter_examples
  create.multipart = true
  update.multipart = true
  save.flag = false
end

# Persist methods with literal arguments are not expected_signature (not AR persist calls)
# RuboCop's expected_signature? rejects literal args that aren't hashes.
create("string")
create("string_#{interpolation}")
create(:"sym_#{interpolation}")
create([{name: 'Joe'}, {name: 'Bob'}])
save([offense])
save(false)
create(true)
update([{id: 1, values: {name: 'Tom'}}])
create(42)
create(/regex/)
first_or_create([{name: 'parrot'}, {name: 'parakeet'}])

# CREATE inside array literal assigned to local variable (RuboCop does not flag)
# RuboCop's VariableForce check_assignment checks `if rhs_node.send_type?` — ArrayNode
# does not match, so create calls inside arrays in local assignments are skipped.
included = [
  Model.create(name: 'foo'),
  Model.create(name: 'bar')
]

matching = [
  Record.create(status: 'active'),
  Record.create(status: 'inactive')
]

# CREATE inside hash literal assigned to local variable (same reason)
lookup = {
  first: Model.create(name: 'first'),
  second: Model.create(name: 'second')
}

# MODIFY in || (return value checked as boolean — exempt)
x = record.save || raise("failed")
y = something || other

# MODIFY in || (return value checked as boolean — exempt)
# (yield and super with modify persist calls moved to offense.rb —
#  RuboCop's argument? doesn't treat yield/super as argument context)

# Block-bearing persist calls in Argument context are exempt.
# In RuboCop's Parser AST, `create { }` becomes Block(Send, Args, Body).
# assignable_node unwraps to block_node, then argument? checks block_node.parent.
# When the block is a direct argument to a method, argument? returns true (exempt).
@calc << InlineBox.create(width: 10, height: 20) {}
Glue.new(InlineBox.create(width: width, height: 10) {})
subscriptions.push(Subscription.create { queues.each {|q| q = [] }})
@buildings << Building.create { |b| b.build_owner(first_name: 'foo') }

# Hash#update and non-AR update with block as argument (exempt — argument context)
test_equal({"a"=>100, "b"=>200, "c"=>300}, h.update(h2) { |k, o, n| o })
expect(atomic.update { |v| v + 1 }).to eq 1001

# Create as keyword arg inside compound boolean — compound_boolean doesn't apply
# because the create's first ancestor is the method call (send), not the or/and node.
# RuboCop's in_condition_or_compound_boolean? checks the FIRST non-begin ancestor.
sash || reload.sash || update(sash: Sash.create)
x = clean_install || cache_version.version != Version.create(Metadata.cache_version)
Person.find_by(handle: id) || Person.new(key: key, pod: Pod.find_or_create_by(url: url))

# Assignment inside assert — persisted? checked on next statement
assert version = parent.versions.create(name: 'test', sharing: 'descendants')
assert version.persisted?

# CREATE in local assignment inside && in if predicate, with persisted? in if-body
# (RuboCop: return_value_assigned? catches lvasgn parent, VariableForce finds persisted?)
def create_in_and_predicate
  if (record = Creator.create(opts)) && record.present?
    increment if record.persisted?
  end
  nil
end

# Multi-write with create and block (return value assigned via masgn)
# RuboCop: assignable_node climbs block → parent is masgn → assignment? true
content, content_type = Formatter.create do |r|
  r.attach name: :data
end

# Multi-write with create inside block inside local variable assignment
# The outer `d3 = c.time_delta do ... end` sets in_local_assignment for d3,
# but the block body's multi-write should NOT inherit that flag.
d3 = c.time_delta do
  content, content_type = Taps::Multipart.create do |r|
    r.attach name: :encoded_data, payload: "data"
  end
end

# MODIFY method in operator-write assignment (+=, &=, etc.)
# RuboCop's return_value_assigned? checks assignable_node.parent.assignment?
# and op_asgn is in SHORTHAND_ASSIGNMENTS, so assignment? returns true.
success &= record.update(v: params[:v])
packet += @cipher.update(data)
total -= record.save

# MODIFY method in if-body structurally identical to if-condition (RuboCop value equality)
# RuboCop's in_condition_or_compound_boolean? uses == (not equal?) to compare nodes.
# When `save` appears both as the if-condition and in the if-body, RuboCop considers
# the body statement as being in condition context (exempted for modify methods).
def copy_from(project)
  Project.transaction do
    if save
      reload
      do_stuff
      save
    else
      false
    end
  end
end

# CREATE method with persisted? on different var in if-condition (RuboCop call_to_persisted? quirk)
# RuboCop's call_to_persisted? replaces node with if-condition when the reference is a
# parenthesized call in an if-body. So `user_export.update_columns(...)` inside
# `if upload.persisted?` triggers suppression even though persisted? is on `upload`, not `user_export`.
def execute
  user_export = UserExport.create(name: "x")
  user_export.id

  File.open("f") do |file|
    upload = UploadCreator.new(file).create_for(1)
    if upload.persisted?
      user_export.update_columns(upload_id: upload.id)
    end
  end
end

# CREATE reassigned to same variable — first assignment with subsequent persisted? is exempt
def metafield_setup
  mf = Chargify::CustomerMetafield.create name: 'test'
  mf.persisted?
end

# Block-bearing create inside hash value inside collect_concat block (Tempfile.create is NOT AR)
# in_transparent_container should NOT leak through block boundaries
def fetch_articles(ftp, files)
  { articles: files.collect_concat do |file|
    Tempfile.create do |temp_file|
      ftp.getbinaryfile(file, temp_file.path)
      JSON.parse(File.read(temp_file.path))[:articles]
    end
  end }
end

Key Source Files

  • Rust implementation: src/cop/rails/save_bang.rs
  • RuboCop Ruby source (ground truth): vendor/rubocop-rails/lib/rubocop/cop/rails/save_bang.rb
  • RuboCop test excerpts: vendor/rubocop-rails/spec/rubocop/cop/rails/save_bang_spec.rb

Read these files before making changes.

Start Here

Use the existing corpus data to focus on the most concentrated regressions first.

Helpful local commands:

  • python3 scripts/investigate_cop.py Rails/SaveBang --repos-only
  • python3 scripts/investigate_cop.py Rails/SaveBang --context
  • python3 scripts/verify_cop_locations.py Rails/SaveBang

Top FN repos:

  • cjstewart88__Tubalr__f6956c8 (10 FN) — example heroku/ruby/1.9.1/gems/rdoc-3.8/lib/rdoc/class_module.rb:60
  • liaoziyang__stackneveroverflow__8f4dce2 (5 FN) — example vendor/bundle/ruby/2.3.0/gems/rdoc-4.3.0/lib/rdoc/class_module.rb:68
  • pitluga__supply_drop__d64c50c (4 FN) — example examples/vendored-puppet/vendor/puppet-2.7.8/lib/puppet/util.rb:83

Representative FN examples:

  • cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.8/lib/rdoc/class_module.rb:60 — Use update! instead of update if the return value is not checked.
  • cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.8/lib/rdoc/class_module.rb:61 — Use update! instead of update if the return value is not checked.
  • cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.8/lib/rdoc/class_module.rb:70 — Use update! instead of update if the return value is not checked.

Pre-diagnostic Results

Diagnosis Summary

Each example was tested by running nitrocop on the extracted source in isolation
with --force-default-config to determine if the issue is a code bug or config issue.
Note: source context is truncated and may not parse perfectly. If a diagnosis
seems wrong (e.g., your test passes immediately for a 'CODE BUG'), treat it as
a config/context issue instead.

  • FN: 15 code bug(s), 0 config/context issue(s)

FN #1: cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.8/lib/rdoc/class_module.rb:60

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Message: Use update!instead ofupdate if the return value is not checked.

Ready-made test snippet (add to offense.rb, adjust ^ count):

    klass.methods_hash.update mod.methods_hash
^ Rails/SaveBang: Use `update!` instead of `update` if the return value is not checked.

Full source context:

    klass.attributes.concat mod.attributes
    klass.method_list.concat mod.method_list
    klass.aliases.concat mod.aliases
    klass.external_aliases.concat mod.external_aliases
    klass.constants.concat mod.constants
    klass.includes.concat mod.includes

    klass.methods_hash.update mod.methods_hash
    klass.constants_hash.update mod.constants_hash

    klass.current_section = mod.current_section
    klass.in_files.concat mod.in_files
    klass.sections.concat mod.sections
    klass.unmatched_alias_lists = mod.unmatched_alias_lists
    klass.current_section = mod.current_section

FN #2: cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.8/lib/rdoc/class_module.rb:61

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Message: Use update!instead ofupdate if the return value is not checked.

Ready-made test snippet (add to offense.rb, adjust ^ count):

    klass.constants_hash.update mod.constants_hash
^ Rails/SaveBang: Use `update!` instead of `update` if the return value is not checked.

Full source context:

    klass.method_list.concat mod.method_list
    klass.aliases.concat mod.aliases
    klass.external_aliases.concat mod.external_aliases
    klass.constants.concat mod.constants
    klass.includes.concat mod.includes

    klass.methods_hash.update mod.methods_hash
    klass.constants_hash.update mod.constants_hash

    klass.current_section = mod.current_section
    klass.in_files.concat mod.in_files
    klass.sections.concat mod.sections
    klass.unmatched_alias_lists = mod.unmatched_alias_lists
    klass.current_section = mod.current_section
    klass.visibility = mod.visibility

FN #3: cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.8/lib/rdoc/class_module.rb:70

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Message: Use update!instead ofupdate if the return value is not checked.

Ready-made test snippet (add to offense.rb, adjust ^ count):

    klass.classes_hash.update mod.classes_hash
^ Rails/SaveBang: Use `update!` instead of `update` if the return value is not checked.

Full source context:

    klass.current_section = mod.current_section
    klass.in_files.concat mod.in_files
    klass.sections.concat mod.sections
    klass.unmatched_alias_lists = mod.unmatched_alias_lists
    klass.current_section = mod.current_section
    klass.visibility = mod.visibility

    klass.classes_hash.update mod.classes_hash
    klass.modules_hash.update mod.modules_hash
    klass.metadata.update mod.metadata

    klass.document_self = mod.received_nodoc ? nil : mod.document_self
    klass.document_children = mod.document_children
    klass.force_documentation = mod.force_documentation
    klass.done_documenting = mod.done_documenting

FN #4: cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.8/lib/rdoc/class_module.rb:71

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Message: Use update!instead ofupdate if the return value is not checked.

Ready-made test snippet (add to offense.rb, adjust ^ count):

    klass.modules_hash.update mod.modules_hash
^ Rails/SaveBang: Use `update!` instead of `update` if the return value is not checked.

Full source context:

    klass.in_files.concat mod.in_files
    klass.sections.concat mod.sections
    klass.unmatched_alias_lists = mod.unmatched_alias_lists
    klass.current_section = mod.current_section
    klass.visibility = mod.visibility

    klass.classes_hash.update mod.classes_hash
    klass.modules_hash.update mod.modules_hash
    klass.metadata.update mod.metadata

    klass.document_self = mod.received_nodoc ? nil : mod.document_self
    klass.document_children = mod.document_children
    klass.force_documentation = mod.force_documentation
    klass.done_documenting = mod.done_documenting

FN #5: cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.8/lib/rdoc/class_module.rb:72

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Message: Use update!instead ofupdate if the return value is not checked.

Ready-made test snippet (add to offense.rb, adjust ^ count):

    klass.metadata.update mod.metadata
^ Rails/SaveBang: Use `update!` instead of `update` if the return value is not checked.

Full source context:

    klass.sections.concat mod.sections
    klass.unmatched_alias_lists = mod.unmatched_alias_lists
    klass.current_section = mod.current_section
    klass.visibility = mod.visibility

    klass.classes_hash.update mod.classes_hash
    klass.modules_hash.update mod.modules_hash
    klass.metadata.update mod.metadata

    klass.document_self = mod.received_nodoc ? nil : mod.document_self
    klass.document_children = mod.document_children
    klass.force_documentation = mod.force_documentation
    klass.done_documenting = mod.done_documenting

    # update the parent of all children

FN #6: cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.9.4/lib/rdoc/class_module.rb:60

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Message: Use update!instead ofupdate if the return value is not checked.

Ready-made test snippet (add to offense.rb, adjust ^ count):

    klass.methods_hash.update mod.methods_hash
^ Rails/SaveBang: Use `update!` instead of `update` if the return value is not checked.

Full source context:

    klass.attributes.concat mod.attributes
    klass.method_list.concat mod.method_list
    klass.aliases.concat mod.aliases
    klass.external_aliases.concat mod.external_aliases
    klass.constants.concat mod.constants
    klass.includes.concat mod.includes

    klass.methods_hash.update mod.methods_hash
    klass.constants_hash.update mod.constants_hash

    klass.current_section = mod.current_section
    klass.in_files.concat mod.in_files
    klass.sections.concat mod.sections
    klass.unmatched_alias_lists = mod.unmatched_alias_lists
    klass.current_section = mod.current_section

FN #7: cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.9.4/lib/rdoc/class_module.rb:61

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Message: Use update!instead ofupdate if the return value is not checked.

Ready-made test snippet (add to offense.rb, adjust ^ count):

    klass.constants_hash.update mod.constants_hash
^ Rails/SaveBang: Use `update!` instead of `update` if the return value is not checked.

Full source context:

    klass.method_list.concat mod.method_list
    klass.aliases.concat mod.aliases
    klass.external_aliases.concat mod.external_aliases
    klass.constants.concat mod.constants
    klass.includes.concat mod.includes

    klass.methods_hash.update mod.methods_hash
    klass.constants_hash.update mod.constants_hash

    klass.current_section = mod.current_section
    klass.in_files.concat mod.in_files
    klass.sections.concat mod.sections
    klass.unmatched_alias_lists = mod.unmatched_alias_lists
    klass.current_section = mod.current_section
    klass.visibility = mod.visibility

FN #8: cjstewart88__Tubalr__f6956c8: heroku/ruby/1.9.1/gems/rdoc-3.9.4/lib/rdoc/class_module.rb:70

NOT DETECTED — CODE BUG
The cop fails to detect this pattern. Fix the detection logic.

Message: Use update!instead ofupdate if the return value is not checked.

Ready-made test snippet (add to offense.rb, adjust ^ count):

    klass.classes_hash.update mod.classes_hash
^ Rails/SaveBang: Use `update!` instead of `update` if the return value is not checked.

Full source context:

    klass.current_section = mod.current_section
    klass.in_files.concat mod.in_files
    klass.sections.concat mod.sections
    klass.unmatched_alias_lists = mod.unmatched_alias_lists
    klass.current_section = mod.current_section
    klass.visibility = mod.visibility

    klass.classes_hash.update mod.classes_hash
    klass.modules_hash.update mod.modules_hash
    klass.metadata.update mod.metadata

    klass.document_self = mod.received_nodoc ? nil : mod.document_self
    klass.document_children = mod.document_children
    klass.force_documentation = mod.force_documentation
    klass.done_documenting = mod.done_documenting

Omitted 7 additional diagnosed FN example(s) for brevity.

@6
Copy link
Copy Markdown
Contributor Author

6 bot commented Mar 25, 2026

Agent failed. See run: https://github.com/6/nitrocop/actions/runs/23565726858

@6 6 bot closed this Mar 25, 2026
@6 6 bot deleted the fix/rails-save_bang-23565726858 branch March 25, 2026 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants