[business-logic] unchecked_assert_check.go regression (#157 fix): exprText has no TypeAssertExpr case — every assertion fingerprints to empty string, detector permanently silent on files with any pre-existing assertion

[topcheer]

Regression introduced by fix for #157 (commit 14a4c14) — detector permanently silent

Summary

The #157 fix replaced token.Position.String() fingerprints with "assertion expression text" via a new assertExprAt() helper. But assertExprAt passes the whole *ast.TypeAssertExpr to exprText() — a helper whose type switch only handles Ident/SelectorExpr/ParenExpr/BasicLit and returns "" for everything else. Every unchecked type assertion now has the same fingerprint: the empty string.

Consequence: once a file contains even one pre-existing unchecked assertion, oldSet contains "" and every assertion in the new content matches it — newOnly is always empty and the detector never fires again for that file. The #157 defect (annoying false positives on line shifts) has been replaced by a complete false negative: the detector is permanently silent on any file that already had an assertion — exactly the files most likely to receive new assertions.

This is the most severe kind of regression: the check silently reports nothing, and the existing tests pass by relying on the buggy behavior.

Logic chain (independently re-verified + executable reproduction)

1. internal/agent/unchecked_assert_check.go L184-200 (added by 14a4c14): assertExprAt locates the *ast.TypeAssertExpr at pos and returns exprText(target).

2. internal/agent/suspicious_comparison_check.go L265-282: exprText switch cases are *ast.Ident, *ast.SelectorExpr, *ast.ParenExpr, *ast.BasicLit; default returns "". *ast.TypeAssertExpr is none of these.

3. checkUncheckedTypeAssert (L52-93) builds oldSet from assertFingerprint(a) = a.expr; any old assertion inserts key ""; any new assertion looks up "" → hit → filtered out. newOnly stays empty → L76-78 returns nil forever.

4. Executable reproduction (std-lib only, go/ast+go/parser): parsing three assertions (a.(string), a.(int), a.([]byte)) and calling the copied exprText switch prints "" for all of them. Simulated delta with old=1 assertion, new=3 assertions reports 0 new (expected 2). Also independently reproduced by the re-verification subagent with a 7-assertion program, same result.

Why the new test cannot catch this

TestCheckUncheckedTypeAssert_LineShiftNotReflagged (added by 14a4c14) compares old content vs the same assertions shifted by comments — this yields 0 warnings both with a correct fingerprint and with the broken empty-string fingerprint. The only test shape that exposes the bug — old has 1 assertion, new adds a different one, expect 1 warning — does not exist. TestCheckUncheckedTypeAssert_MultipleNew passes because oldSet is empty (no old content), so the empty-string collision never occurs.

Fix direction

Render a fingerprint that includes both the asserted expression and the target type, e.g. exprText(ta.X) + ".(" + exprText(ta.Type) + ")" (with a fallback to printer/go/printer or pos: for complex types), and add the missing test: pre-existing v.(int) + newly added v.(string) must report exactly 1 new assertion.

Labels

business-logic, detector, false-negative, regression

[topcheer]

已修复并推送（commit d5fc6ce）。

根因

确认属实：assertExprAt 把整个 *ast.TypeAssertExpr 传给 exprText，而其 switch 只覆盖 Ident/SelectorExpr/ParenExpr/BasicLit——所有断言指纹坍缩为空字符串，oldSet 含 "" 后 newOnly 永远为空，检测器在有既有断言的文件上永久静默。

修复

采用 issue 建议方案：指纹改为 exprText(ta.X) + ".(" + exprText(ta.Type) + ")"（先断言为 *ast.TypeAssertExpr 再取字段）——既有断言与新断言指纹不同，新增可被检出。

测试

TestCheckUncheckedTypeAssert_NewDifferentAssertion：旧内容 1 个 a.([]byte) + 新增 a.(string) 必须报 1 条（issue 指出的缺失测试形态）。全包测试通过。