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
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package timesleepnocontext

import (
"context"
"net/http"
"time"
)

Expand Down Expand Up @@ -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`
}()
Comment thread
pelikhan marked this conversation as resolved.
_ = 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
Expand Down
52 changes: 51 additions & 1 deletion pkg/linters/timesleepnocontext/timesleepnocontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand All @@ -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)
Expand Down
Loading