Skip to content

unchecked_deferredclose (79th linter) directly contradicts closeerrorunchecked's documented best-practice for defer Close() #67608

Description

@github-actions

Finding (new-linter day-one audit, registry delta 77->79)

The brand-new unchecked_deferredclose analyzer (pkg/linters/unchecked-deferred-close/unchecked_deferredclose.go, added 2026-10-10 via PR #67481) flags every bare defer x.Close() whose Close() returns an error, with the message "defer close() call ignores error return value; consider handling errors explicitly".

This is the OPPOSITE guidance of an existing sibling linter, closeerrorunchecked (pkg/linters/closeerrorunchecked/closeerrorunchecked.go), whose own test fixture explicitly documents the exact same shape as correct:

  • closeerrorunchecked/testdata/src/closeerrorunchecked/closeerrorunchecked.go:74-79, comment: "GoodDeferClose uses defer, which is the best practice -- not flagged." (f, _ := os.Open(...); defer f.Close())
  • closeerrorunchecked/testdata/.../closeerrorunchecked.go:132-136, comment: "GoodCustomCloserDefer defers custom type Close -- not flagged." (defer c.Close() where c.Close() error)

unchecked_deferredclose's own testdata reproduces the identical pattern and expects the opposite verdict:

  • unchecked-deferred-close/testdata/src/basic/bad.go:20-28, function bad(filename): f, err := os.Open(filename); if err != nil {...}; defer f.Close() -- flagged with want "defer close\(\) call ignores error return value".

So the SAME idiom (open with an err check, then a bare defer f.Close()) is simultaneously:

  • documented as "best practice, not flagged" by closeerrorunchecked (structurally true today too -- closeerrorunchecked's nodeFilter is {AssignStmt, ExprStmt} only, so it never even visits DeferStmt nodes), and
  • the canonical positive test case for unchecked_deferredclose.

Impact

This is not a theoretical edge case. A grep for the bare defer x.Close() shape across pkg/ (excluding test files) found roughly 150+ production call sites (pkg/cli, pkg/parser, pkg/workflow, pkg/fileutil, etc -- e.g. pkg/cli/firewall_log.go, pkg/cli/update_workflows.go, pkg/cli/deps_security.go, pkg/parser/remote_client.go). If both linters are ever enforced together (or even just run side by side in local make lint), a contributor following closeerrorunchecked's own documented guidance ("defer is best practice") would immediately get flagged by unchecked_deferredclose for doing exactly that, with contradictory remediation advice.

Recommendation

Pick one policy and align both linters/testdata to it:

  • If defer x.Close() (error silently dropped) is acceptable because it's already an improvement over a forgotten Close() entirely, unchecked_deferredclose should not exist as currently scoped, or should be merged into/gated behind the same philosophy as closeerrorunchecked.
  • If explicit error handling in a wrapped defer closure (defer func() { if err := f.Close(); err != nil {...} }()) is now the desired codebase-wide standard, closeerrorunchecked's GoodDeferClose/GoodCustomCloserDefer testdata comments and behavior are stale and should be updated to flag the same pattern (or deprecated in favor of unchecked_deferredclose).

Neither linter is currently enforced via a shared CI gate together with contradictory outcomes today (unchecked_deferredclose is not yet in any cgo.yml LINTER_FLAGS list), so there is no live CI breakage yet -- but this should be resolved before unchecked_deferredclose is wired into CI enforcement, to avoid shipping two custom linters with opposite opinions on the same code shape.

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

  • expires on Oct 17, 2026, 8:06 PM UTC-08:00

Activity

  1. github-actions commented on Oct 11, 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 · 11.1 AIC · ⌖ 13.3 AIC · ⊞ 12.1K · ◷

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