Add coverage-aware perf gating for allocation-focused custom linters

.github/skills/go-linters/SKILL.md

@@ -26,3 +26,45 @@ For PR-driven linter generation (derive a rule from a specific pull request patt
   - `make golint-custom`
 
 `make golint-custom` builds `cmd/linters` and runs it against `./cmd/...` and `./pkg/...`.
+
+## Coverage-aware perf gating
+
+For linters that flag micro-optimizations (allocation/perf rules), only apply them on lines that
+tests actually exercise — "hot paths" — rather than on dead or rarely-executed code where the
+optimization brings no measurable benefit. Use the shared `pkg/linters/internal/coverage` package:
+
+1. In your analyzer file, register a `-hot-threshold` flag in `init()` (not as a var initializer,
+   to avoid an `Analyzer`/`run`/flag initialization cycle):
+
+   ```go
+   var hotThreshold *int
+
+   func init() {
+       hotThreshold = coverage.RegisterHotThresholdFlag(Analyzer)
+   }
+   ```
+
+2. Immediately before reporting a diagnostic, gate it with `coverage.ShouldApply`:
+
+   ```go
+   if !coverage.ShouldApply(pass, node.Pos(), *hotThreshold) {
+       return
+   }
+   ```
+
+`coverage.ShouldApply` is permissive by default: when no coverage profile is loaded via the
+`GH_AW_LINT_COVERAGE_PROFILE` environment variable, or when `hot-threshold` is `0`, it always
+returns `true`, preserving pre-coverage-aware behavior. Only wire this into linters whose fix has
+a genuine performance rationale (extra allocations, O(n²) behavior, etc.) — purely
+readability/style linters should not be coverage-gated.
+
+### Generating the coverage profile
+
+```bash
+go test -covermode=count -coverprofile=/tmp/coverage.out ./...
+export GH_AW_LINT_COVERAGE_PROFILE=/tmp/coverage.out
+make golint-custom
+```
+
+This profile is read once per linter-runner process. To lint only a specific subtree, scope
+the `go test` and `golint-custom` commands to the same package path.

.gitignore

@@ -75,7 +75,7 @@ docs/public/wasm/wasm_exec.js
 .env.*.backup
 
 coverage.html
-coverage/
+/coverage/
 logs/
 
 # Benchmark results

docs/adr/51573-coverage-aware-gating-for-perf-linters.md

@@ -0,0 +1,45 @@
+# ADR-51573: Coverage-Aware Gating for Performance-Oriented Custom Linters
+
+**Date**: 2026-08-09
+**Status**: Accepted
+**Deciders**: pelikhan, copilot-swe-agent
+
+---
+
+### Context
+
+The repository maintains a suite of custom Go static-analysis linters. A subset of these linters flag micro-optimizations that only matter on hot paths — code that executes frequently under real workloads. Examples include `stringsconcatloop` (O(n²) string concatenation), `appendbytestring`, `bytesbufferstring`, `seenmapbool`, and 9 others. Applying these rules uniformly to all code — including dead code or paths that tests never reach — generates review noise without a measurable performance payoff. At the same time, purely stylistic linters (e.g. `stringsindexcontains`, `lenstringzero`) carry no performance benefit regardless of execution frequency, so gating them on coverage would be misleading.
+
+### Decision
+
+We will add a shared `pkg/linters/internal/coverage` package that loads a Go coverage profile (produced by `go test -covermode=count -coverprofile=<path>`, referenced via the `GH_AW_LINT_COVERAGE_PROFILE` environment variable) and exposes two helpers: `ShouldApply(pass, pos, threshold)` to gate a diagnostic on the line's recorded execution hit count, and `RegisterHotThresholdFlag(analyzer)` to register a per-linter `-hot-threshold` flag. All 13 allocation/perf-oriented linters will integrate this mechanism via an `init()` function to avoid analyzer-initialization cycles. When no profile is loaded (the default), gating is a no-op and all linters behave exactly as before; purely stylistic linters are left ungated.
+
+### Alternatives Considered
+
+#### Alternative 1: Uniform application (status quo)
+
+Continue running all linters on all code regardless of coverage. This is simpler — no new package, no env-var convention, no per-linter flag — but produces diagnostic noise on dead or rarely-executed code paths, leading to review churn with no measurable performance return.
+
+#### Alternative 2: Per-site `nolint` suppression
+
+Require developers to suppress false-positive perf diagnostics on a case-by-case basis with `//nolint:stringsconcatloop` (or similar) comments. This keeps the linter infrastructure simple but places the burden on individual contributors each time a new cold-path finding appears, and does not scale as the codebase or linter set grows.
+
+### Consequences
+
+#### Positive
+- Perf linters fire only on code paths that tests actually exercise, eliminating diagnostic noise on dead or rarely-executed code.
+- Fully permissive fallback: existing CI pipelines that do not set `GH_AW_LINT_COVERAGE_PROFILE` see no behavioral change.
+- The `-hot-threshold` flag gives per-linter control; passing `0` disables coverage gating for a specific linter even when a profile is present.
+- A documented, reusable pattern (`init()` + `coverage.RegisterHotThresholdFlag` + `coverage.ShouldApply`) makes it straightforward to wire future perf linters into the same mechanism.
+
+#### Negative
+- Activating coverage gating requires generating a coverage profile (`go test -covermode=count`) and setting `GH_AW_LINT_COVERAGE_PROFILE` in the lint environment; CI/CD pipeline configuration changes may be needed to realize the benefit.
+- Two behavioral modes (gated vs. ungated) exist per perf linter, increasing the number of states to reason about when debugging a linter that is unexpectedly silent.
+
+#### Neutral
+- Purely stylistic/readability linters (`stringsindexcontains`, `stringsindexhasprefix`, `stringscountcontains`, `lenstringzero`, etc.) are intentionally excluded from coverage gating, which means the two linter categories now diverge in their configuration surface.
+- The `init()` pattern (rather than a `var` initializer) is required to avoid an `Analyzer`/`run`/flag initialization cycle; this convention is documented in `pkg/linters/README.md` and `.github/skills/go-linters/SKILL.md`.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/linters/README.md

@@ -69,7 +69,28 @@ This package currently provides custom Go analyzers in the following subpackages
 - `uncheckedflushreturn` — reports `Flush()` method calls where the error return is discarded, which silently drops buffered data on failure.
 - `wgdonenotdeferred` — reports non-deferred `sync.WaitGroup.Done()` calls that can deadlock on panics or early returns.
 - `writebytestring` — reports `w.Write([]byte(s))` calls where `s` is a string, which can be replaced with `io.WriteString` to avoid an unnecessary `[]byte` allocation.
-- `internal` — shared helper packages for analyzers (file checks and `nolint` handling).
+- `internal` — shared helper packages for analyzers (file checks, `nolint` handling, and coverage-aware perf gating).
+
+## Coverage-aware perf gating
+
+Micro-optimizations flagged by allocation/perf linters (e.g. `stringsconcatloop`, `appendoneelement`,
+`appendbytestring`, `bytesbufferstring`, `bytescomparestring`, `lenstringsplit`, `mapclearloop`,
+`seenmapbool`, `sortslice`, `stringbytesroundtrip`, `stringsjoinone`, `tolowerequalfold`, and
+`writebytestring`) only matter on hot paths: applying them to code that tests never execute adds
+churn without a measurable benefit. These linters consult the shared
+`pkg/linters/internal/coverage` package, which loads a Go coverage profile (produced by
+`go test -covermode=count -coverprofile=<path>`) referenced by the `GH_AW_LINT_COVERAGE_PROFILE`
+environment variable and gates findings on the recorded execution hit count for the reported line.
+
+- When `GH_AW_LINT_COVERAGE_PROFILE` is unset (the default), coverage gating is a no-op and every
+  perf linter reports exactly as it did before coverage-awareness was introduced.
+- When a profile is loaded, a perf linter only reports a finding once the code path's execution
+  count is at least its `-hot-threshold` flag (default `1`: any recorded execution).
+- Pass `-<linter>.hot-threshold=0` to disable coverage gating for a specific linter even when a
+  profile is loaded.
+- Purely stylistic/readability linters (e.g. `stringsindexcontains`, `stringsindexhasprefix`,
+  `stringscountcontains`, `lenstringzero`) are intentionally **not** coverage-gated: they carry no
+  measurable performance difference, so "hot path" relevance does not apply to them.
 
 ## Public API
 

pkg/linters/appendbytestring/appendbytestring.go

@@ -11,13 +11,21 @@ import (
 
 	"github.com/github/gh-aw/pkg/linters/internal/analyzerutil"
 	"github.com/github/gh-aw/pkg/linters/internal/astutil"
+	"github.com/github/gh-aw/pkg/linters/internal/coverage"
 	"github.com/github/gh-aw/pkg/linters/internal/filecheck"
 	"github.com/github/gh-aw/pkg/linters/internal/nolint"
 )
 
 // Analyzer is the append-byte-string analysis pass.
 var Analyzer = analyzerutil.New("appendbytestring", "reports append(b, []byte(s)...) calls where s is a string that can be simplified to append(b, s...)", run)
 
+// hotThreshold gates findings on coverage data; see coverage package docs.
+var hotThreshold *int
+
+func init() {
+	hotThreshold = coverage.RegisterHotThresholdFlag(Analyzer)
+}
+
 func run(pass *analysis.Pass) (any, error) {
 	noLintIndex, err := nolint.Index(pass)
 	if err != nil {
@@ -84,6 +92,9 @@ func analyzeAppendByteString(pass *analysis.Pass, n ast.Node, generatedFiles fil
 	if sText == "" {
 		return
 	}
+	if !coverage.ShouldApply(pass, call.Pos(), *hotThreshold) {
+		return
+	}
 
 	pass.Report(analysis.Diagnostic{
 		Pos:            call.Pos(),

pkg/linters/appendoneelement/appendoneelement.go

@@ -12,13 +12,21 @@ import (
 
 	"github.com/github/gh-aw/pkg/linters/internal/analyzerutil"
 	"github.com/github/gh-aw/pkg/linters/internal/astutil"
+	"github.com/github/gh-aw/pkg/linters/internal/coverage"
 	"github.com/github/gh-aw/pkg/linters/internal/filecheck"
 	"github.com/github/gh-aw/pkg/linters/internal/nolint"
 )
 
 // Analyzer is the append-one-element analysis pass.
 var Analyzer = analyzerutil.New("appendoneelement", "reports append(s, []T{x}...) calls where a single-element slice literal is spread and can be simplified to append(s, x)", run)
 
+// hotThreshold gates findings on coverage data; see coverage package docs.
+var hotThreshold *int
+
+func init() {
+	hotThreshold = coverage.RegisterHotThresholdFlag(Analyzer)
+}
+
 func run(pass *analysis.Pass) (any, error) {
 	noLintIndex, err := nolint.Index(pass)
 	if err != nil {
@@ -66,6 +74,9 @@ func analyzeAppendOneElement(pass *analysis.Pass, n ast.Node, generatedFiles fil
 	if !ok {
 		return
 	}
+	if !coverage.ShouldApply(pass, call.Pos(), *hotThreshold) {
+		return
+	}
 
 	pass.Report(analysis.Diagnostic{
 		Pos:     call.Pos(),

pkg/linters/bytesbufferstring/bytesbufferstring.go

@@ -12,13 +12,21 @@ import (
 
 	"github.com/github/gh-aw/pkg/linters/internal/analyzerutil"
 	"github.com/github/gh-aw/pkg/linters/internal/astutil"
+	"github.com/github/gh-aw/pkg/linters/internal/coverage"
 	"github.com/github/gh-aw/pkg/linters/internal/filecheck"
 	"github.com/github/gh-aw/pkg/linters/internal/nolint"
 )
 
 // Analyzer is the bytes-buffer-string analysis pass.
 var Analyzer = analyzerutil.New("bytesbufferstring", "reports string(buf.Bytes()) calls where buf is a bytes.Buffer value and suggests buf.String() instead", run)
 
+// hotThreshold gates findings on coverage data; see coverage package docs.
+var hotThreshold *int
+
+func init() {
+	hotThreshold = coverage.RegisterHotThresholdFlag(Analyzer)
+}
+
 func run(pass *analysis.Pass) (any, error) {
 	noLintIndex, err := nolint.Index(pass)
 	if err != nil {
@@ -73,6 +81,9 @@ func analyzeStringBytesCall(pass *analysis.Pass, n ast.Node, generatedFiles file
 	if receiverText == "" {
 		return
 	}
+	if !coverage.ShouldApply(pass, call.Pos(), *hotThreshold) {
+		return
+	}
 
 	pass.Report(analysis.Diagnostic{
 		Pos:     call.Pos(),

pkg/linters/bytescomparestring/bytescomparestring.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/github/gh-aw/pkg/linters/internal/analyzerutil"
 	"github.com/github/gh-aw/pkg/linters/internal/astutil"
+	"github.com/github/gh-aw/pkg/linters/internal/coverage"
 	"github.com/github/gh-aw/pkg/linters/internal/filecheck"
 	"github.com/github/gh-aw/pkg/linters/internal/nolint"
 )
@@ -23,6 +24,13 @@ const bytesPkg = "bytes"
 // Analyzer is the bytes-compare-string analysis pass.
 var Analyzer = analyzerutil.New("bytescomparestring", "flags string(a) == string(b) and string(a) != string(b) as []byte comparisons written the long way; use bytes.Equal for clearer intent", run)
 
+// hotThreshold gates findings on coverage data; see coverage package docs.
+var hotThreshold *int
+
+func init() {
+	hotThreshold = coverage.RegisterHotThresholdFlag(Analyzer)
+}
+
 func run(pass *analysis.Pass) (any, error) {
 	noLintIndex, err := nolint.Index(pass)
 	if err != nil {
@@ -70,6 +78,9 @@ func analyzeBinaryExpr(pass *analysis.Pass, n ast.Node, generatedFiles filecheck
 	if lText == "" || rText == "" {
 		return
 	}
+	if !coverage.ShouldApply(pass, bin.Pos(), *hotThreshold) {
+		return
+	}
 	qualifier, skipFix := bytesQualifier(pass, bin.Pos())
 	if bin.Op == token.EQL {
 		var fixes []analysis.SuggestedFix

pkg/linters/internal/coverage/coverage.go

@@ -0,0 +1,124 @@
+// Package coverage provides helpers for coverage-aware perf linter gating.
+//
+// When the GH_AW_LINT_COVERAGE_PROFILE environment variable points to a Go
+// coverage profile (produced by "go test -covermode=count -coverprofile=<path>"),
+// [ShouldApply] returns false for code positions whose recorded hit count is
+// below the configured threshold, suppressing findings on cold paths.
+//
+// When the variable is unset (the default), every call to [ShouldApply]
+// returns true so all gated linters behave exactly as before.
+package coverage
+
+import (
+	"go/token"
+	"os"
+	"path/filepath"
+	"strings"
+	"sync"
+
+	xcov "golang.org/x/tools/cover"
+	"golang.org/x/tools/go/analysis"
+)
+
+const envVar = "GH_AW_LINT_COVERAGE_PROFILE"
+
+// profileIndex holds the lazily loaded coverage profiles indexed by filename.
+type profileIndex struct {
+	profiles map[string]*xcov.Profile // key: Profile.FileName (package-qualified path)
+}
+
+var (
+	once  sync.Once
+	index *profileIndex // nil means no profile loaded (permissive fallback)
+)
+
+// load loads the profile at most once per process from GH_AW_LINT_COVERAGE_PROFILE.
+// On any error (env unset, bad path, parse failure) it leaves index nil so
+// callers fall back to permissive behaviour.
+func load() {
+	once.Do(func() {
+		path := os.Getenv(envVar)
+		if path == "" {
+			return
+		}
+		profiles, err := xcov.ParseProfiles(path)
+		if err != nil {
+			return
+		}
+		m := make(map[string]*xcov.Profile, len(profiles))
+		for _, p := range profiles {
+			m[p.FileName] = p
+		}
+		index = &profileIndex{profiles: m}
+	})
+}
+
+// findProfile looks up the profile entry that corresponds to the given on-disk
+// filename. Coverage profile keys are package-qualified paths such as
+// "github.com/org/repo/pkg/foo/foo.go", while pass.Fset returns absolute OS
+// paths. We match by checking whether the normalised on-disk path ends with
+// the profile key.
+func (idx *profileIndex) findProfile(filename string) *xcov.Profile {
+	norm := filepath.ToSlash(filename)
+	for key, p := range idx.profiles {
+		normKey := filepath.ToSlash(key)
+		if strings.HasSuffix(norm, "/"+normKey) || norm == normKey {
+			return p
+		}
+	}
+	return nil
+}
+
+// hitCount returns the execution count for the given 1-based line number.
+// When multiple coverage blocks span the same line, the last one wins (standard
+// Go coverage tool behaviour). Returns 0 when no block covers the line.
+func hitCount(p *xcov.Profile, line int) int {
+	count := 0
+	for _, b := range p.Blocks {
+		if b.StartLine <= line && line <= b.EndLine {
+			count = b.Count
+		}
+	}
+	return count
+}
+
+// ShouldApply reports whether a linter finding at pos should be reported.
+//
+//   - When threshold is 0, coverage gating is disabled and the function
+//     always returns true.
+//   - When no coverage profile is loaded (GH_AW_LINT_COVERAGE_PROFILE is
+//     unset or the file cannot be parsed), the function always returns true
+//     (permissive fallback).
+//   - Otherwise it returns true only when the recorded hit count for the
+//     position's line is >= threshold.
+func ShouldApply(pass *analysis.Pass, pos token.Pos, threshold int) bool {
+	if threshold == 0 {
+		return true
+	}
+	load()
+	if index == nil {
+		return true
+	}
+	position := pass.Fset.Position(pos)
+	if !position.IsValid() {
+		return true
+	}
+	p := index.findProfile(position.Filename)
+	if p == nil {
+		return true
+	}
+	return hitCount(p, position.Line) >= threshold
+}
+
+// RegisterHotThresholdFlag registers a -hot-threshold flag on the given
+// analyzer and returns a pointer to the flag value. The default value is 1
+// (gate on any recorded execution). Pass 0 to disable gating entirely.
+//
+// This function must be called from an init() function, not from the analyzer
+// Run function, to avoid an analyzer initialisation cycle.
+func RegisterHotThresholdFlag(a *analysis.Analyzer) *int {
+	v := new(int)
+	*v = 1
+	a.Flags.IntVar(v, "hot-threshold", 1, "minimum coverage hit count to report a finding (0 = always report)")
+	return v
+}