From fec045adfcbf08bfcc98e557cd505da65422a5ac Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 27 May 2026 21:18:00 +0000 Subject: [PATCH 1/2] Initial plan From 44fbb138ef0b803ae727d5d34a1b0849894af285 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 27 May 2026 21:25:59 +0000 Subject: [PATCH 2/2] Add contextcancelnotdeferred custom analyzer Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- cmd/linters/main.go | 2 + .../contextcancelnotdeferred.go | 148 ++++++++++++++++++ .../contextcancelnotdeferred_test.go | 16 ++ .../contextcancelnotdeferred.go | 56 +++++++ 4 files changed, 222 insertions(+) create mode 100644 pkg/linters/contextcancelnotdeferred/contextcancelnotdeferred.go create mode 100644 pkg/linters/contextcancelnotdeferred/contextcancelnotdeferred_test.go create mode 100644 pkg/linters/contextcancelnotdeferred/testdata/src/contextcancelnotdeferred/contextcancelnotdeferred.go diff --git a/cmd/linters/main.go b/cmd/linters/main.go index de711dccea5..30cb62d23d3 100644 --- a/cmd/linters/main.go +++ b/cmd/linters/main.go @@ -16,6 +16,7 @@ package main import ( "golang.org/x/tools/go/analysis/multichecker" + "github.com/github/gh-aw/pkg/linters/contextcancelnotdeferred" "github.com/github/gh-aw/pkg/linters/ctxbackground" "github.com/github/gh-aw/pkg/linters/errormessage" "github.com/github/gh-aw/pkg/linters/errstringmatch" @@ -35,6 +36,7 @@ import ( func main() { multichecker.Main( + contextcancelnotdeferred.Analyzer, ctxbackground.Analyzer, errormessage.Analyzer, fprintlnsprintf.Analyzer, diff --git a/pkg/linters/contextcancelnotdeferred/contextcancelnotdeferred.go b/pkg/linters/contextcancelnotdeferred/contextcancelnotdeferred.go new file mode 100644 index 00000000000..2772e1d63b7 --- /dev/null +++ b/pkg/linters/contextcancelnotdeferred/contextcancelnotdeferred.go @@ -0,0 +1,148 @@ +// Package contextcancelnotdeferred implements a Go analysis linter that flags +// context cancel functions called manually instead of deferred. +package contextcancelnotdeferred + +import ( + "fmt" + "go/ast" + "go/token" + "go/types" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" + + "github.com/github/gh-aw/pkg/linters/internal/filecheck" +) + +// Analyzer is the context-cancel-not-deferred analysis pass. +var Analyzer = &analysis.Analyzer{ + Name: "contextcancelnotdeferred", + Doc: "reports context cancel functions that are called directly instead of deferred", + URL: "https://github.com/github/gh-aw/tree/main/pkg/linters/contextcancelnotdeferred", + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (any, error) { + insp, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + if !ok { + return nil, fmt.Errorf("inspect analyzer result has unexpected type %T", pass.ResultOf[inspect.Analyzer]) + } + + nodeFilter := []ast.Node{ + (*ast.FuncDecl)(nil), + } + + insp.Preorder(nodeFilter, func(n ast.Node) { + fn, ok := n.(*ast.FuncDecl) + if !ok || fn.Body == nil { + return + } + + pos := pass.Fset.PositionFor(fn.Pos(), false) + if filecheck.IsTestFile(pos.Filename) { + return + } + + cancelVars := make(map[types.Object]*cancelVarState) + + ast.Inspect(fn.Body, func(node ast.Node) bool { + if node == nil { + return false + } + + if _, ok := node.(*ast.FuncLit); ok { + return false + } + + if assign, ok := node.(*ast.AssignStmt); ok { + for i, rhs := range assign.Rhs { + call, ok := rhs.(*ast.CallExpr) + if !ok || !isContextWithCancelCall(call) { + continue + } + if len(assign.Rhs) == 1 && i == 0 && len(assign.Lhs) >= 2 { + ident, ok := assign.Lhs[1].(*ast.Ident) + if !ok || ident.Name == "_" { + continue + } + obj := pass.TypesInfo.ObjectOf(ident) + if obj == nil { + continue + } + if prev, exists := cancelVars[obj]; exists && prev.hasDirectCancel && !prev.hasDeferCancel { + pass.Report(analysis.Diagnostic{ + Pos: prev.createPos, + Message: "context cancel function should be deferred immediately after context.WithCancel/WithTimeout/WithDeadline", + }) + } + cancelVars[obj] = &cancelVarState{createPos: call.Pos()} + } + } + } + + if deferStmt, ok := node.(*ast.DeferStmt); ok { + if obj := getCancelCallObj(pass, deferStmt.Call); obj != nil { + if state, found := cancelVars[obj]; found { + state.hasDeferCancel = true + } + } + } + + if exprStmt, ok := node.(*ast.ExprStmt); ok { + if call, ok := exprStmt.X.(*ast.CallExpr); ok { + if obj := getCancelCallObj(pass, call); obj != nil { + if state, found := cancelVars[obj]; found { + state.hasDirectCancel = true + } + } + } + } + + return true + }) + + for _, state := range cancelVars { + if state.hasDirectCancel && !state.hasDeferCancel { + pass.Report(analysis.Diagnostic{ + Pos: state.createPos, + Message: "context cancel function should be deferred immediately after context.WithCancel/WithTimeout/WithDeadline", + }) + } + } + }) + + return nil, nil +} + +type cancelVarState struct { + createPos token.Pos + hasDeferCancel bool + hasDirectCancel bool +} + +func isContextWithCancelCall(call *ast.CallExpr) bool { + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return false + } + pkgIdent, ok := sel.X.(*ast.Ident) + if !ok || pkgIdent.Name != "context" { + return false + } + switch sel.Sel.Name { + case "WithCancel", "WithTimeout", "WithDeadline": + return true + default: + return false + } +} + +func getCancelCallObj(pass *analysis.Pass, call *ast.CallExpr) types.Object { + ident, ok := call.Fun.(*ast.Ident) + if !ok { + return nil + } + return pass.TypesInfo.ObjectOf(ident) +} diff --git a/pkg/linters/contextcancelnotdeferred/contextcancelnotdeferred_test.go b/pkg/linters/contextcancelnotdeferred/contextcancelnotdeferred_test.go new file mode 100644 index 00000000000..9888d890b36 --- /dev/null +++ b/pkg/linters/contextcancelnotdeferred/contextcancelnotdeferred_test.go @@ -0,0 +1,16 @@ +//go:build !integration + +package contextcancelnotdeferred_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/github/gh-aw/pkg/linters/contextcancelnotdeferred" +) + +func TestContextCancelNotDeferred(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, contextcancelnotdeferred.Analyzer, "contextcancelnotdeferred") +} diff --git a/pkg/linters/contextcancelnotdeferred/testdata/src/contextcancelnotdeferred/contextcancelnotdeferred.go b/pkg/linters/contextcancelnotdeferred/testdata/src/contextcancelnotdeferred/contextcancelnotdeferred.go new file mode 100644 index 00000000000..621aceae7c9 --- /dev/null +++ b/pkg/linters/contextcancelnotdeferred/testdata/src/contextcancelnotdeferred/contextcancelnotdeferred.go @@ -0,0 +1,56 @@ +package contextcancelnotdeferred + +import ( + "context" + "time" +) + +func BadWithTimeout(parent context.Context) error { + ctx, cancel := context.WithTimeout(parent, time.Second) // want `context cancel function should be deferred immediately after context.WithCancel/WithTimeout/WithDeadline` + _ = ctx + cancel() + return nil +} + +func BadWithCancel(parent context.Context) error { + ctx, cancel := context.WithCancel(parent) // want `context cancel function should be deferred immediately after context.WithCancel/WithTimeout/WithDeadline` + _ = ctx + cancel() + return nil +} + +func GoodWithDeadline(parent context.Context, d time.Time) error { + ctx, cancel := context.WithDeadline(parent, d) + defer cancel() + _ = ctx + return nil +} + +func GoodInClosure(parent context.Context) { + ctx, cancel := context.WithTimeout(parent, time.Second) + defer cancel() + _ = ctx + + fn := func() error { + innerCtx, cancel := context.WithTimeout(parent, time.Second) + defer cancel() + _ = innerCtx + return nil + } + _ = fn() +} + +func BadReassignThenGood(parent context.Context) error { + ctx, cancel := context.WithTimeout(parent, time.Second) // want `context cancel function should be deferred immediately after context.WithCancel/WithTimeout/WithDeadline` + _ = ctx + cancel() + + ctx, cancel = context.WithTimeout(parent, time.Second) + defer cancel() + _ = ctx + return nil +} + +func BlankIdentifier(parent context.Context) { + _, _ = context.WithTimeout(parent, time.Second) +}