From 229fe2fe92e1c40f7c2bc22f37524dfc88ce1fa6 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 19 Jul 2026 17:52:15 +0000 Subject: [PATCH 1/4] feat(linters): add timenowsub linter Flags time.Now().Sub(t) calls that can be simplified to time.Since(t). The time.Since function is the idiomatic Go way to compute the elapsed time since a fixed point and avoids the more verbose two-step pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cmd/linters/main.go | 2 + .../testdata/src/timenowsub/timenowsub.go | 19 +++ .../src/timenowsub/timenowsub.go.golden | 19 +++ pkg/linters/timenowsub/timenowsub.go | 118 ++++++++++++++++++ pkg/linters/timenowsub/timenowsub_test.go | 16 +++ 5 files changed, 174 insertions(+) create mode 100644 pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go create mode 100644 pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go.golden create mode 100644 pkg/linters/timenowsub/timenowsub.go create mode 100644 pkg/linters/timenowsub/timenowsub_test.go diff --git a/cmd/linters/main.go b/cmd/linters/main.go index b9434a85dc6..218ee04423d 100644 --- a/cmd/linters/main.go +++ b/cmd/linters/main.go @@ -63,6 +63,7 @@ import ( "github.com/github/gh-aw/pkg/linters/stringscountcontains" "github.com/github/gh-aw/pkg/linters/stringsindexcontains" "github.com/github/gh-aw/pkg/linters/timeafterleak" + "github.com/github/gh-aw/pkg/linters/timenowsub" "github.com/github/gh-aw/pkg/linters/timesleepnocontext" "github.com/github/gh-aw/pkg/linters/tolowerequalfold" "github.com/github/gh-aw/pkg/linters/trimleftright" @@ -121,6 +122,7 @@ func main() { lenstringsplit.Analyzer, timeafterleak.Analyzer, timesleepnocontext.Analyzer, + timenowsub.Analyzer, tolowerequalfold.Analyzer, trimleftright.Analyzer, uncheckedtypeassertion.Analyzer, diff --git a/pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go b/pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go new file mode 100644 index 00000000000..90a50875d8d --- /dev/null +++ b/pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go @@ -0,0 +1,19 @@ +package timenowsub + +import "time" + +func bad(t time.Time) { + _ = time.Now().Sub(t) // want `time\.Now\(\)\.Sub\(t\) can be simplified to time\.Since\(t\)` +} + +func badAssign(start time.Time) time.Duration { + return time.Now().Sub(start) // want `time\.Now\(\)\.Sub\(start\) can be simplified to time\.Since\(start\)` +} + +func good(t time.Time) { + _ = time.Since(t) +} + +func goodOtherSub(a, b time.Time) { + _ = a.Sub(b) +} diff --git a/pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go.golden b/pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go.golden new file mode 100644 index 00000000000..7cdeefdec63 --- /dev/null +++ b/pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go.golden @@ -0,0 +1,19 @@ +package timenowsub + +import "time" + +func bad(t time.Time) { + _ = time.Since(t) // want `time\.Now\(\)\.Sub\(t\) can be simplified to time\.Since\(t\)` +} + +func badAssign(start time.Time) time.Duration { + return time.Since(start) // want `time\.Now\(\)\.Sub\(start\) can be simplified to time\.Since\(start\)` +} + +func good(t time.Time) { + _ = time.Since(t) +} + +func goodOtherSub(a, b time.Time) { + _ = a.Sub(b) +} diff --git a/pkg/linters/timenowsub/timenowsub.go b/pkg/linters/timenowsub/timenowsub.go new file mode 100644 index 00000000000..1d314ed4a8f --- /dev/null +++ b/pkg/linters/timenowsub/timenowsub.go @@ -0,0 +1,118 @@ +// Package timenowsub implements a Go analysis linter that flags +// time.Now().Sub(t) calls that can be simplified to time.Since(t). +package timenowsub + +import ( + "fmt" + "go/ast" + "go/types" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + + "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-now-sub analysis pass. +var Analyzer = &analysis.Analyzer{ + Name: "timenowsub", + Doc: "reports time.Now().Sub(t) calls that should be simplified to time.Since(t)", + URL: "https://github.com/github/gh-aw/tree/main/pkg/linters/timenowsub", + Requires: []*analysis.Analyzer{inspect.Analyzer, nolint.Analyzer, filecheck.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (any, error) { + insp, err := astutil.Inspector(pass) + if err != nil { + return nil, err + } + noLintIndex, err := nolint.Index(pass) + if err != nil { + return nil, err + } + generatedFiles, err := filecheck.Index(pass) + if err != nil { + return nil, err + } + + nodeFilter := []ast.Node{ + (*ast.CallExpr)(nil), + } + + insp.Preorder(nodeFilter, func(n ast.Node) { + outer, ok := n.(*ast.CallExpr) + if !ok { + return + } + + // Match .Sub() where is time.Now(). + sel, ok := outer.Fun.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "Sub" { + return + } + if len(outer.Args) != 1 { + return + } + + // Verify the receiver is a call to time.Now(). + nowCall, ok := sel.X.(*ast.CallExpr) + if !ok { + return + } + if !isTimeNow(pass, nowCall) { + return + } + + pos := pass.Fset.PositionFor(outer.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return + } + if nolint.HasDirectiveForLinter(pos, noLintIndex, "timenowsub") { + return + } + + argText := astutil.NodeText(pass.Fset, outer.Args[0]) + if argText == "" { + return + } + + pass.Report(analysis.Diagnostic{ + Pos: outer.Pos(), + End: outer.End(), + Message: fmt.Sprintf("time.Now().Sub(%s) can be simplified to time.Since(%s)", argText, argText), + SuggestedFixes: []analysis.SuggestedFix{{ + Message: fmt.Sprintf("Replace time.Now().Sub(%s) with time.Since(%s)", argText, argText), + TextEdits: []analysis.TextEdit{{ + Pos: outer.Pos(), + End: outer.End(), + NewText: []byte(fmt.Sprintf("time.Since(%s)", argText)), + }}, + }}, + }) + }) + + return nil, nil +} + +// isTimeNow reports whether call is a call to time.Now(). +func isTimeNow(pass *analysis.Pass, call *ast.CallExpr) bool { + if len(call.Args) != 0 { + return false + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "Now" { + return false + } + obj := pass.TypesInfo.ObjectOf(sel.Sel) + if obj == nil { + return false + } + fn, ok := obj.(*types.Func) + if !ok { + return false + } + return fn.FullName() == "time.Now" +} diff --git a/pkg/linters/timenowsub/timenowsub_test.go b/pkg/linters/timenowsub/timenowsub_test.go new file mode 100644 index 00000000000..5d69d501e26 --- /dev/null +++ b/pkg/linters/timenowsub/timenowsub_test.go @@ -0,0 +1,16 @@ +//go:build !integration + +package timenowsub_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/github/gh-aw/pkg/linters/timenowsub" +) + +func TestAnalyzer(t *testing.T) { + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, timenowsub.Analyzer, "timenowsub") +} From 77899512136ed6046cb74171e78e032ead9bb45f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 19 Jul 2026 18:26:19 +0000 Subject: [PATCH 2/4] docs(adr): add draft ADR-46633 for timenowsub linter Co-Authored-By: Claude Sonnet 4.6 --- docs/adr/46633-add-timenowsub-linter.md | 49 +++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 docs/adr/46633-add-timenowsub-linter.md diff --git a/docs/adr/46633-add-timenowsub-linter.md b/docs/adr/46633-add-timenowsub-linter.md new file mode 100644 index 00000000000..f9d081de633 --- /dev/null +++ b/docs/adr/46633-add-timenowsub-linter.md @@ -0,0 +1,49 @@ +# ADR-46633: Add timenowsub Custom Linter for time.Now().Sub(t) → time.Since(t) + +**Date**: 2026-07-19 +**Status**: Draft +**Deciders**: Unknown + +--- + +### Context + +The repository maintains a collection of custom Go static analysis linters (under `pkg/linters/`) enforced via `cmd/linters/main.go`. A static scan of `pkg/` and `cmd/` identified occurrences of the verbose `time.Now().Sub(t)` pattern, which has a direct idiomatic replacement: `time.Since(t)`. Go's standard library documents `time.Since(t)` as shorthand for `time.Now().Sub(t)` (see [Go issue #16351](https://github.com/golang/go/issues/16351)), and this simplification is consistently flagged in Go code reviews. The pattern produces zero false positives since every `time.Now().Sub(x)` call has an exact mechanical replacement. + +### Decision + +We will add a new custom `timenowsub` analyzer to `pkg/linters/timenowsub/` and register it in `cmd/linters/main.go`. The analyzer uses `golang.org/x/tools/go/analysis` to detect AST nodes matching `time.Now().Sub()` and reports a diagnostic with a `SuggestedFix` rewriting them to `time.Since()`. Generated files and files with `//nolint:timenowsub` directives are skipped. + +### Alternatives Considered + +#### Alternative 1: Enable gosimple (S1012) from golangci-lint / staticcheck + +`gosimple` check S1012 already covers this exact pattern. Enabling it from the broader `golangci-lint` / `staticcheck` toolchain would address the issue without writing custom code. + +This was not chosen because: the repository's custom linter framework provides consistent enforcement, nolint-directive handling, generated-file skipping, and test infrastructure that the generic `gosimple` integration does not supply out of the box. Relying on `gosimple` also requires maintaining the `golangci-lint` configuration and ensuring S1012 is not accidentally disabled, whereas a custom linter is unconditionally active. + +#### Alternative 2: Rely on code review without automated enforcement + +Reviewers could flag `time.Now().Sub(t)` patterns manually during PR review without any tooling. + +This was not chosen because: manual code review is inconsistent and does not scale — patterns are missed, especially in large diffs. The zero-false-positive nature of this check makes automated enforcement strictly better than human review for this specific pattern. + +### Consequences + +#### Positive +- Zero false positives: every flagged `time.Now().Sub(x)` call has a safe mechanical replacement. +- Automatic fix: the `SuggestedFix` in the diagnostic allows `gopls` and `go fix`-style tools to apply the rewrite with no manual intervention. +- Unconditional enforcement: the linter is always active regardless of golangci-lint configuration changes. +- Consistent with existing linter patterns: follows the same structure as other custom linters in `pkg/linters/`. + +#### Negative +- New package to maintain: adds `pkg/linters/timenowsub/` to the custom linter collection, which must be updated if internal shared utilities (e.g., `astutil`, `filecheck`, `nolint`) change their APIs. +- Slightly increases binary size of the `cmd/linters` tool. + +#### Neutral +- The linter only fires on `time.Now().Sub(x)` where the receiver is verified via type-checker to be `time.Now` — other `.Sub()` calls (e.g., `a.Sub(b)`) are unaffected. +- Test coverage is provided via `analysistest.RunWithSuggestedFixes` and a golden file, following the established test pattern for this linter suite. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* From 2b842a421cdd0f65b9b310e55969583a6389c6af Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 19 Jul 2026 19:33:24 +0000 Subject: [PATCH 3/4] fix(linters): avoid fmt.Sprintf in timenowsub fix text Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/linters/timenowsub/timenowsub.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/linters/timenowsub/timenowsub.go b/pkg/linters/timenowsub/timenowsub.go index 1d314ed4a8f..d49c4511f64 100644 --- a/pkg/linters/timenowsub/timenowsub.go +++ b/pkg/linters/timenowsub/timenowsub.go @@ -88,7 +88,7 @@ func run(pass *analysis.Pass) (any, error) { TextEdits: []analysis.TextEdit{{ Pos: outer.Pos(), End: outer.End(), - NewText: []byte(fmt.Sprintf("time.Since(%s)", argText)), + NewText: []byte("time.Since(" + argText + ")"), }}, }}, }) From 9f2bd60eeaa5d9c905cfa25dd65259697d7e2280 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 19 Jul 2026 20:19:11 +0000 Subject: [PATCH 4/4] fix(linters): make timenowsub rewrite conservative Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../testdata/src/faketime/time/time.go | 13 ++++++ .../testdata/src/timenowsub/alias.go | 7 +++ .../testdata/src/timenowsub/alias.go.golden | 7 +++ .../testdata/src/timenowsub/timenowsub.go | 17 ++++++- .../src/timenowsub/timenowsub.go.golden | 17 ++++++- pkg/linters/timenowsub/timenowsub.go | 46 ++++++++++++++----- 6 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 pkg/linters/timenowsub/testdata/src/faketime/time/time.go create mode 100644 pkg/linters/timenowsub/testdata/src/timenowsub/alias.go create mode 100644 pkg/linters/timenowsub/testdata/src/timenowsub/alias.go.golden diff --git a/pkg/linters/timenowsub/testdata/src/faketime/time/time.go b/pkg/linters/timenowsub/testdata/src/faketime/time/time.go new file mode 100644 index 00000000000..a052695221b --- /dev/null +++ b/pkg/linters/timenowsub/testdata/src/faketime/time/time.go @@ -0,0 +1,13 @@ +package time + +import stdtime "time" + +type Time struct{} + +func Now() Time { + return Time{} +} + +func (Time) Sub(Time) stdtime.Duration { + return 0 +} diff --git a/pkg/linters/timenowsub/testdata/src/timenowsub/alias.go b/pkg/linters/timenowsub/testdata/src/timenowsub/alias.go new file mode 100644 index 00000000000..b61dbe7d075 --- /dev/null +++ b/pkg/linters/timenowsub/testdata/src/timenowsub/alias.go @@ -0,0 +1,7 @@ +package timenowsub + +import clock "time" + +func badAlias(t clock.Time) { + _ = clock.Now().Sub(t) // want `clock\.Now\(\)\.Sub\(t\) can be simplified to clock\.Since\(t\)` +} diff --git a/pkg/linters/timenowsub/testdata/src/timenowsub/alias.go.golden b/pkg/linters/timenowsub/testdata/src/timenowsub/alias.go.golden new file mode 100644 index 00000000000..d4453c7e1b5 --- /dev/null +++ b/pkg/linters/timenowsub/testdata/src/timenowsub/alias.go.golden @@ -0,0 +1,7 @@ +package timenowsub + +import clock "time" + +func badAlias(t clock.Time) { + _ = clock.Since(t) // want `clock\.Now\(\)\.Sub\(t\) can be simplified to clock\.Since\(t\)` +} diff --git a/pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go b/pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go index 90a50875d8d..eff820e2715 100644 --- a/pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go +++ b/pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go @@ -1,6 +1,9 @@ package timenowsub -import "time" +import ( + faketime "faketime/time" + "time" +) func bad(t time.Time) { _ = time.Now().Sub(t) // want `time\.Now\(\)\.Sub\(t\) can be simplified to time\.Since\(t\)` @@ -17,3 +20,15 @@ func good(t time.Time) { func goodOtherSub(a, b time.Time) { _ = a.Sub(b) } + +func goodCallExprArg() { + _ = time.Now().Sub(loadStart()) +} + +func goodOtherTimePackage(t faketime.Time) { + _ = faketime.Now().Sub(t) +} + +func loadStart() time.Time { + return time.Now() +} diff --git a/pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go.golden b/pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go.golden index 7cdeefdec63..30efd30913e 100644 --- a/pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go.golden +++ b/pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go.golden @@ -1,6 +1,9 @@ package timenowsub -import "time" +import ( + faketime "faketime/time" + "time" +) func bad(t time.Time) { _ = time.Since(t) // want `time\.Now\(\)\.Sub\(t\) can be simplified to time\.Since\(t\)` @@ -17,3 +20,15 @@ func good(t time.Time) { func goodOtherSub(a, b time.Time) { _ = a.Sub(b) } + +func goodCallExprArg() { + _ = time.Now().Sub(loadStart()) +} + +func goodOtherTimePackage(t faketime.Time) { + _ = faketime.Now().Sub(t) +} + +func loadStart() time.Time { + return time.Now() +} diff --git a/pkg/linters/timenowsub/timenowsub.go b/pkg/linters/timenowsub/timenowsub.go index d49c4511f64..e0b247f41b1 100644 --- a/pkg/linters/timenowsub/timenowsub.go +++ b/pkg/linters/timenowsub/timenowsub.go @@ -62,7 +62,11 @@ func run(pass *analysis.Pass) (any, error) { if !ok { return } - if !isTimeNow(pass, nowCall) { + qualifier, ok := timeNowQualifier(pass, nowCall) + if !ok { + return + } + if !isSafeSinceArg(outer.Args[0]) { return } @@ -78,17 +82,18 @@ func run(pass *analysis.Pass) (any, error) { if argText == "" { return } + sinceText := qualifier + ".Since(" + argText + ")" pass.Report(analysis.Diagnostic{ Pos: outer.Pos(), End: outer.End(), - Message: fmt.Sprintf("time.Now().Sub(%s) can be simplified to time.Since(%s)", argText, argText), + Message: fmt.Sprintf("%s.Now().Sub(%s) can be simplified to %s", qualifier, argText, sinceText), SuggestedFixes: []analysis.SuggestedFix{{ - Message: fmt.Sprintf("Replace time.Now().Sub(%s) with time.Since(%s)", argText, argText), + Message: fmt.Sprintf("Replace %s.Now().Sub(%s) with %s", qualifier, argText, sinceText), TextEdits: []analysis.TextEdit{{ Pos: outer.Pos(), End: outer.End(), - NewText: []byte("time.Since(" + argText + ")"), + NewText: []byte(sinceText), }}, }}, }) @@ -97,22 +102,39 @@ func run(pass *analysis.Pass) (any, error) { return nil, nil } -// isTimeNow reports whether call is a call to time.Now(). -func isTimeNow(pass *analysis.Pass, call *ast.CallExpr) bool { +// timeNowQualifier reports the imported identifier used for time.Now(). +func timeNowQualifier(pass *analysis.Pass, call *ast.CallExpr) (string, bool) { if len(call.Args) != 0 { - return false + return "", false } sel, ok := call.Fun.(*ast.SelectorExpr) if !ok || sel.Sel.Name != "Now" { - return false + return "", false } - obj := pass.TypesInfo.ObjectOf(sel.Sel) + ident, ok := sel.X.(*ast.Ident) + if !ok { + return "", false + } + obj := pass.TypesInfo.ObjectOf(ident) if obj == nil { - return false + return "", false } - fn, ok := obj.(*types.Func) + pkgName, ok := obj.(*types.PkgName) if !ok { + return "", false + } + return ident.Name, pkgName.Imported().Path() == "time" +} + +// isSafeSinceArg reports whether expr can be evaluated before time.Now() +// without introducing calls or other potentially observable behavior changes. +func isSafeSinceArg(expr ast.Expr) bool { + switch e := expr.(type) { + case *ast.Ident: + return true + case *ast.ParenExpr: + return isSafeSinceArg(e.X) + default: return false } - return fn.FullName() == "time.Now" }