Problem
pkg/linters/uncheckedflushreturn/uncheckedflushreturn.go is meant to catch discarded error returns from Flush(). Its AST node filter is:
nodeFilter := []ast.Node{(*ast.ExprStmt)(nil), (*ast.AssignStmt)(nil)}(uncheckedflushreturn.go:45). This covers a bare w.Flush() statement (ExprStmt) and _ = w.Flush() (AssignStmt), but ast.DeferStmt is not in the filter at all. defer w.Flush() wraps the call in its own *ast.DeferStmt node — it is not an ExprStmt or AssignStmt — so insp.Preorder never visits it and the call is invisible to this analyzer, regardless of type.
Evidence
uncheckedflushreturn.go:45-63: run() only registers ExprStmt/AssignStmt in nodeFilter; there is no case *ast.DeferStmt anywhere in the file.testdata/src/uncheckedflushreturn/uncheckedflushreturn.go has zero test cases using defer. bad()/badBlankAssign() test the bare-statement and blank-assign forms only; good()/suppressed() likewise never exercise defer. This is an untested gap, not a documented/intentional scope decision (nothing in the package doc or ADR calls out defer as out of scope).defer bufWriter.Flush() (or defer w.Flush() for tabwriter/gzip.Writer/similar) is widely recognized as the single most common way Go code silently drops a flush error — it is the canonical motivating example in Go's own error-handling guidance for exactly this class of bug, and is the pattern errcheck-style tools specifically target for Close/Flush. That makes this the highest-value case for a linter with this Doc string ("reports Flush() method calls where the error return is discarded") to miss.- No current production hits in
pkg/ (grep -rn "defer.*\.Flush()" pkg/ is empty), so this is a coverage gap rather than an active false negative today — but it means the linter provides no protection if such code is added later, which is exactly the scenario it exists to prevent.
Impact
A developer who writes defer w.Flush() — the idiomatic-looking but incorrect pattern for flush error handling — gets no diagnostic from this linter, even though w.Flush() and _ = w.Flush() right next to it would both be flagged. The analyzer's own two non-deferred forms are covered; the deferred form, arguably the primary motivating case, silently passes through.
Recommendation
Add *ast.DeferStmt to nodeFilter and a case in the switch that unwraps stmt.Call (an *ast.CallExpr) and runs it through the existing isFlushCallReturningError check, then reportUncheckedFlush — mirroring checkDiscardedFlushExpr but sourced from DeferStmt.Call instead of ExprStmt.X. Add a deferBad() golden test case (and a deferGood()/deferSuppressed() if the project's convention is to test the nolint escape hatch per form, matching the existing bad/good/suppressed triplet structure) to testdata/src/uncheckedflushreturn/uncheckedflushreturn.go.
Validation checklist
Effort
Small - one new node-filter entry, one new case function (~10-15 lines mirroring checkDiscardedFlushExpr), plus testdata additions.
Generated by 🤖 Sergo - Serena Go Expert · agent · 167.6 AIC · ⌖ 33.2 AIC · ⊞ 6K · ◷
Problem
pkg/linters/uncheckedflushreturn/uncheckedflushreturn.gois meant to catch discarded error returns fromFlush(). Its AST node filter is:(
uncheckedflushreturn.go:45). This covers a barew.Flush()statement (ExprStmt) and_ = w.Flush()(AssignStmt), butast.DeferStmtis not in the filter at all.defer w.Flush()wraps the call in its own*ast.DeferStmtnode — it is not anExprStmtorAssignStmt— soinsp.Preordernever visits it and the call is invisible to this analyzer, regardless of type.Evidence
uncheckedflushreturn.go:45-63:run()only registersExprStmt/AssignStmtinnodeFilter; there is nocase *ast.DeferStmtanywhere in the file.testdata/src/uncheckedflushreturn/uncheckedflushreturn.gohas zero test cases usingdefer.bad()/badBlankAssign()test the bare-statement and blank-assign forms only;good()/suppressed()likewise never exercisedefer. This is an untested gap, not a documented/intentional scope decision (nothing in the package doc or ADR calls outdeferas out of scope).defer bufWriter.Flush()(ordefer w.Flush()fortabwriter/gzip.Writer/similar) is widely recognized as the single most common way Go code silently drops a flush error — it is the canonical motivating example in Go's own error-handling guidance for exactly this class of bug, and is the patternerrcheck-style tools specifically target forClose/Flush. That makes this the highest-value case for a linter with this Doc string ("reports Flush() method calls where the error return is discarded") to miss.pkg/(grep -rn "defer.*\.Flush()" pkg/is empty), so this is a coverage gap rather than an active false negative today — but it means the linter provides no protection if such code is added later, which is exactly the scenario it exists to prevent.Impact
A developer who writes
defer w.Flush()— the idiomatic-looking but incorrect pattern for flush error handling — gets no diagnostic from this linter, even thoughw.Flush()and_ = w.Flush()right next to it would both be flagged. The analyzer's own two non-deferred forms are covered; the deferred form, arguably the primary motivating case, silently passes through.Recommendation
Add
*ast.DeferStmttonodeFilterand a case in the switch that unwrapsstmt.Call(an*ast.CallExpr) and runs it through the existingisFlushCallReturningErrorcheck, thenreportUncheckedFlush— mirroringcheckDiscardedFlushExprbut sourced fromDeferStmt.Callinstead ofExprStmt.X. Add adeferBad()golden test case (and adeferGood()/deferSuppressed()if the project's convention is to test the nolint escape hatch per form, matching the existingbad/good/suppressedtriplet structure) totestdata/src/uncheckedflushreturn/uncheckedflushreturn.go.Validation checklist
nodeFilterincludes(*ast.DeferStmt)(nil)stmt.CallviaisFlushCallReturningError+reportUncheckedFlushdefer w.Flush()flagged with the same diagnostic messagedefer w.Flush() (nolint/redacted):uncheckedflushreturncorrectly suppressedanalysistest.Runpasses with updated golden expectationsEffort
Small - one new node-filter entry, one new case function (~10-15 lines mirroring
checkDiscardedFlushExpr), plus testdata additions.