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 +}