From 6fb7e1d19cb5561a4581df6be7a7c8df73b5a727 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 15 Jun 2026 19:30:11 +0000 Subject: [PATCH] linter: add fprintstderrinlibrary analyzer Reports fmt.Fprintf/Fprintln/Fprint calls that write directly to os.Stderr in library (pkg/) packages. Library code should use pkg/logger for structured logging rather than writing raw text to the standard error stream. This linter complements the existing rawloginlib analyzer (which flags log.* calls) by covering the fmt.Fprint* family of stderr writes, closing the gap for 133+ call sites found across pkg/workflow/ and pkg/parser/. Evidence: - Code-pattern scan found 133 fmt.Fprint*(os.Stderr, ...) calls in pkg/ - Consistent with the library logging policy enforced by rawloginlib - Not covered by any standard golangci-lint rule enabled by default Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cmd/linters/main.go | 4 +- .../fprintstderrinlibrary.go | 80 +++++++++++++++++++ .../fprintstderrinlibrary_test.go | 16 ++++ .../fprintstderrinlibrary.go | 21 +++++ 4 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 pkg/linters/fprintstderrinlibrary/fprintstderrinlibrary.go create mode 100644 pkg/linters/fprintstderrinlibrary/fprintstderrinlibrary_test.go create mode 100644 pkg/linters/fprintstderrinlibrary/testdata/src/fprintstderrinlibrary/fprintstderrinlibrary.go diff --git a/cmd/linters/main.go b/cmd/linters/main.go index fd3486fe000..77f5282da67 100644 --- a/cmd/linters/main.go +++ b/cmd/linters/main.go @@ -26,6 +26,7 @@ import ( "github.com/github/gh-aw/pkg/linters/fileclosenotdeferred" "github.com/github/gh-aw/pkg/linters/fmterrorfnoverbs" "github.com/github/gh-aw/pkg/linters/fprintlnsprintf" + "github.com/github/gh-aw/pkg/linters/fprintstderrinlibrary" "github.com/github/gh-aw/pkg/linters/hardcodedfilepath" "github.com/github/gh-aw/pkg/linters/httpnoctx" "github.com/github/gh-aw/pkg/linters/jsonmarshalignoredeerror" @@ -52,8 +53,9 @@ func main() { contextcancelnotdeferred.Analyzer, ctxbackground.Analyzer, errormessage.Analyzer, - fprintlnsprintf.Analyzer, errstringmatch.Analyzer, + fprintlnsprintf.Analyzer, + fprintstderrinlibrary.Analyzer, errorfwrapv.Analyzer, execcommandwithoutcontext.Analyzer, excessivefuncparams.Analyzer, diff --git a/pkg/linters/fprintstderrinlibrary/fprintstderrinlibrary.go b/pkg/linters/fprintstderrinlibrary/fprintstderrinlibrary.go new file mode 100644 index 00000000000..3fc806fc6d3 --- /dev/null +++ b/pkg/linters/fprintstderrinlibrary/fprintstderrinlibrary.go @@ -0,0 +1,80 @@ +// Package fprintstderrinlibrary implements a Go analysis linter that flags +// fmt.Fprint-style calls writing to os.Stderr in library (pkg/) packages. +package fprintstderrinlibrary + +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" +) + +var Analyzer = &analysis.Analyzer{ + Name: "fprintstderrinlibrary", + Doc: "reports fmt.Fprintf/Fprintln/Fprint calls to os.Stderr in library packages where pkg/logger should be used instead", + URL: "https://github.com/github/gh-aw/tree/main/pkg/linters/fprintstderrinlibrary", + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +var fprintFuncs = map[string]bool{ + "Fprint": true, "Fprintf": true, "Fprintln": true, +} + +func run(pass *analysis.Pass) (any, error) { + pkgPath := pass.Pkg.Path() + 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, "fprintstderrinlibrary") + + nodeFilter := []ast.Node{(*ast.CallExpr)(nil)} + + insp.Preorder(nodeFilter, func(n ast.Node) { + call, ok := n.(*ast.CallExpr) + if !ok { + return + } + if filecheck.IsTestFile(pass.Fset.Position(call.Pos()).Filename) { + return + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return + } + ident, ok := sel.X.(*ast.Ident) + if !ok { + return + } + if ident.Name != "fmt" || !fprintFuncs[sel.Sel.Name] || len(call.Args) == 0 { + return + } + argSel, ok := call.Args[0].(*ast.SelectorExpr) + if !ok { + return + } + argIdent, ok := argSel.X.(*ast.Ident) + if !ok { + return + } + if argIdent.Name == "os" && argSel.Sel.Name == "Stderr" { + position := pass.Fset.PositionFor(call.Pos(), false) + if nolint.HasDirective(position, noLintLinesByFile) { + return + } + pass.ReportRangef(call, "fmt.%s(os.Stderr, ...) called in library package %s; use pkg/logger instead", sel.Sel.Name, pkgPath) + } + }) + + return nil, nil +} diff --git a/pkg/linters/fprintstderrinlibrary/fprintstderrinlibrary_test.go b/pkg/linters/fprintstderrinlibrary/fprintstderrinlibrary_test.go new file mode 100644 index 00000000000..02e58ac7d4d --- /dev/null +++ b/pkg/linters/fprintstderrinlibrary/fprintstderrinlibrary_test.go @@ -0,0 +1,16 @@ +//go:build !integration + +package fprintstderrinlibrary_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/github/gh-aw/pkg/linters/fprintstderrinlibrary" +) + +func TestFprintStderrInLibrary(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, fprintstderrinlibrary.Analyzer, "fprintstderrinlibrary") +} diff --git a/pkg/linters/fprintstderrinlibrary/testdata/src/fprintstderrinlibrary/fprintstderrinlibrary.go b/pkg/linters/fprintstderrinlibrary/testdata/src/fprintstderrinlibrary/fprintstderrinlibrary.go new file mode 100644 index 00000000000..d0b42ce3c64 --- /dev/null +++ b/pkg/linters/fprintstderrinlibrary/testdata/src/fprintstderrinlibrary/fprintstderrinlibrary.go @@ -0,0 +1,21 @@ +package fprintstderrinlibrary + +import ( + "fmt" + "os" +) + +func bad() { + fmt.Fprintf(os.Stderr, "error: %s", "something") // want `fmt\.Fprintf.*os\.Stderr.*called in library package` + fmt.Fprintln(os.Stderr, "something") // want `fmt\.Fprintln.*os\.Stderr.*called in library package` +} + +func good() { + fmt.Fprintf(os.Stdout, "output: %s", "val") +} + +func suppressed() { + //nolint:fprintstderrinlibrary + fmt.Fprintf(os.Stderr, "suppressed previous line") + fmt.Fprint(os.Stderr, "suppressed same line") //nolint:fprintstderrinlibrary +}