diff --git a/pkg/linters/timesleepnocontext/testdata/src/timesleepnocontext/timesleepnocontext.go b/pkg/linters/timesleepnocontext/testdata/src/timesleepnocontext/timesleepnocontext.go index e6be802d097..184f1f97e0d 100644 --- a/pkg/linters/timesleepnocontext/testdata/src/timesleepnocontext/timesleepnocontext.go +++ b/pkg/linters/timesleepnocontext/testdata/src/timesleepnocontext/timesleepnocontext.go @@ -2,6 +2,7 @@ package timesleepnocontext import ( "context" + "net/http" "time" ) @@ -58,6 +59,53 @@ func doWork(fn func(context.Context, time.Duration)) { fn(context.Background(), time.Second) } +// Good: callback context source is request-scoped, not the outer registration context. +func GoodHTTPHandleFuncCallbackInCtxFunc(ctx context.Context, d time.Duration) { + mux := http.NewServeMux() + mux.HandleFunc("/wait", func(w http.ResponseWriter, r *http.Request) { + _ = w + _ = r + time.Sleep(d) + }) + _ = ctx +} + +// Good: ordinary synchronous callback closure should not be attributed to outer ctx. +func GoodSyncCallbackInCtxFunc(ctx context.Context, d time.Duration) { + register(func() { + time.Sleep(d) + }) + _ = ctx +} + +// Bad: deferred closure shares outer context lifetime. +func BadDeferWithCtx(ctx context.Context, d time.Duration) { + defer func() { + time.Sleep(d) // want `use select with ctx\.Done\(\) instead of time\.Sleep to allow context cancellation` + }() + _ = ctx +} + +// Bad: parenthesized deferred closure shares outer context lifetime. +func BadDeferParenWithCtx(ctx context.Context, d time.Duration) { + defer (func() { + time.Sleep(d) // want `use select with ctx\.Done\(\) instead of time\.Sleep to allow context cancellation` + })() + _ = ctx +} + +// Bad: parenthesized goroutine closure shares outer context lifetime. +func BadGoParenWithCtx(ctx context.Context, d time.Duration) { + go (func() { + time.Sleep(d) // want `use select with ctx\.Done\(\) instead of time\.Sleep to allow context cancellation` + })() + _ = ctx +} + +func register(fn func()) { + fn() +} + // Good: inline nolint suppresses intentional sleep. func GoodNoLint(ctx context.Context, d time.Duration) { _ = ctx diff --git a/pkg/linters/timesleepnocontext/timesleepnocontext.go b/pkg/linters/timesleepnocontext/timesleepnocontext.go index d098895a3fa..aca6183807d 100644 --- a/pkg/linters/timesleepnocontext/timesleepnocontext.go +++ b/pkg/linters/timesleepnocontext/timesleepnocontext.go @@ -11,6 +11,7 @@ import ( "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/astutil" "github.com/github/gh-aw/pkg/linters/internal/filecheck" @@ -51,12 +52,16 @@ func run(pass *analysis.Pass) (any, error) { } for encl := range cur.Enclosing((*ast.FuncDecl)(nil), (*ast.FuncLit)(nil)) { - funcType := enclosingFuncType(encl.Node()) + funcNode := encl.Node() + funcType := enclosingFuncType(funcNode) if funcType == nil { continue } ctxParamName, hasCtx := contextParamName(pass, funcType) if !hasCtx { + if _, isFuncLit := funcNode.(*ast.FuncLit); isFuncLit && !isGoOrDeferClosure(encl) { + break + } continue } pass.Report(analysis.Diagnostic{ @@ -71,6 +76,51 @@ func run(pass *analysis.Pass) (any, error) { return nil, nil } +func isGoOrDeferClosure(funcLitCur inspector.Cursor) bool { + // Walk up from the FuncLit, unwrapping any ParenExpr wrappers, to find the + // enclosing CallExpr. This handles parenthesized forms like defer (func(){})(). + cur := funcLitCur.Parent() + for { + if cur.Node() == nil { + return false + } + if _, ok := cur.Node().(*ast.ParenExpr); ok { + cur = cur.Parent() + continue + } + break + } + + call, ok := cur.Node().(*ast.CallExpr) + if !ok { + return false + } + // Unwrap ParenExpr from call.Fun and verify it resolves to our FuncLit. + callee := call.Fun + for { + if paren, ok := callee.(*ast.ParenExpr); ok { + callee = paren.X + } else { + break + } + } + if callee != funcLitCur.Node() { + return false + } + + grandparent := cur.Parent().Node() + if grandparent == nil { + return false + } + + switch grandparent.(type) { + case *ast.GoStmt, *ast.DeferStmt: + return true + default: + return false + } +} + // isTimeSleepCall reports whether call is a call to time.Sleep. func isTimeSleepCall(pass *analysis.Pass, call *ast.CallExpr) bool { sel, ok := call.Fun.(*ast.SelectorExpr)