[linter-miner] feat(linters): add timenowsub linter — flag time.Now().Sub(t) → time.Since(t)

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,

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(<arg>)` and reports a diagnostic with a `SuggestedFix` rewriting them to `time.Since(<arg>)`. 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.*

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
+}

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\)`
+}

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\)`
+}

pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go

@@ -0,0 +1,34 @@
+package timenowsub
+
+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\)`
+}
+
+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)
+}
+
+func goodCallExprArg() {
+	_ = time.Now().Sub(loadStart())
+}
+
+func goodOtherTimePackage(t faketime.Time) {
+	_ = faketime.Now().Sub(t)
+}
+
+func loadStart() time.Time {
+	return time.Now()
+}

pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go.golden

@@ -0,0 +1,34 @@
+package timenowsub
+
+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\)`
+}
+
+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)
+}
+
+func goodCallExprArg() {
+	_ = time.Now().Sub(loadStart())
+}
+
+func goodOtherTimePackage(t faketime.Time) {
+	_ = faketime.Now().Sub(t)
+}
+
+func loadStart() time.Time {
+	return time.Now()
+}

pkg/linters/timenowsub/timenowsub.go

@@ -0,0 +1,140 @@
+// 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 <expr>.Sub(<arg>) where <expr> 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
+		}
+		qualifier, ok := timeNowQualifier(pass, nowCall)
+		if !ok {
+			return
+		}
+		if !isSafeSinceArg(outer.Args[0]) {
+			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
+		}
+		sinceText := qualifier + ".Since(" + argText + ")"
+
+		pass.Report(analysis.Diagnostic{
+			Pos:     outer.Pos(),
+			End:     outer.End(),
+			Message: fmt.Sprintf("%s.Now().Sub(%s) can be simplified to %s", qualifier, argText, sinceText),
+			SuggestedFixes: []analysis.SuggestedFix{{
+				Message: fmt.Sprintf("Replace %s.Now().Sub(%s) with %s", qualifier, argText, sinceText),
+				TextEdits: []analysis.TextEdit{{
+					Pos:     outer.Pos(),
+					End:     outer.End(),
+					NewText: []byte(sinceText),
+				}},
+			}},
+		})
+	})
+
+	return nil, nil
+}
+
+// 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
+	}
+	sel, ok := call.Fun.(*ast.SelectorExpr)
+	if !ok || sel.Sel.Name != "Now" {
+		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 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
+	}
+}

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")
+}