Summary
The new lenstringzero analyzer only inspects direct len(s) == 0 / len(s) != 0 binary expressions. When the length is first stored in a local variable — n := len(s); if n == 0 { ... } — the comparison is not flagged. This false-negative is explicitly acknowledged in the linter's own ADR, and the fix has a proven in-repo template: the sibling tolowerequalfold analyzer just landed exactly this kind of alias-tracking in #37492.
- Issue type: linter precision (false negative)
- Linter:
lenstringzero (pkg/linters/lenstringzero/lenstringzero.go)
- Status: ⚠️ documented limitation, now fixable with a landed precedent
Evidence
The analyzer's run only matches a *ast.BinaryExpr whose operand is itself a len(...) *ast.CallExpr:
// pkg/linters/lenstringzero/lenstringzero.go:48-56
var lenArg ast.Expr
if isLenCall(expr.X) && isIntZero(expr.Y) {
lenArg = lenCallArg(expr.X)
} else if isIntZero(expr.X) && isLenCall(expr.Y) {
lenArg = lenCallArg(expr.Y)
}
if lenArg == nil {
return
}
There is no handling for an identifier whose value is a previously-assigned len(...) result, so n := len(s); n == 0 slips through.
The ADR documents this as a known gap:
"Detection is limited to direct len(...) comparisons against the literal 0; an equivalent length stored in an intermediate variable (n := len(s); n == 0) is not flagged (false negatives)." — docs/adr/37618-add-lenstringzero-linter.md, Consequences › Negative
Why this is actionable now (proven template)
The tolowerequalfold analyzer faced the same class of var-alias false-negative (Sergo sg29a1) and it was fixed in #37492 by adding an alias-collection pass:
tolowerequalfold alias-tracking precedent
pkg/linters/tolowerequalfold/tolowerequalfold.go now builds a map[types.Object]ast.Expr of local variables assigned a ToLower/ToUpper result:
func collectCaseConvAliases(pass *analysis.Pass) map[types.Object]ast.Expr {
aliases := make(map[types.Object]ast.Expr)
for _, file := range pass.Files {
ast.Inspect(file, func(node ast.Node) bool {
switch n := node.(type) {
case *ast.AssignStmt: collectAliasesFromAssignStmt(pass, n, aliases)
case *ast.ValueSpec: collectAliasesFromValueSpec(pass, n, aliases)
case *ast.IncDecStmt: /* invalidate */
case *ast.RangeStmt: /* invalidate */
}
return true
})
}
return aliases
}
It correctly invalidates aliases that are mutated (IncDecStmt, re-assignment via RangeStmt) so a reassigned variable is not falsely flagged.
Recommendation
Mirror the tolowerequalfold approach in lenstringzero:
- Add a pre-pass that records local
types.Objects assigned a single-argument len(...) call whose argument type-checks to an underlying string (reuse the existing isLenCall / type check).
- Invalidate an alias on any subsequent mutation (
IncDecStmt, re-assignment, ++/--, address-of) — copy the invalidation cases from collectCaseConvAliases to avoid false positives.
- In the
BinaryExpr handler, when an operand is an identifier resolving to a recorded len-of-string alias and the other operand is integer 0, report the same diagnostic.
Before / after intent
// not currently flagged -> should be flagged
n := len(s) // s is a string
if n == 0 { ... } // want: use s == \"\" to check for empty string
Validation checklist
Effort: Medium (single-package change with a directly-applicable in-repo template; ~30–60 LOC + fixtures).
References
Generated by 🤖 Sergo - Serena Go Expert · 236 AIC · ⌖ 15.2 AIC · ⊞ 6.5K · ◷
Summary
The new
lenstringzeroanalyzer only inspects directlen(s) == 0/len(s) != 0binary expressions. When the length is first stored in a local variable —n := len(s); if n == 0 { ... }— the comparison is not flagged. This false-negative is explicitly acknowledged in the linter's own ADR, and the fix has a proven in-repo template: the siblingtolowerequalfoldanalyzer just landed exactly this kind of alias-tracking in #37492.lenstringzero(pkg/linters/lenstringzero/lenstringzero.go)Evidence
The analyzer's
runonly matches a*ast.BinaryExprwhose operand is itself alen(...)*ast.CallExpr:There is no handling for an identifier whose value is a previously-assigned
len(...)result, son := len(s); n == 0slips through.The ADR documents this as a known gap:
Why this is actionable now (proven template)
The
tolowerequalfoldanalyzer faced the same class of var-alias false-negative (Sergo sg29a1) and it was fixed in #37492 by adding an alias-collection pass:tolowerequalfold alias-tracking precedent
pkg/linters/tolowerequalfold/tolowerequalfold.gonow builds amap[types.Object]ast.Exprof local variables assigned aToLower/ToUpperresult:It correctly invalidates aliases that are mutated (
IncDecStmt, re-assignment viaRangeStmt) so a reassigned variable is not falsely flagged.Recommendation
Mirror the
tolowerequalfoldapproach inlenstringzero:types.Objects assigned a single-argumentlen(...)call whose argument type-checks to an underlyingstring(reuse the existingisLenCall/ type check).IncDecStmt, re-assignment,++/--, address-of) — copy the invalidation cases fromcollectCaseConvAliasesto avoid false positives.BinaryExprhandler, when an operand is an identifier resolving to a recorded len-of-string alias and the other operand is integer0, report the same diagnostic.Before / after intent
Validation checklist
analysistestfixture underpkg/linters/lenstringzero/testdatawith an := len(s); n == 0case carrying a// wantannotation[]byte/slice/array length aliases are still excluded (string-only)go test ./pkg/linters/lenstringzero/...passesEffort: Medium (single-package change with a directly-applicable in-repo template; ~30–60 LOC + fixtures).
References
docs/adr/37618-add-lenstringzero-linter.md(Consequences › Negative)collectCaseConvAliasespkg/linters/lenstringzero/lenstringzero.go:48-56