Skip to content

deferinloop: range-over-func (Go 1.23+ iterators) false-positives defer-does-not-run-per-iteration, which is backwards for that #67336

Description

@github-actions

Linter: deferinloop (pkg/linters/deferinloop/deferinloop.go)

Problem

isInsideLoop (deferinloop.go:69-83) treats every *ast.RangeStmt identically: if a defer is enclosed by a for or range statement without an intervening *ast.FuncLit, it is flagged with the message defer inside a loop does not execute at the end of each iteration; it runs when the enclosing function returns.

That message is false for range-over-func loops (Go 1.23+, go.mod here already pins go 1.26.9). Per the language spec/design (go.dev/wiki range-over-func design, confirmed by the spec section on range clauses over func values), a for ... range someFunc { ... } loop body is compiled as an implicit function literal passed to the iterator as the yield continuation. A defer inside that body is scoped to that implicit per-iteration function, so it runs at the end of each iteration (or when the iterator stops), not when the enclosing function returns. This is the exact opposite of the diagnostic text, and the opposite of every other RangeStmt case (slice/map/channel/string/int), which is precisely why this is easy to miss: the AST shape (*ast.RangeStmt with a BlockStmt body) is indistinguishable from an ordinary range loop without consulting type information on rangeStmt.X.

isInsideLoop never looks at rangeStmt.X or pass.TypesInfo at all - it pattern-matches purely on node kind (*ast.ForStmt, *ast.RangeStmt, *ast.FuncLit), so a range over an iter.Seq[T]/iter.Seq2[K,V]-shaped function (or any func(func(...) bool) value) is indistinguishable from ranging over a slice.

Evidence

  • deferinloop.go:39-60 (run): walks every *ast.DeferStmt, calls isInsideLoop(cur), reports unconditionally if true - no type check anywhere in the analyzer.
  • deferinloop.go:69-83 (isInsideLoop): cur.Enclosing((*ast.ForStmt)(nil), (*ast.RangeStmt)(nil), (*ast.FuncLit)(nil)) returns true on the first ForStmt/RangeStmt match, false on the first FuncLit match - identical handling for every RangeStmt regardless of what is being ranged over.
  • testdata/src/deferinloop/deferinloop.go has zero coverage of range-over-func (only slice ranges, classic for, infinite for, select-in-for, and FuncLit-boundary cases) - the gap was never exercised, positively or negatively.
  • go.mod pins go 1.26.9, so this repos own toolchain fully supports range-over-func; it is an increasingly common idiom for resource-scoped iterators (e.g. a helper that yields rows/files and expects the caller to defer cleanup per item) - exactly the shape this linter is meant to help with, which makes a false positive here actively counterproductive (it would push a user to remove a defer that is actually correct, or to restructure correct code to silence a wrong warning).
  • No live trigger found in this repo today (grep -r iter.Seq pkg and func(yield func turned up only a comment referencing iter.Seq overhead in pkg/cli/remove_command.go, not an actual range-over-func call site) - this is a latent/forward-looking correctness gap, not a currently-firing false positive, matching the filing bar already used for sg90a1/sg91a1 (reasoned from code+spec, no confirmed live trigger yet, but the dormant mechanism is real and will misfire the moment someone writes this idiom).

Impact

A correct, idiomatic range-over-func + defer pattern gets flagged with a diagnostic message that asserts the opposite of what actually happens, which would mislead a developer into either removing a needed defer or adding incorrect manual cleanup, and burns review time arguing with a linter that is wrong about language semantics for this one case.

Recommendation

In isInsideLoop, when the enclosing match is a *ast.RangeStmt, consult pass.TypesInfo.TypeOf(rangeStmt.X) (the pattern already used elsewhere in this package, e.g. astutil.go:425/448): if its underlying type is a *types.Signature (a function value), treat it as a non-flaggable loop boundary for this purpose - either skip it like a FuncLit boundary, or (more precisely) continue walking outward past it rather than returning true, since the real per-iteration function boundary has already been crossed. This requires threading pass (or just pass.TypesInfo) into isInsideLoop, which does not currently take it.

Validation checklist

  • Add a testdata case ranging over a local func(yield func(int) bool) (or an iter.Seq[int]) value with a defer in the body - expect no diagnostic.
  • Confirm existing slice/classic-for/infinite-for/select-in-for/FuncLit-boundary cases in testdata/src/deferinloop/deferinloop.go still fire exactly as before (no regression to the real bug class this linter exists for).
  • Confirm a range over a plain channel/map/slice stored in a variable of non-func type is unaffected (only *types.Signature-underlying types should be exempted).

Effort: small - one type lookup added to isInsideLoop plus one new testdata case.

Generated by 🤖 Sergo - Serena Go Expert · claude · agent · 314.9 AIC · ⊞ 5.2K · ◷

  • expires on Oct 16, 2026, 8:04 PM UTC-08:00

Activity

  1. github-actions commented on Oct 10, 2026

    @github-actions
    ContributorAuthor

    🍪 Issue Monster selected this for Copilot

    I've identified this issue as a good candidate for automated resolution and requested assignment to the Copilot coding agent.

    If assignment succeeds, the Copilot coding agent will analyze the issue and create a pull request with the fix.

    Om nom nom! 🍪

    🍪 Om nom nom by Issue Monster · pi · gpt54 · 10.4 AIC · ⌖ 9.67 AIC · ⊞ 12.6K · ◷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    cookieIssue Monster Loves Cookies!sergo

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions