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 @@ -16,6 +16,7 @@ package main
import (
"golang.org/x/tools/go/analysis/multichecker"

"github.com/github/gh-aw/pkg/linters/ctxbackground"
"github.com/github/gh-aw/pkg/linters/excessivefuncparams"
"github.com/github/gh-aw/pkg/linters/largefunc"
"github.com/github/gh-aw/pkg/linters/osexitinlibrary"
Expand All @@ -24,6 +25,7 @@ import (

func main() {
multichecker.Main(
ctxbackground.Analyzer,
excessivefuncparams.Analyzer,
largefunc.Analyzer,
osexitinlibrary.Analyzer,
Expand Down
106 changes: 106 additions & 0 deletions pkg/linters/ctxbackground/ctxbackground.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// Package ctxbackground implements a Go analysis linter that flags
// calls to context.Background() inside functions that already receive
// a context.Context parameter.
package ctxbackground

import (
"go/ast"
"go/types"

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

// Analyzer is the ctx-background analysis pass.
var Analyzer = &analysis.Analyzer{
Comment on lines +15 to +16
Name: "ctxbackground",
Doc: "reports calls to context.Background() inside functions that already receive a context.Context parameter",
URL: "https://github.com/github/gh-aw/tree/main/pkg/linters/ctxbackground",
Requires: []*analysis.Analyzer{inspect.Analyzer},
Run: run,
}

func run(pass *analysis.Pass) (any, error) {
insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)

nodeFilter := []ast.Node{
(*ast.FuncDecl)(nil),
}
Comment on lines +27 to +29
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 nodeFilter only includes *ast.FuncDecl, so anonymous functions (*ast.FuncLit) that themselves receive a context.Context parameter are never checked. For example:

func Register(mux *http.ServeMux) {
    mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
        ctx := context.Background() // NOT flagged, but should be
        doWork(ctx)
    })
}

If this is an intentional limitation (keeping the linter simple), add a comment to nodeFilter documenting that FuncLit is excluded on purpose. If the intent is to catch all cases, extend nodeFilter to also include (*ast.FuncLit)(nil) and apply hasContextParam to FuncLit.Type as well.


insp.Preorder(nodeFilter, func(n ast.Node) {
fn, ok := n.(*ast.FuncDecl)
if !ok || fn.Body == nil {
return
}

// Check if any parameter is context.Context (and not blank).
if !hasContextParam(pass, fn) {
return
}

// Walk the function body for context.Background() calls.
ast.Inspect(fn.Body, func(node ast.Node) bool {
call, ok := node.(*ast.CallExpr)
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] ast.Inspect recursively descends into nested *ast.FuncLit nodes. This means context.Background() inside a closure that captures ctx from the outer scope will be flagged — even when the closure deliberately detaches from the parent context (e.g. a background goroutine that must outlive the request). You should skip nested func literals:

ast.Inspect(fn.Body, func(node ast.Node) bool {
    // Do not descend into nested function literals.
    if _, ok := node.(*ast.FuncLit); ok {
        return false
    }
    call, ok := node.(*ast.CallExpr)
    ...

Without this guard, patterns like go func() { ctx := context.Background(); ... }() inside a function that has a ctx param produce false positives.

if !ok {
return true
}
sel, ok := call.Fun.(*ast.SelectorExpr)
if !ok {
return true
}
ident, ok := sel.X.(*ast.Ident)
if !ok {
return true
}
if ident.Name == "context" && sel.Sel.Name == "Background" {
pass.Reportf(call.Pos(), "use the context.Context parameter instead of context.Background()")
Comment on lines +56 to +57
}
return true
})
})

return nil, nil
}

// hasContextParam returns true if fn has at least one non-blank parameter
// whose type is context.Context.
func hasContextParam(pass *analysis.Pass, fn *ast.FuncDecl) bool {
if fn.Type.Params == nil {
return false
}
ctxType := contextType(pass)
if ctxType == nil {
return false
}
for _, field := range fn.Type.Params.List {
t := pass.TypesInfo.TypeOf(field.Type)
if t == nil {
continue
}
if !types.Identical(t, ctxType) {
continue
}
// At least one name must not be blank.
for _, name := range field.Names {
if name.Name != "_" {
return true
}
}
}
return false
}

// contextType returns the types.Type for context.Context, or nil if the
// package is not imported.
func contextType(pass *analysis.Pass) types.Type {
for _, pkg := range pass.Pkg.Imports() {
if pkg.Path() == "context" {
obj := pkg.Scope().Lookup("Context")
if obj != nil {
return obj.Type()
}
}
}
return nil
}
16 changes: 16 additions & 0 deletions pkg/linters/ctxbackground/ctxbackground_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//go:build !integration

package ctxbackground_test

import (
"testing"

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

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

func TestCtxBackground(t *testing.T) {
testdata := analysistest.TestData()
analysistest.Run(t, testdata, ctxbackground.Analyzer, "ctxbackground")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package ctxbackground

import (
"context"
)

// flagged: function receives ctx context.Context but calls context.Background()
func DoWork(ctx context.Context) {
_ = context.Background() // want `use the context.Context parameter instead of context.Background\(\)`
Comment on lines +7 to +9
}

// not flagged: no context parameter
func DoWorkNoCtx() {
_ = context.Background()
}

// not flagged: blank identifier context parameter
func DoWorkBlank(_ context.Context) {
_ = context.Background()
}

// flagged: method with context param
type Worker struct{}

func (w *Worker) Run(ctx context.Context) {
_ = context.Background() // want `use the context.Context parameter instead of context.Background\(\)`
}

// not flagged: init function
func init() {
_ = context.Background()
}
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 testdata doesn't cover the anonymous-function/closure scenario, which is where the ast.Inspect descent issue above would surface. Consider adding:

// not flagged: context.Background() inside a func literal that has no ctx param
// (outer function has ctx, but the inner func literal is a distinct scope)
func DoWorkWithClosure(ctx context.Context) {
    go func() {
        _ = context.Background() // want? depends on the FuncLit guard decision
    }()
}

Having an explicit fixture for this case makes the intended behaviour clear and prevents regressions when the linter is later modified.

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] context.TODO() is a closely related anti-pattern ("I'll wire up a real context later") but isn't flagged by this linter. Either add a test fixture that explicitly marks it as not flagged (to document the intentional scope boundary), or extend the linter to also catch context.TODO(). Leaving it undocumented means the next person to maintain this linter won't know whether the omission was deliberate.

Loading