Summary
Three CI-enforced custom linters still identify standard-library package calls by the syntactic identifier name (ident.Name == "<pkg>") instead of by package identity via pass.TypesInfo. The canonical helper astutil.IsPkgSelector(pass, sel, pkgPath) already exists (pkg/linters/internal/astutil/astutil.go:75-94) and is already proven in production — rawloginlib was migrated to it (pkg/linters/rawloginlib/rawloginlib.go:58). Only three holdouts remain; migrating them closes a recurring false-positive/false-negative class across the enforced suite.
The holdouts
| Linter |
Location |
Current code |
osexitinlibrary |
pkg/linters/osexitinlibrary/osexitinlibrary.go:59 |
ident.Name == "os" && sel.Sel.Name == "Exit" |
fprintlnsprintf |
pkg/linters/fprintlnsprintf/fprintlnsprintf.go:144 |
ident.Name == "fmt" && sel.Sel.Name == name |
errstringmatch |
pkg/linters/errstringmatch/errstringmatch.go:85 |
ident.Name == "strings" && sel.Sel.Name == "Contains" |
All three are part of the 14 CI-enforced linters (.github/workflows/cgo.yml LINTER_FLAGS), so any false positive is a build-blocker and any false negative silently lets the targeted anti-pattern ship.
Why syntactic matching is wrong (two failure modes)
- Alias-import false negative.
import str "strings" then str.Contains(err.Error(), "x") — ident.Name is "str", so the call escapes the linter. Same for import xos "os" / import xfmt "fmt".
- Shadowing false positive (build-blocker). A local variable/parameter/field named
os, fmt, or strings with a matching method/field name (e.g. os.Exit) matches by name even though it is not the stdlib package. CI-enforced ⇒ a spurious diagnostic fails the build.
The established correct approach resolves the selector base to a *types.PkgName and compares the imported path:
obj := pass.TypesInfo.ObjectOf(pkgIdent)
pkgName, ok := obj.(*types.PkgName)
return ok && pkgName.Imported().Path() == pkgPath
This is exactly what astutil.IsPkgSelector (and astutil.IsFmtErrorf, astutil.go:51-73) already do.
Current impact
Latent today (no aliased strings/os/fmt imports in pkg/, and these identifiers are rarely shadowed), but this is the same class already accepted and fixed for sortslice (#38029, landed), rawloginlib (#39981, fix landed in code), and regexpcompileinfunction (#39733). The helper and precedent now exist, so finishing the migration is cheap and removes the class entirely.
Recommendation
Replace each syntactic check with the existing helper, mirroring rawloginlib.go:58:
if astutil.IsPkgSelector(pass, sel, "os") && sel.Sel.Name == "Exit" {
fprintlnsprintf.go:144 (isFmtFunc)
return astutil.IsPkgSelector(pass, sel, "fmt") && sel.Sel.Name == name
errstringmatch.go:85 (isStringsContains)
return astutil.IsPkgSelector(pass, sel, "strings") && sel.Sel.Name == "Contains"
Validation checklist
Effort
Small — one-line change per linter plus two testdata cases each; the helper already exists. Single PR.
Related
Same syntactic-package-identity class: #38029 (sortslice, landed), #39981 (rawloginlib, landed in code), #39733 (regexpcompileinfunction), #38789 (ctxbackground). This issue finishes the migration for the last three holdouts.
Generated by 🤖 Sergo - Serena Go Expert · 339.2 AIC · ⌖ 14.7 AIC · ⊞ 5.8K · ◷
Summary
Three CI-enforced custom linters still identify standard-library package calls by the syntactic identifier name (
ident.Name == "<pkg>") instead of by package identity viapass.TypesInfo. The canonical helperastutil.IsPkgSelector(pass, sel, pkgPath)already exists (pkg/linters/internal/astutil/astutil.go:75-94) and is already proven in production —rawloginlibwas migrated to it (pkg/linters/rawloginlib/rawloginlib.go:58). Only three holdouts remain; migrating them closes a recurring false-positive/false-negative class across the enforced suite.The holdouts
osexitinlibrarypkg/linters/osexitinlibrary/osexitinlibrary.go:59ident.Name == "os" && sel.Sel.Name == "Exit"fprintlnsprintfpkg/linters/fprintlnsprintf/fprintlnsprintf.go:144ident.Name == "fmt" && sel.Sel.Name == nameerrstringmatchpkg/linters/errstringmatch/errstringmatch.go:85ident.Name == "strings" && sel.Sel.Name == "Contains"All three are part of the 14 CI-enforced linters (
.github/workflows/cgo.ymlLINTER_FLAGS), so any false positive is a build-blocker and any false negative silently lets the targeted anti-pattern ship.Why syntactic matching is wrong (two failure modes)
import str "strings"thenstr.Contains(err.Error(), "x")—ident.Nameis"str", so the call escapes the linter. Same forimport xos "os"/import xfmt "fmt".os,fmt, orstringswith a matching method/field name (e.g.os.Exit) matches by name even though it is not the stdlib package. CI-enforced ⇒ a spurious diagnostic fails the build.The established correct approach resolves the selector base to a
*types.PkgNameand compares the imported path:This is exactly what
astutil.IsPkgSelector(andastutil.IsFmtErrorf,astutil.go:51-73) already do.Current impact
Latent today (no aliased
strings/os/fmtimports inpkg/, and these identifiers are rarely shadowed), but this is the same class already accepted and fixed forsortslice(#38029, landed),rawloginlib(#39981, fix landed in code), andregexpcompileinfunction(#39733). The helper and precedent now exist, so finishing the migration is cheap and removes the class entirely.Recommendation
Replace each syntactic check with the existing helper, mirroring
rawloginlib.go:58:osexitinlibrary.go:59fprintlnsprintf.go:144(isFmtFunc)errstringmatch.go:85(isStringsContains)Validation checklist
str "strings") — confirm it is now flagged (FN closed).os/fmt/strings— confirm it is NOT flagged (FP closed).os.Exit/fmt.Fprintln/strings.Containscases.go test ./pkg/linters/...green; CI linter pass overpkg/produces no new diagnostics.Effort
Small — one-line change per linter plus two testdata cases each; the helper already exists. Single PR.
Related
Same syntactic-package-identity class: #38029 (sortslice, landed), #39981 (rawloginlib, landed in code), #39733 (regexpcompileinfunction), #38789 (ctxbackground). This issue finishes the migration for the last three holdouts.