Summary
New bug (first-ever solo audit of the alias-tracking feature added by prior issue 37741). The len()-alias lookup table used to detect the n := len(s); n == 0 pattern is built ONCE for the entire pass before any comparison is analyzed, so it reflects only the FINAL state of each variable at the end of the whole package scan, not the state at the actual position of each comparison. Any later mutation of the same variable, anywhere further down in the same file, retroactively erases a genuinely valid earlier match.
Location
pkg/linters/lenstringzero/lenstringzero.go:23-33 (run) and :243-266 (collectLenStringAliases)
Evidence
run() computes the alias map exactly once, up front:
lenStringAliases := collectLenStringAliases(pass) // one snapshot for the whole pass
nodeFilter := []ast.Node{(*ast.BinaryExpr)(nil)}
return analyzerutil.Preorder(pass, nodeFilter, func(n ast.Node) {
analyzeLenStringExpr(pass, n, generatedFiles, noLintIndex, lenStringAliases) // same static map for every comparison, anywhere in the pass
})
collectLenStringAliasesFromAssignStmt inserts an alias entry on a := len(x) DEFINE, and unconditionally DELETES it on any later = reassignment or ++ / range-reuse of the same variable (lines 279-297). Since the whole map is finalized before any comparison is checked, a comparison that is textually BEFORE the later mutation still sees the mutation-caused deletion, because the deletion already happened during the single up-front collectLenStringAliases walk.
Impact example
func f(s string) bool {
n := len(s)
if n == 0 { // textually valid: n really does alias len(s) here -- SHOULD be flagged
return true
}
n = 99 // later reassignment, correctly invalidates n from THIS point onward
return n == 0 // correctly should NOT be flagged
}
With the current implementation, BOTH comparisons are silently left unflagged: the up-front collection processes the := then the = in source order and ends with no entry for n, so the first (valid) comparison is treated exactly like the second (invalid) one. Existing testdata (aliasReassignedNotFlagged) only covers reassign-then-use, which happens to produce the right answer by accident since there is no valid use before the reassignment; it does not cover use-then-reassign, which is where the bug surfaces as a false negative on a legitimate diagnostic.
Recommendation
Either: (a) make alias tracking a single forward flow-sensitive scan per function that queries an as-of-this-point map at each comparison instead of pre-computing one global map, or (b) restrict the feature to variables that are provably never reassigned/incremented anywhere in their enclosing function (a simple single-assignment check) before trusting the alias for a comparison later than the definition.
Validation checklist
- Add a golden testdata case: n := len(s); if n == 0 { ... } followed later in the same function by n = something-else, expecting the first comparison to still be flagged.
- Confirm no regression on the existing aliasReassignedNotFlagged / aliasIncrementedNotFlagged cases (both should remain unflagged).
Effort: Medium (requires switching from a single pre-computed map to a position-aware or single-assignment-only model).
Generated by 🤖 Sergo - Serena Go Expert · claude · agent · 215 AIC · ⌖ 8.66 AIC · ⊞ 6.9K · ◷
Summary
New bug (first-ever solo audit of the alias-tracking feature added by prior issue 37741). The len()-alias lookup table used to detect the n := len(s); n == 0 pattern is built ONCE for the entire pass before any comparison is analyzed, so it reflects only the FINAL state of each variable at the end of the whole package scan, not the state at the actual position of each comparison. Any later mutation of the same variable, anywhere further down in the same file, retroactively erases a genuinely valid earlier match.
Location
pkg/linters/lenstringzero/lenstringzero.go:23-33 (run) and :243-266 (collectLenStringAliases)
Evidence
run() computes the alias map exactly once, up front:
lenStringAliases := collectLenStringAliases(pass) // one snapshot for the whole pass
nodeFilter := []ast.Node{(*ast.BinaryExpr)(nil)}
return analyzerutil.Preorder(pass, nodeFilter, func(n ast.Node) {
analyzeLenStringExpr(pass, n, generatedFiles, noLintIndex, lenStringAliases) // same static map for every comparison, anywhere in the pass
})
collectLenStringAliasesFromAssignStmt inserts an alias entry on a := len(x) DEFINE, and unconditionally DELETES it on any later = reassignment or ++ / range-reuse of the same variable (lines 279-297). Since the whole map is finalized before any comparison is checked, a comparison that is textually BEFORE the later mutation still sees the mutation-caused deletion, because the deletion already happened during the single up-front collectLenStringAliases walk.
Impact example
func f(s string) bool {
n := len(s)
if n == 0 { // textually valid: n really does alias len(s) here -- SHOULD be flagged
return true
}
n = 99 // later reassignment, correctly invalidates n from THIS point onward
return n == 0 // correctly should NOT be flagged
}
With the current implementation, BOTH comparisons are silently left unflagged: the up-front collection processes the := then the = in source order and ends with no entry for n, so the first (valid) comparison is treated exactly like the second (invalid) one. Existing testdata (aliasReassignedNotFlagged) only covers reassign-then-use, which happens to produce the right answer by accident since there is no valid use before the reassignment; it does not cover use-then-reassign, which is where the bug surfaces as a false negative on a legitimate diagnostic.
Recommendation
Either: (a) make alias tracking a single forward flow-sensitive scan per function that queries an as-of-this-point map at each comparison instead of pre-computing one global map, or (b) restrict the feature to variables that are provably never reassigned/incremented anywhere in their enclosing function (a simple single-assignment check) before trusting the alias for a comparison later than the definition.
Validation checklist
Effort: Medium (requires switching from a single pre-computed map to a position-aware or single-assignment-only model).