[linter-miner] linter: add timeafterleak — flag time.After in for+select cases

cmd/linters/main.go

@@ -40,6 +40,7 @@ import (
 	"github.com/github/gh-aw/pkg/linters/sortslice"
 	"github.com/github/gh-aw/pkg/linters/ssljson"
 	"github.com/github/gh-aw/pkg/linters/strconvparseignorederror"
+	"github.com/github/gh-aw/pkg/linters/timeafterleak"
 	"github.com/github/gh-aw/pkg/linters/timesleepnocontext"
 	"github.com/github/gh-aw/pkg/linters/tolowerequalfold"
 	"github.com/github/gh-aw/pkg/linters/uncheckedtypeassertion"
@@ -71,6 +72,7 @@ func main() {
 		strconvparseignorederror.Analyzer,
 		jsonmarshalignoredeerror.Analyzer,
 		lenstringzero.Analyzer,
+		timeafterleak.Analyzer,
 		timesleepnocontext.Analyzer,
 		tolowerequalfold.Analyzer,
 		uncheckedtypeassertion.Analyzer,

pkg/linters/timeafterleak/testdata/src/timeafterleak/timeafterleak.go

@@ -0,0 +1,102 @@
+package timeafterleak
+
+import (
+	"context"
+	"time"
+)
+
+// BadForLoop is the canonical timer-leak: time.After in a for+select Comm.
+func BadForLoop(ctx context.Context) {
+	for {
+		select {
+		case <-time.After(time.Second): // want `time\.After creates a new timer on each loop iteration`
+			doWork()
+		case <-ctx.Done():
+			return
+		}
+	}
+}
+
+// BadForLoopAssign also leaks: the receive is the Comm of the case clause.
+func BadForLoopAssign(ctx context.Context) {
+	for {
+		select {
+		case t := <-time.After(time.Second): // want `time\.After creates a new timer on each loop iteration`
+			_ = t
+		case <-ctx.Done():
+			return
+		}
+	}
+}
+
+// BadRangeLoop flags time.After in a range-based loop select.
+func BadRangeLoop(items []string, ctx context.Context) {
+	for range items {
+		select {
+		case <-time.After(time.Millisecond): // want `time\.After creates a new timer on each loop iteration`
+		case <-ctx.Done():
+			return
+		}
+	}
+}
+
+// GoodNoLoop is fine: time.After in a select that is not inside a loop.
+func GoodNoLoop(ctx context.Context) {
+	select {
+	case <-time.After(time.Second):
+		doWork()
+	case <-ctx.Done():
+		return
+	}
+}
+
+// GoodNewTimer uses the correct pattern: a single timer reused each iteration.
+func GoodNewTimer(ctx context.Context) {
+	t := time.NewTimer(time.Second)
+	defer t.Stop()
+	for {
+		if !t.Stop() {
+			select {
+			case <-t.C:
+			default:
+			}
+		}
+		t.Reset(time.Second)
+		select {
+		case <-t.C:
+			doWork()
+		case <-ctx.Done():
+			return
+		}
+	}
+}
+
+// GoodTimeAfterInBody calls time.After inside the case body, not as the Comm.
+// The timer fires exactly once here, so there is no accumulation leak.
+func GoodTimeAfterInBody(ctx context.Context, ch <-chan struct{}) {
+	for {
+		select {
+		case <-ch:
+			<-time.After(time.Second) // in Body, not Comm — not flagged
+		case <-ctx.Done():
+			return
+		}
+	}
+}
+
+// GoodFuncLitInsideLoop: the select is inside a goroutine closure launched
+// per iteration; the for loop does not directly enclose the CommClause.
+func GoodFuncLitInsideLoop(ctx context.Context) {
+	for {
+		go func() {
+			select {
+			case <-time.After(time.Second): // FuncLit boundary — not flagged
+			case <-ctx.Done():
+			}
+		}()
+		<-ctx.Done()
+		return
+	}
+}
+
+func doWork() {}

pkg/linters/timeafterleak/timeafterleak.go

@@ -0,0 +1,134 @@
+// Package timeafterleak implements a Go analysis linter that flags
+// time.After calls used as select case channel receives inside loops,
+// which allocate a new timer on every iteration that is not garbage
+// collected until it fires when another case is selected first.
+package timeafterleak
+
+import (
+	"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/astutil"
+	"github.com/github/gh-aw/pkg/linters/internal/filecheck"
+	"github.com/github/gh-aw/pkg/linters/internal/nolint"
+)
+
+// Analyzer is the time-after-leak analysis pass.
+var Analyzer = &analysis.Analyzer{
+	Name:     "timeafterleak",
+	Doc:      "reports time.After calls in select cases inside loops that leak a timer goroutine on each iteration when another case fires first",
+	URL:      "https://github.com/github/gh-aw/tree/main/pkg/linters/timeafterleak",
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      run,
+}
+
+func run(pass *analysis.Pass) (any, error) {
+	insp, err := astutil.Inspector(pass)
+	if err != nil {
+		return nil, err
+	}
+	noLintLinesByFile := nolint.BuildLineIndex(pass, "timeafterleak")
+
+	for cur := range insp.Root().Preorder((*ast.CallExpr)(nil)) {
+		call, ok := cur.Node().(*ast.CallExpr)
+		if !ok {
+			continue
+		}
+		if !isTimeAfterCall(pass, call) {
+			continue
+		}
+
+		pos := pass.Fset.PositionFor(call.Pos(), false)
+		if filecheck.IsTestFile(pos.Filename) {
+			continue
+		}
+		if nolint.HasDirective(pos, noLintLinesByFile) {
+			continue
+		}
+
+		if !isInsideLoopSelectComm(cur) {
+			continue
+		}
+
+		pass.ReportRangef(call,
+			"time.After creates a new timer on each loop iteration that is not garbage collected until it fires; use time.NewTimer with Reset and Stop instead")
+	}
+
+	return nil, nil
+}
+
+// isTimeAfterCall reports whether call is an invocation of time.After.
+func isTimeAfterCall(pass *analysis.Pass, call *ast.CallExpr) bool {
+	sel, ok := call.Fun.(*ast.SelectorExpr)
+	if !ok || sel.Sel.Name != "After" {
+		return false
+	}
+	ident, ok := sel.X.(*ast.Ident)
+	if !ok {
+		return false
+	}
+	obj := pass.TypesInfo.ObjectOf(ident)
+	if obj == nil {
+		return false
+	}
+	pkgName, ok := obj.(*types.PkgName)
+	if !ok {
+		return false
+	}
+	return pkgName.Imported().Path() == "time"
+}
+
+// isInsideLoopSelectComm reports whether cur is a time.After call used as the
+// channel receive expression in the Comm field of a select CommClause that is
+// enclosed by a for or range loop, without crossing a function literal boundary.
+func isInsideLoopSelectComm(cur inspector.Cursor) bool {
+	// The immediate parent of time.After(...) must be a channel-receive UnaryExpr.
+	recvCur := cur.Parent()
+	unary, ok := recvCur.Node().(*ast.UnaryExpr)
+	if !ok || unary.Op != token.ARROW {
+		return false
+	}
+
+	// The parent of the receive expression must be the Comm statement of a CommClause.
+	// Comm is an ExprStmt (case <-ch:) or AssignStmt (case v := <-ch:).
+	commStmtCur := recvCur.Parent()
+	var commStmt ast.Stmt
+	switch s := commStmtCur.Node().(type) {
+	case *ast.ExprStmt:
+		commStmt = s
+	case *ast.AssignStmt:
+		commStmt = s
+	default:
+		return false
+	}
+
+	// The parent of the Comm statement must be a CommClause, and commStmt must
+	// be the Comm field (not a statement in the Body).
+	clauseCur := commStmtCur.Parent()
+	cc, ok := clauseCur.Node().(*ast.CommClause)
+	if !ok || cc.Comm != commStmt {
+		return false
+	}
+
+	// Walk up from the CommClause to find an enclosing for or range loop,
+	// stopping at any function literal or declaration boundary.
+	for encl := range clauseCur.Enclosing(
+		(*ast.ForStmt)(nil),
+		(*ast.RangeStmt)(nil),
+		(*ast.FuncLit)(nil),
+		(*ast.FuncDecl)(nil),
+	) {
+		switch encl.Node().(type) {
+		case *ast.ForStmt, *ast.RangeStmt:
+			return true
+		case *ast.FuncLit, *ast.FuncDecl:
+			return false
+		}
+	}
+	return false
+}

pkg/linters/timeafterleak/timeafterleak_test.go

@@ -0,0 +1,16 @@
+//go:build !integration
+
+package timeafterleak_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+
+	"github.com/github/gh-aw/pkg/linters/timeafterleak"
+)
+
+func TestAnalyzer(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, timeafterleak.Analyzer, "timeafterleak")
+}