Skip to content

[code-review] error_swallow_check delta detection misses new instances when code is prepended — count-based offset assumes new instances always appear after old ones #98

Description

@topcheer

File and Lines

internal/agent/error_swallow_check.go lines 69-83 (checkErrorSwallowing)

Problem

The delta detection logic uses a count-based offset (newInstances[oldCount+i]) that assumes new error-swallowing instances always appear AFTER old instances in the position-ordered list. When new code is added at the BEGINNING of a file, the new instance appears at index 0, and the logic reports an OLD instance as "newly introduced" while missing the actual new instance.

Trigger Scenario

Before edit (oldContent) — 2 existing swallows:

func oldFunc1() error {
    err := doSomething()
    if err != nil {} // OLD instance 1
    return nil
}
func oldFunc2() error {
    err := doSomething()
    if err != nil {} // OLD instance 2
    return nil
}

oldCount = 2

After edit (newContent) — new function added at top:

func newFunc() error {
    err := doSomething()
    if err != nil {} // NEW instance at index 0
    return nil
}
func oldFunc1() error { ... if err != nil {} ... } // OLD at index 1
func oldFunc2() error { ... if err != nil {} ... } // OLD at index 2

Delta logic execution:

  1. oldCount = 2, len(newInstances) = 3
  2. newCount = 3 - 2 = 1
  3. Loop: i=0, accesses newInstances[0+2] = newInstances[2] = OLD instance at line 22
  4. Warning issued for OLD instance — NEW instance at index 0 is MISSED

Expected vs Actual Behavior

  • Expected: New error-swallowing at the beginning of a file should be detected and flagged.
  • Actual: New instance is missed (false negative), old instance is misattributed as "newly introduced".

Root Cause

findErrorSwallows() returns instances ordered by source position (top to bottom). The delta logic assumes indices 0..oldCount-1 = OLD, oldCount..end = NEW. This only works when code is appended to the end. When code is prepended, old instances shift to higher indices and the offset breaks.

Severity

MEDIUM — Only affects delta detection (new files and appends work correctly). Causes both false negatives (missed new bugs) and misattribution (flagging old code as new).

Fix Suggestion

Since error swallowing detection is fast (<1ms), the simplest robust fix is to drop delta optimization entirely and report all instances each time. Alternatively, use position-based matching (store token.Pos for old instances and match against new ones).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions