[code-review] nil_deref_check clears ALL nil-risk variables on any error check — multi-error-variable false negatives

[topcheer]

File and Lines

internal/agent/nil_deref_check.go lines 183-206 (clearNilRiskOnErrorCheck)

Problem

When if err != nil is encountered, clearNilRiskOnErrorCheck clears ALL entries in the nilRisk map, regardless of which error variable was actually checked. This causes false negatives when multiple error variables exist and only some are checked.

Trigger Scenario

func example() {
    a, err1 := getA()   // "a" marked as nil-risk (associated with err1)
    b, err2 := getB()   // "b" marked as nil-risk (associated with err2)

    if err1 != nil {     // checks err1, but clears BOTH "a" and "b"
        return
    }

    // err2 was NEVER checked, so b could still be nil
    fmt.Println(b.Field) // FALSE NEGATIVE: b dereferenced but not flagged
}

Execution trace:

1. After a, err1 := getA(): nilRisk = {"a": pos}
2. After b, err2 := getB(): nilRisk = {"a": pos, "b": pos}
3. At if err1 != nil: clearNilRiskOnErrorCheck deletes ALL entries
4. At fmt.Println(b.Field): detectNilDeref finds no entry for "b" → no warning

Expected vs Actual Behavior

• Expected: Only variables associated with err1 should be cleared when err1 is checked. Variables associated with err2 should remain tracked.
• Actual: ALL nil-risk variables are cleared regardless of which error variable is checked.

The comment at lines 200-201 says "we only clear those whose position is before this if-statement, but since ast.Inspect visits in order, all existing entries qualify" — but the real issue is that clearing is not specific to the error variable being checked. The nilRisk map (map[string]int) tracks variable name → position but does not track which error variable each non-error variable is associated with.

Severity

HIGH — Silent false negatives for dangerous nil-pointer dereference patterns. Multi-error-variable patterns are common in non-trivial Go code.

Fix Suggestion

Change nilRisk to track the association between non-error variables and their error variable (e.g., map[string]string for varName → errVarName). Then clearNilRiskOnErrorCheck should only clear entries associated with the specific error variable being checked.

[topcheer]

已修复 (commit 7264faa)。

问题：clearNilRiskOnErrorCheck 在遇到 if err != nil 时会清除 nilRisk map 中的所有条目，无论实际检查的是哪个错误变量。

修复：

• 将 nilRisk 的类型从 map[string]int 改为 map[string]nilRiskEntry，其中 nilRiskEntry 同时记录变量位置和关联的错误变量名
• clearNilRiskOnErrorCheck 现在只清除与被检查错误变量匹配的条目，其他错误变量关联的 nil-risk 变量保持跟踪

测试：新增 TestCheckNilDerefAfterError_MultiErrorVariables 验证：当 err1 被检查但 err2 未检查时，与 err2 关联的变量仍被正确标记为 nil-risk。

--- PASS: TestCheckNilDerefAfterError_MultiErrorVariables (0.00s)

Closes #97