logfatallibrary duplicates the already-CI-enforced rawloginlib — its entire detection surface (log.Fatal/Fatalf/Fatalln in libra [Content truncated due to length]

[github-actions]

Summary

The new logfatallibrary analyzer flags log.Fatal / log.Fatalf / log.Fatalln in library (pkg/) packages. But the already-CI-enforced rawloginlib analyzer (.github/workflows/cgo.yml:1208, flag -rawloginlib) already flags every standard-log call in library packages — its rawLogFuncs set explicitly includes Fatal/Fatalf/Fatalln (pkg/linters/rawloginlib/rawloginlib.go:26-30). Both linters use identical gating (astutil.IsPkgSelector(pass, sel, "log"), the same /main+/cmd/ package exemption, the same test-file skip), so logfatallibrary's detection set is a strict subset of rawloginlib's — it reports nothing rawloginlib doesn't already report.

Net effect: logfatallibrary adds no new coverage. If it were added to CI it would double-report every log.Fatal* site (two diagnostics, two distinct //nolint directives required), and today it is dead weight in the registry.

Evidence

Aspect	rawloginlib (enforced)	logfatallibrary (new, not enforced)
Matched funcs	Print/Printf/Println, Fatal/Fatalf/Fatalln, Panic/Panicf/Panicln (rawloginlib.go:26-30)	Fatal/Fatalf/Fatalln (logfatallibrary.go:20-24)
Package identity	astutil.IsPkgSelector(pass, sel, "log") (rawloginlib.go:58)	astutil.IsPkgSelector(pass, sel, "log") (logfatallibrary.go:67)
Package exemption	HasSuffix(pkgPath,"/main") || Contains(pkgPath,"/cmd/") (rawloginlib.go:34)	same (logfatallibrary.go:38)
Test skip	filecheck.IsTestFile (rawloginlib.go:51)	filecheck.IsTestFile + .test pkg suffix (logfatallibrary.go:57)
nolint	per-linter index (rawloginlib.go:42)	per-linter index (logfatallibrary.go:46)
CI-enforced	yes — -rawloginlib in default + wasm runs (cgo.yml:1208, :1211)	no

Because the gating predicates are identical and {Fatal,Fatalf,Fatalln} ⊂ rawLogFuncs, every call flagged by logfatallibrary is also flagged by rawloginlib (which already runs in CI). The two diagnostics even share the same prefix — log.Fatal called in library package <pkg>; ... — differing only in the trailing advice (use pkg/logger instead vs use error returns instead to avoid implicit os.Exit).

Why this is a real problem, not just cosmetic

1. Double diagnostics / dual suppression. The nolint index is per-linter (nolint.BuildLineIndex(pass, "<name>")). A contributor with a justified log.Fatal in library code would need //nolint:rawloginlib,logfatallibrary once both are enforced — a footgun, since suppressing only the visible rawloginlib warning leaves a latent logfatallibrary one.
2. Zero added coverage. There are 0 production log.Fatal* sites in pkg/ today (all grep hits are doc comments, README samples, t.Fatalf from testing.T, or linter testdata), so logfatallibrary currently catches nothing that rawloginlib isn't already positioned to catch.
3. Misleading remediation split. For Fatal, rawloginlib's advice (use pkg/logger instead) is actually weaker than logfatallibrary's (return errors ... avoid implicit os.Exit) — swapping to pkg/logger does not address the os.Exit(1) termination semantics. So the accurate message currently lives in the linter that is NOT enforced.

Recommendation

Partition the two linters so each owns a disjoint, correctly-messaged slice — do not simply add logfatallibrary to CI alongside rawloginlib.

Preferred: remove Fatal/Fatalf/Fatalln from rawloginlib's rawLogFuncs (leaving Print* and Panic*), then wire logfatallibrary into LINTER_FLAGS. Result: rawloginlib owns the "use structured logging" funcs, logfatallibrary owns the Fatal family with the accurate os.Exit remediation, no site is double-reported, and each needs only one //nolint.

Alternative: if rawloginlib is meant to remain the single catch-all, drop logfatallibrary from the registry (cmd/linters/main.go) and fold its more-accurate wording into rawloginlib's message for the Fatal funcs.

Secondary (either path): the designated Fatal-catcher misses *log.Logger instance calls (logger.Fatal(...) from log.New(...)), which have the identical os.Exit(1) harm — currently latent (0 prod sites) but worth a // TODO if logfatallibrary becomes the canonical Fatal linter.

Validation checklist

• Decide ownership: Fatal* handled by exactly one enforced linter.
• If partitioning: rawLogFuncs no longer contains Fatal/Fatalf/Fatalln; rawloginlib testdata updated (e.g. alias_import.go moved/retargeted); logfatallibrary added to LINTER_FLAGS in both cgo.yml runs.
• make golint-custom LINTER_FLAGS="-rawloginlib -logfatallibrary -test=false" produces at most one diagnostic per log.Fatal* site (no duplicates).
• A single //nolint:<owner> suppresses a justified log.Fatal with no residual warning from the other linter.

Effort: small — one-line rawLogFuncs edit + testdata retarget + 2 CI flag additions (partition path), or a registry removal (drop path).

Generated by 🤖 Sergo - Serena Go Expert · 312.4 AIC · ⌖ 14 AIC · ⊞ 5.8K · ◷

• expires on Jul 20, 2026, 8:49 PM UTC-08:00

[github-actions]

🍪 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 · 2.91 AIC · ⌖ 6 AIC · ⊞ 5.4K · ◷