Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cmd/linters/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ import (

"github.com/github/gh-aw/pkg/linters/excessivefuncparams"
"github.com/github/gh-aw/pkg/linters/largefunc"
"github.com/github/gh-aw/pkg/linters/osexitinlibrary"
)

func main() {
multichecker.Main(
excessivefuncparams.Analyzer,
largefunc.Analyzer,
osexitinlibrary.Analyzer,
)
}
56 changes: 56 additions & 0 deletions pkg/linters/osexitinlibrary/osexitinlibrary.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Package osexitinlibrary implements a Go analysis linter that flags
// os.Exit calls in library (pkg/) packages.
package osexitinlibrary

import (
"go/ast"
"path/filepath"
"strings"

"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/ast/inspector"
)

// Analyzer is the os-exit-in-library analysis pass.
var Analyzer = &analysis.Analyzer{
Name: "osexitinlibrary",
Doc: "reports os.Exit calls inside library packages where they bypass deferred cleanup and prevent testing",
URL: "https://github.com/github/gh-aw/tree/main/pkg/linters/osexitinlibrary",
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 os.Exit.
if strings.HasSuffix(pkgPath, "/main") || strings.Contains(pkgPath, "/cmd/") {
return nil, nil
}

insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)

nodeFilter := []ast.Node{
(*ast.CallExpr)(nil),
}

insp.Preorder(nodeFilter, func(n ast.Node) {
call := n.(*ast.CallExpr)
if strings.HasSuffix(pkgPath, ".test") || strings.HasSuffix(filepath.Base(pass.Fset.Position(call.Pos()).Filename), "_test.go") {
return
}
sel, ok := call.Fun.(*ast.SelectorExpr)
if !ok {
return
}
ident, ok := sel.X.(*ast.Ident)
if !ok {
return
}
if ident.Name == "os" && sel.Sel.Name == "Exit" {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The check ident.Name == "os" matches by identifier name only — it doesn't verify that the os identifier actually refers to the standard library os package. If a file imports a third-party package with the alias os (unusual but valid Go), the linter would produce a false positive. The golang.org/x/tools/go/analysis framework provides type information via pass.TypesInfo that lets you confirm the import path:

if obj := pass.TypesInfo.ObjectOf(sel.Sel); obj != nil {
    if fn, ok := obj.(*types.Func); ok {
        if fn.Pkg().Path() == "os" && fn.Name() == "Exit" {
            pass.Reportf(call.Pos(), ...)
        }
    }
}

This is the recommended pattern in the golang.org/x/tools/go/analysis documentation and makes the linter robust to aliased imports.

pass.Reportf(call.Pos(), "os.Exit called in library package %s; move process termination to a cmd/ entry-point", pkgPath)
}
})

return nil, nil
}
16 changes: 16 additions & 0 deletions pkg/linters/osexitinlibrary/osexitinlibrary_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//go:build !integration

package osexitinlibrary_test

import (
"testing"

"golang.org/x/tools/go/analysis/analysistest"

"github.com/github/gh-aw/pkg/linters/osexitinlibrary"
)

func TestOsExitInLibrary(t *testing.T) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The test only exercises the "flagged" case. The skip logic (strings.Contains(pkgPath, "/cmd/")) is the other code path and it's untested. A second testdata package (e.g. testdata/src/cmd/main/main.go) containing an os.Exit call that should not be flagged would verify that the allow-list works correctly and prevent regressions if the skip condition is ever changed.

// testdata/src/cmd/main/main.go
package main

import "os"

func main() {
    os.Exit(0) // should NOT be flagged
}

Then add to the test:

analysistest.Run(t, testdata, osexitinlibrary.Analyzer, "osexitinlibrary", "cmd/main")

testdata := analysistest.TestData()
analysistest.Run(t, testdata, osexitinlibrary.Analyzer, "osexitinlibrary")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package osexitinlibrary

import "os"

// bad: os.Exit in a pkg/ package.
func stopProcess() {
os.Exit(1) // want `os.Exit called in library package`
}

// ok: helper that does NOT call os.Exit.
func doWork() error {
return nil
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package osexitinlibrary

import (
"os"
"testing"
)

func TestMain(m *testing.M) {
os.Exit(m.Run())
}