Summary
The codebase has been systematically eliminating the syntactic package match antipattern — identifying a stdlib call by ident.Name == "<pkg>" instead of by package identity via pass.TypesInfo. The canonical helper astutil.IsPkgSelector(pass, sel, pkgPath) exists (pkg/linters/internal/astutil/astutil.go:75-94) and is proven in production (sortslice, rawloginlib, regexpcompileinfunction), with #40243 currently consolidating the last three CI-enforced holdouts (os/fmt/strings).
The two most recently added linters re-introduce the same antipattern:
| Linter |
Site |
Syntactic check |
fileclosenotdeferred |
pkg/linters/fileclosenotdeferred/fileclosenotdeferred.go:172 |
ident.Name != "os" (gates os.Open/Create/OpenFile) |
contextcancelnotdeferred |
pkg/linters/contextcancelnotdeferred/contextcancelnotdeferred.go:138 |
pkgIdent.Name != "context" (gates context.WithCancel/WithTimeout/WithDeadline) |
Defect class (same as the landed precedents)
- False positive (shadowing): a local variable named
os or context — e.g. os := newObjectStore(); os.Open(path) — is matched as the stdlib package, producing a spurious diagnostic.
- False negative (alias import):
import osx "os" → osx.Open(...), or import ctx "context" → ctx.WithCancel(...), escapes detection entirely.
Note contextcancelnotdeferred's "context" syntactic match is the exact class flagged and fixed for the sibling ctxbackground linter in #38789 (closed) — the same file family detected context.Background() syntactically there.
Why a separate issue from #40243
#40243 is explicitly scoped to the three remaining CI-enforced linters (os/fmt/strings holdouts: osexitinlibrary, fprintlnsprintf, errstringmatch). These two linters are newly landed and not in that scope, so they would otherwise slip through. Closing them completes the codebase-wide consolidation #40243 began and prevents the antipattern from regressing on every new linter.
Evidence
pkg/linters/fileclosenotdeferred/fileclosenotdeferred.go:166-176 — isFileOpenCall matches sel.X.(*ast.Ident) then ident.Name != "os"pkg/linters/contextcancelnotdeferred/contextcancelnotdeferred.go:132-147 — isContextWithCancelCall matches sel.X.(*ast.Ident) then pkgIdent.Name != "context"pkg/linters/internal/astutil/astutil.go:75-94 — IsPkgSelector resolves ObjectOf → *types.PkgName → Imported().Path()- Precedent:
pkg/linters/rawloginlib/rawloginlib.go:58 (migrated), and sprintferrdot.go:109 — the newest format linter already uses IsPkgSelector(pass, sel, "fmt") correctly, showing the helper is the established convention these two missed.
Recommendation
Replace the ident.Name == "<pkg>" checks with astutil.IsPkgSelector(pass, sel, "os") / astutil.IsPkgSelector(pass, sel, "context") respectively (one-line change each; the *ast.SelectorExpr is already in hand at both sites).
Validation checklist
Effort: small (two single-line changes + shadowing/alias testdata).
References: §27860925409
Generated by 🤖 Sergo - Serena Go Expert · 276.8 AIC · ⌖ 11.6 AIC · ⊞ 5.8K · ◷
Summary
The codebase has been systematically eliminating the syntactic package match antipattern — identifying a stdlib call by
ident.Name == "<pkg>"instead of by package identity viapass.TypesInfo. The canonical helperastutil.IsPkgSelector(pass, sel, pkgPath)exists (pkg/linters/internal/astutil/astutil.go:75-94) and is proven in production (sortslice,rawloginlib,regexpcompileinfunction), with #40243 currently consolidating the last three CI-enforced holdouts (os/fmt/strings).The two most recently added linters re-introduce the same antipattern:
fileclosenotdeferredpkg/linters/fileclosenotdeferred/fileclosenotdeferred.go:172ident.Name != "os"(gatesos.Open/Create/OpenFile)contextcancelnotdeferredpkg/linters/contextcancelnotdeferred/contextcancelnotdeferred.go:138pkgIdent.Name != "context"(gatescontext.WithCancel/WithTimeout/WithDeadline)Defect class (same as the landed precedents)
osorcontext— e.g.os := newObjectStore(); os.Open(path)— is matched as the stdlib package, producing a spurious diagnostic.import osx "os"→osx.Open(...), orimport ctx "context"→ctx.WithCancel(...), escapes detection entirely.Note
contextcancelnotdeferred's"context"syntactic match is the exact class flagged and fixed for the siblingctxbackgroundlinter in #38789 (closed) — the same file family detectedcontext.Background()syntactically there.Why a separate issue from #40243
#40243 is explicitly scoped to the three remaining CI-enforced linters (
os/fmt/stringsholdouts:osexitinlibrary,fprintlnsprintf,errstringmatch). These two linters are newly landed and not in that scope, so they would otherwise slip through. Closing them completes the codebase-wide consolidation #40243 began and prevents the antipattern from regressing on every new linter.Evidence
pkg/linters/fileclosenotdeferred/fileclosenotdeferred.go:166-176—isFileOpenCallmatchessel.X.(*ast.Ident)thenident.Name != "os"pkg/linters/contextcancelnotdeferred/contextcancelnotdeferred.go:132-147—isContextWithCancelCallmatchessel.X.(*ast.Ident)thenpkgIdent.Name != "context"pkg/linters/internal/astutil/astutil.go:75-94—IsPkgSelectorresolvesObjectOf→*types.PkgName→Imported().Path()pkg/linters/rawloginlib/rawloginlib.go:58(migrated), andsprintferrdot.go:109— the newest format linter already usesIsPkgSelector(pass, sel, "fmt")correctly, showing the helper is the established convention these two missed.Recommendation
Replace the
ident.Name == "<pkg>"checks withastutil.IsPkgSelector(pass, sel, "os")/astutil.IsPkgSelector(pass, sel, "context")respectively (one-line change each; the*ast.SelectorExpris already in hand at both sites).Validation checklist
os/contextvariable does not trigger a diagnosticosx "os"/ctx "context"calls are detectedgrepshows no aliasedos/contextimports inpkg/non-test today, so this is precision-hardening with zero churn)Effort: small (two single-line changes + shadowing/alias testdata).
References: §27860925409