Summary
First-ever audit of pkg/linters/internal/coverage (ADR-51573, coverage-aware perf-linter gating), the shared mechanism used by all 15 allocation/perf-oriented linters (appendbytestring, appendoneelement, bytesbufferstring, bytescomparestring, lenstringsplit, mapclearloop, reflectdeepequalusage, seenmapbool, slicemakezerolength, sortslice, stringbytesroundtrip, stringsconcatloop, stringsjoinone, tolowerequalfold, writebytestring) via coverage.ShouldApply. The helper hitCount resolves a diagnostic hit count by LINE only, with no column awareness, and silently picks the wrong blocks count whenever a source line is spanned by more than one coverage block.
Evidence
pkg/linters/internal/coverage/coverage.go hitCount (lines 103-111):
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
}This loops over every block in the profile and keeps overwriting count whenever a block spans the queried line, so whichever matching block happens to sort last in p.Blocks wins, regardless of whether that block is the one actually containing the diagnostic position. ShouldApply (lines 122-139) calls this with only position.Line from pass.Fset.Position(pos) and never consults position.Column, so it cannot disambiguate.
A single source line can legitimately be spanned by two distinct coverage blocks, for example a one-line if x { a() } else { b() }, or any construct where go/cover emits adjacent blocks with the same StartLine/EndLine. If a gated linter reports a finding inside one block on that line but a different block on the same line happens to sort later in p.Blocks, hitCount silently returns the LATER blocks count instead of the block that actually contains the flagged position. Depending on which branch is hot vs cold, this either (a) suppresses a genuine hot-path finding because it inherits a colder sibling blocks count, or (b) reports a genuinely cold or dead-path finding because it inherits a hotter sibling blocks count - the opposite of what coverage gating is meant to guarantee.
Impact
Currently latent: GH_AW_LINT_COVERAGE_PROFILE is not set anywhere in CI today (grep of .github confirms it is referenced only in .github/skills/go-linters/SKILL.md documentation, never exported in a workflow), so ShouldApply always takes the permissive index == nil fallback and this code path never executes in practice. However this is the shared gating primitive for all 15 perf linters by design per ADR-51573, whose own stated intent is for a future CI pipeline to set this variable and activate gating - at that point every one of the 15 linters would inherit this column-blind mismatch on any source line shared by multiple coverage blocks, silently defeating the gating guarantee the ADR describes.
Recommendation
Either: (1) have hitCount select the block whose column range (StartCol/EndCol) actually contains the position, falling back to line-only matching only when no block has column info, or (2) change the last-one-wins tie-break to select the block with the smallest (most specific) span containing the position, consistent with how go tool cover -html resolves overlapping blocks. Add a coverage_test.go case with two synthetic blocks sharing one StartLine/EndLine but different Count and Column ranges to lock in the fix.
Validation checklist
- Add a coverage_test.go case: two Profile.Blocks on the same line with different columns and counts, assert hitCount/ShouldApply picks the block matching the queried position
- Re-run go test ./pkg/linters/internal/coverage/...
- Confirm no behavioral change when GH_AW_LINT_COVERAGE_PROFILE is unset (permissive fallback path untouched)
Effort: small (single-file fix plus a targeted test), no risk to the 15 call sites since the public ShouldApply signature is unchanged.
Generated by 🤖 Sergo - Serena Go Expert · claude · agent · 277.9 AIC · ⊞ 5.2K · ◷
Summary
First-ever audit of pkg/linters/internal/coverage (ADR-51573, coverage-aware perf-linter gating), the shared mechanism used by all 15 allocation/perf-oriented linters (appendbytestring, appendoneelement, bytesbufferstring, bytescomparestring, lenstringsplit, mapclearloop, reflectdeepequalusage, seenmapbool, slicemakezerolength, sortslice, stringbytesroundtrip, stringsconcatloop, stringsjoinone, tolowerequalfold, writebytestring) via coverage.ShouldApply. The helper hitCount resolves a diagnostic hit count by LINE only, with no column awareness, and silently picks the wrong blocks count whenever a source line is spanned by more than one coverage block.
Evidence
pkg/linters/internal/coverage/coverage.go hitCount (lines 103-111):
This loops over every block in the profile and keeps overwriting count whenever a block spans the queried line, so whichever matching block happens to sort last in p.Blocks wins, regardless of whether that block is the one actually containing the diagnostic position. ShouldApply (lines 122-139) calls this with only position.Line from pass.Fset.Position(pos) and never consults position.Column, so it cannot disambiguate.
A single source line can legitimately be spanned by two distinct coverage blocks, for example a one-line if x { a() } else { b() }, or any construct where go/cover emits adjacent blocks with the same StartLine/EndLine. If a gated linter reports a finding inside one block on that line but a different block on the same line happens to sort later in p.Blocks, hitCount silently returns the LATER blocks count instead of the block that actually contains the flagged position. Depending on which branch is hot vs cold, this either (a) suppresses a genuine hot-path finding because it inherits a colder sibling blocks count, or (b) reports a genuinely cold or dead-path finding because it inherits a hotter sibling blocks count - the opposite of what coverage gating is meant to guarantee.
Impact
Currently latent: GH_AW_LINT_COVERAGE_PROFILE is not set anywhere in CI today (grep of .github confirms it is referenced only in .github/skills/go-linters/SKILL.md documentation, never exported in a workflow), so ShouldApply always takes the permissive index == nil fallback and this code path never executes in practice. However this is the shared gating primitive for all 15 perf linters by design per ADR-51573, whose own stated intent is for a future CI pipeline to set this variable and activate gating - at that point every one of the 15 linters would inherit this column-blind mismatch on any source line shared by multiple coverage blocks, silently defeating the gating guarantee the ADR describes.
Recommendation
Either: (1) have hitCount select the block whose column range (StartCol/EndCol) actually contains the position, falling back to line-only matching only when no block has column info, or (2) change the last-one-wins tie-break to select the block with the smallest (most specific) span containing the position, consistent with how go tool cover -html resolves overlapping blocks. Add a coverage_test.go case with two synthetic blocks sharing one StartLine/EndLine but different Count and Column ranges to lock in the fix.
Validation checklist
Effort: small (single-file fix plus a targeted test), no risk to the 15 call sites since the public ShouldApply signature is unchanged.