[linter-miner] feat(linters): add logfatallibrary linter to flag log.Fatal calls in library packages

cmd/linters/main.go

@@ -38,6 +38,7 @@ import (
 	"github.com/github/gh-aw/pkg/linters/largefunc"
 	"github.com/github/gh-aw/pkg/linters/lenstringsplit"
 	"github.com/github/gh-aw/pkg/linters/lenstringzero"
+	"github.com/github/gh-aw/pkg/linters/logfatallibrary"
 	"github.com/github/gh-aw/pkg/linters/manualmutexunlock"
 	"github.com/github/gh-aw/pkg/linters/osexitinlibrary"
 	"github.com/github/gh-aw/pkg/linters/osgetenvlibrary"
@@ -84,6 +85,7 @@ func main() {
 		httprespbodyclose.Analyzer,
 		httpstatuscode.Analyzer,
 		largefunc.Analyzer,
+		logfatallibrary.Analyzer,
 		manualmutexunlock.Analyzer,
 		osexitinlibrary.Analyzer,
 		osgetenvlibrary.Analyzer,

docs/adr/45115-logfatallibrary-linter-for-pkg-packages.md

@@ -0,0 +1,43 @@
+# ADR-45115: Add logfatallibrary Linter to Flag log.Fatal Calls in Library Packages
+
+**Date**: 2026-07-13
+**Status**: Draft
+**Deciders**: Unknown
+
+---
+
+### Context
+
+The repository enforces a culture of safe library code through static analysis linters. Library packages under `pkg/` must not terminate the process directly because callers have no opportunity to recover or clean up. The existing `osexitinlibrary` linter catches direct `os.Exit` calls, and `panicinlibrarycode` catches `panic`. However, `log.Fatal`, `log.Fatalf`, and `log.Fatalln` from the standard `log` package also call `os.Exit(1)` internally, making them equally dangerous in library code. This implicit exit path bypasses deferred cleanup (open files, database connections, in-flight requests), makes packages untestable in isolation (a test binary can be aborted by a library call), and prevents callers from handling errors gracefully. No automated check existed to catch this pattern.
+
+### Decision
+
+We will add a new `logfatallibrary` Go analysis pass under `pkg/linters/logfatallibrary/` that reports any call to `log.Fatal`, `log.Fatalf`, or `log.Fatalln` inside a non-`cmd/` package. Entry-point packages (those under `cmd/` or with path suffix `/main`) are explicitly exempted because process termination is acceptable in executables. Library code must return errors instead, consistent with standard Go idiom and the existing linter family.
+
+### Alternatives Considered
+
+#### Alternative 1: Extend the existing `osexitinlibrary` linter
+
+The existing linter already detects direct `os.Exit` calls in library packages. It could be extended to also detect `log.Fatal*` calls, consolidating the logic in one place. This was rejected because the two linters flag conceptually distinct constructs (explicit vs. implicit exit) and keeping them separate makes each linter's doc string, suppression directive name, and test fixture self-contained. Merging them would also widen the blast radius of any future change to either rule.
+
+#### Alternative 2: Rely on documentation and code review
+
+The prohibition on `log.Fatal*` in library code could be documented in the contributing guide and enforced through code review alone. This was rejected because code review is inconsistent and does not scale — the pattern is easy to miss, especially for contributors unfamiliar with the implicit `os.Exit` behavior of `log.Fatal`. Automated enforcement is the only approach that catches every occurrence unconditionally.
+
+### Consequences
+
+#### Positive
+- Every future PR that introduces `log.Fatal*` in a library package is flagged automatically, eliminating a class of hidden process-termination bugs.
+- The new linter is consistent with the existing `osexitinlibrary` and `panicinlibrarycode` passes, extending the suite without introducing new patterns or conventions.
+
+#### Negative
+- Existing library code that currently uses `log.Fatal*` must be migrated to return errors, which may require changes to function signatures and call sites.
+- Developers who are unaware that `log.Fatal` calls `os.Exit` may be surprised by the diagnostic and need to understand the reason before they can fix it.
+
+#### Neutral
+- The linter supports `//nolint:logfatallibrary` directives (both same-line and previous-line) for the rare legitimate suppression case.
+- Test files (files matching `_test.go`) and test packages (path suffix `.test`) are excluded from analysis, so test helpers that use `log.Fatal` for test setup are unaffected.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/linters/logfatallibrary/logfatallibrary.go

@@ -0,0 +1,78 @@
+// Package logfatallibrary implements a Go analysis linter that flags
+// log.Fatal, log.Fatalf, and log.Fatalln calls in library (pkg/) packages.
+// These functions call os.Exit(1) internally, which bypasses deferred cleanup
+// and makes the package untestable in isolation.
+package logfatallibrary
+
+import (
+	"go/ast"
+	"strings"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+
+	"github.com/github/gh-aw/pkg/linters/internal/astutil"
+	"github.com/github/gh-aw/pkg/linters/internal/filecheck"
+	"github.com/github/gh-aw/pkg/linters/internal/nolint"
+)
+
+// fatalFuncs is the set of log functions that call os.Exit(1) internally.
+var fatalFuncs = map[string]bool{
+	"Fatal":   true,
+	"Fatalf":  true,
+	"Fatalln": true,
+}
+
+// Analyzer is the log-fatal-in-library analysis pass.
+var Analyzer = &analysis.Analyzer{
+	Name:     "logfatallibrary",
+	Doc:      "reports log.Fatal, log.Fatalf, and log.Fatalln calls inside library packages where they implicitly call os.Exit and bypass deferred cleanup",
+	URL:      "https://github.com/github/gh-aw/tree/main/pkg/linters/logfatallibrary",
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      run,
+}
+
+func run(pass *analysis.Pass) (any, error) {
+	pkgPath := pass.Pkg.Path()
+	// Skip packages under cmd/ entry-points — they are allowed to call log.Fatal.
+	if strings.HasSuffix(pkgPath, "/main") || strings.Contains(pkgPath, "/cmd/") {
+		return nil, nil
+	}
+
+	insp, err := astutil.Inspector(pass)
+	if err != nil {
+		return nil, err
+	}
+	noLintLinesByFile := nolint.BuildLineIndex(pass, "logfatallibrary")
+
+	nodeFilter := []ast.Node{
+		(*ast.CallExpr)(nil),
+	}
+
+	insp.Preorder(nodeFilter, func(n ast.Node) {
+		call, ok := n.(*ast.CallExpr)
+		if !ok {
+			return
+		}
+		if strings.HasSuffix(pkgPath, ".test") || filecheck.IsTestFile(pass.Fset.Position(call.Pos()).Filename) {
+			return
+		}
+		sel, ok := call.Fun.(*ast.SelectorExpr)
+		if !ok {
+			return
+		}
+		if !fatalFuncs[sel.Sel.Name] {
+			return
+		}
+		if !astutil.IsPkgSelector(pass, sel, "log") {
+			return
+		}
+		position := pass.Fset.PositionFor(call.Pos(), false)
+		if nolint.HasDirective(position, noLintLinesByFile) {
+			return
+		}
+		pass.ReportRangef(call, "log.%s called in library package %s; use error returns instead to avoid implicit os.Exit", sel.Sel.Name, pkgPath)
+	})
+
+	return nil, nil
+}

pkg/linters/logfatallibrary/logfatallibrary_test.go

@@ -0,0 +1,16 @@
+//go:build !integration
+
+package logfatallibrary_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+
+	"github.com/github/gh-aw/pkg/linters/logfatallibrary"
+)
+
+func TestLogFatalLibrary(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, logfatallibrary.Analyzer, "logfatallibrary")
+}

pkg/linters/logfatallibrary/testdata/src/logfatallibrary/logfatallibrary.go

@@ -0,0 +1,34 @@
+package logfatallibrary
+
+import "log"
+
+// bad: log.Fatal in a pkg/ package.
+func stopWithFatal() {
+	log.Fatal("something went wrong") // want `log.Fatal called in library package`
+}
+
+// bad: log.Fatalf in a pkg/ package.
+func stopWithFatalf(msg string) {
+	log.Fatalf("error: %s", msg) // want `log.Fatalf called in library package`
+}
+
+// bad: log.Fatalln in a pkg/ package.
+func stopWithFatalln() {
+	log.Fatalln("fatal error") // want `log.Fatalln called in library package`
+}
+
+// ok: log.Print is not fatal.
+func logSomething() {
+	log.Print("just a message")
+}
+
+// ok: nolint suppression on previous line.
+func suppressedPreviousLine() {
+	//nolint:logfatallibrary
+	log.Fatal("suppressed")
+}
+
+// ok: nolint suppression on same line.
+func suppressedSameLine() {
+	log.Fatal("suppressed") //nolint:logfatallibrary
+}