github / github/gh-aw

bufioscannererunchecked (70th linter): per-block Err() check misses checks placed in an enclosing block

Open
#60,738 0 comments 0 reactions 0 assignees View on GitHub
cookie sergo
Dominant language
Go
Stars
5.1k
Forks
541
Avg merge
5h 48m
Merged PRs (30d)
773

Description

Fresh audit of the 70th registered linter (bufioscannererunchecked, added 2026-09-13, registry grew 69 to 70 this run, never previously audited).

**Bug**: analyzeBlockStatements (bufioscannererunchecked.go:72-109) is invoked separately for every *ast.BlockStmt/CaseClause/CommClause via ast.Inspect in analyzeFuncBody (line 52-63). For each block it independently looks for a for-loop using scanner.Scan(), then calls hasScannerErrCheck(stmts[i+1:], scanner) (line 98) to search ONLY the sibling statements that follow the loop within that SAME statement list. A scanner.Err() check placed in an outer, enclosing block (after an if/for/switch that wraps the scan loop) lives in a different stmts slice entirely and is never seen.

**Concrete false positive**:

```go
func processFile(path string) error {
f, _ := os.Open(path)
defer f.Close()
scanner := bufio.NewScanner(f)
if shouldFilter(path) {
for scanner.Scan() {
process(scanner.Text())
}
}
return scanner.Err()
}
```

The loop is the only statement in the ifs body block, so hasScannerErrCheck sees an empty stmts[i+1:] and flags "Scanner loop does not check Err() after completion" even though scanner.Err() is correctly checked right after the if closes. Any real-world pattern that guards a scan loop with a conditional (skip parsing unless some flag is set, wrap the loop in a retry for-loop, dispatch it from a switch case) while still checking Err() once at the end of the function triggers this false positive.

**Evidence this is untested**: testdata/src/basic/bad.go badNestedBlock and testdata/src/basic/good.go goodNestedBlock both place the Err() check INSIDE the same if-block as the loop (good.go:141-149) - the only nested-block case exercised. No testdata covers a check performed in the block that ENCLOSES the loops block, which is the actual gap.

**Cross-check against prod**: grepped every scanner.Scan()/scanner.Err() pair in pkg/ (excluding _test.go) - all ~20 production call sites place the loop and its Err() check as direct siblings in the same top-level function block, so this is currently latent (0 live false positives) but is the same block_scope_state_reset class already confirmed in bufferresetbeforereuse (#60170/sg63a1) — a per-block state design that silently loses context across nested control-flow boundaries. Since this linter is not yet in cgo.yml LINTER_FLAGS (not CI-enforced), the blast radius is contained to local go vet-style runs for now, but will misfire the moment anyone writes the very common guarded-scan-loop idiom.

**Fix direction**: track the scanner-Err() search across the whole enclosing function body (walk all statements reachable after the loop, not just the current blocks tail), or compute reachability via the functions unified statement list before descending into nested blocks - mirroring how a control-flow-aware pass would need to unify state the same way bufferresetbeforereuse needs to for its own per-block reset bug.

**Validation checklist**: add a testdata case with the loop nested in an if/for/switch and the Err() check in the enclosing block (expect NO diagnostic); add the reverse case (loop nested, no Err() check anywhere - expect diagnostic); confirm fix does not reintroduce false negatives for genuinely unchecked nested loops.

Effort: small-medium (single file, ~70 lines of block-walking logic; needs a small refactor from per-block to whole-function-reachable-statement tracking).

> [!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/34803883363) · claude · agent · 209.1 AIC · ⌖ 7.52 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 20, 2026, 7:59 PM UTC-08:00

Contributor guide

Open the contributing guide

Research direction

Start in bufioscannererunchecked.go, reading analyzeFuncBody and analyzeBlockStatements to understand the per-block scan and Err() search. Add nested-block cases in testdata/src/basic/bad.go and good.go, then run the linter tests; done means an enclosing-block Err() check avoids a diagnostic, while genuinely unchecked nested loops still report one.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
devtools, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.