bufferresetbeforereuse: per-block state isolation misses reuse across if/for/switch boundaries
- Dominant language
- Go
- Stars
- 5.1k
- Forks
- 541
- Avg merge
- 5h 48m
- Merged PRs (30d)
- 773
Description
**Summary**
bufferresetbeforereuse (the newest, 68th registered analyzer -- pkg/linters/bufferresetbeforereuse/bufferresetbeforereuse.go) is supposed to flag bytes.Buffer/strings.Builder reuse (write, then read via String()/Bytes()/Len(), then write again) without an intervening Reset(). The detector never links a write/read pair in an outer block with a later write inside a nested if/for/switch/select block, so the exact pattern the linter is designed to catch escapes detection whenever it spans a control-flow boundary -- arguably the most common real-world shape of this bug.
**Root cause**
- analyzeBlockForBufferReuse (bufferresetbeforereuse.go:74-84) walks the whole function body with ast.Inspect and calls analyzeStraightLineBlock once per BlockStmt node it encounters (the function body itself, plus every nested if/for/switch body, since ast.Inspects default return true lets it descend into those nested blocks).
- analyzeStraightLineBlock (bufferresetbeforereuse.go:86-130) creates brand-new written/read maps on each call (line 92-93) and calls collectEvents, which explicitly stops descent at IfStmt/ForStmt/RangeStmt/SwitchStmt/TypeSwitchStmt/SelectStmt/nested BlockStmt (collectEvents, line 133-155, the return false cases at line 137-139).
- Net effect: each blocks write/read/reset history is analyzed in total isolation from its enclosing block. A write in the outer scope, followed by a read, followed by a reuse-write inside a nested if/for block, is invisible to the linter because the inner blocks local written/read maps start empty -- the reuse-write is treated as if it were the first write ever seen.
**Reproduction (not in testdata)**
```
func reuseCrossIf(cond bool) {
var buf bytes.Buffer
buf.WriteString("first")
_ = buf.String()
if cond {
buf.WriteString("second") // reused without Reset -- NOT flagged
}
}
```
Trace: the outer blocks collectEvents treats the whole IfStmt as opaque (stops before descending into it), so it only sees write, read for buf at the outer level -- no violation (correct, in isolation). Separately, analyzeBlockForBufferReuses outer ast.Inspect also visits the ifs BlockStmt body directly and re-invokes analyzeStraightLineBlock on it with fresh maps; that isolated call sees only buf.WriteString("second"), a first write with no prior read recorded, so it is never reported. The same escape applies to for/switch/select bodies.
**Impact**
This is a false negative on the linters core, advertised purpose. The existing testdata (testdata/src/a/a.go) only tests same-block reuse and one true-negative branch case (okMutuallyExclusiveBranches, an early-return before the reuse point) -- there is no test at all for the (very common) case where a write+read happens before a conditional/loop and the buffer is written again inside it. Since the analyzer is newly registered and not yet in cgo.yml LINTER_FLAGS, this is latent rather than a live CI gap, but the gap will persist invisibly once enforcement is turned on.
**Recommendation**
Track write/read/reset state per-variable across the whole function scope (not per-BlockStmt), the same way the existing resourcetracker framework (used by fileclosenotdeferred/contextcancelnotdeferred/manualmutexunlock) tracks acquisitions across a FuncDecl, only breaking at FuncLit boundaries (which bufferresetbeforereuse already does correctly at the top level). A single ast.Inspect per FuncDecl/FuncLit that accumulates one written/read map for the whole function (still stopping only at FuncLit, not at every control-flow block) would catch the cross-block case while keeping the existing single-block tests passing.
**Validation checklist**
- Add a testdata case reproducing reuseCrossIf above (write+read outside, reuse-write inside if) and confirm it currently passes silently, then add a want comment after the fix
- Add a for-loop-body variant (write+read before the loop, reuse-write inside the loop) to same testdata
- Verify okMutuallyExclusiveBranches (early-return case) still reports no diagnostic after the fix
- Confirm existing same-block cases (notOkReuseWithoutReset, etc.) still report correctly
**Effort:** small-medium -- the state-tracking restructure is localized to analyzeBlockForBufferReuse/analyzeStraightLineBlock/collectEvents; no changes needed to run, checkBufferReuse, or the object-identification helpers.
> [!WARNING]
>
> Firewall blocked 1 domain
>
> The following domain was blocked by the firewall during workflow execution:
>
> - `api.anthropic.com`
>
> To allow these domains, add them to the `network.allowed` list in your workflow frontmatter:
>
> ```yaml
> network:
> allowed:
> - defaults
> - "api.anthropic.com"
> ```
>
> See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information.
>
>
> Generated by [🤖 Sergo - Serena Go Expert](https://github.com/github/gh-aw/actions/runs/34559696103) · claude · agent · 252.5 AIC · ⌖ 6.37 AIC · ⊞ 6.8K · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw+is%3Aissue+%22gh-aw-workflow-call-id%3A+github%2Fgh-aw%2Fsergo%22&type=issues)
> - [x] expires on Sep 17, 2026, 7:58 PM UTC-08:00
Contributor guide
Research direction
Start in pkg/linters/bufferresetbeforereuse/bufferresetbeforereuse.go by reading analyzeBlockForBufferReuse, analyzeStraightLineBlock, and collectEvents. Add testdata cases for reuseCrossIf and the for-loop variant in testdata/src/a/a.go, then verify the new cases report diagnostics while okMutuallyExclusiveBranches and existing same-block cases retain their current behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100