Overview

The custom linter suite (pkg/linters/, registered in cmd/linters/main.go) has an inconsistent suppression story. The shared helper pkg/linters/internal/nolint already exists and provides a clean, reusable BuildLineIndex + HasDirective pair that lets a linter honor //nolint:<name> (and //nolint:all) comments — matching golangci-lint behavior. But only 3 of 21 registered analyzers actually wire it up.

Critically, 9 of the 11 linters that are already enforced in CI (.github/workflows/cgo.yml:1078, make golint-custom LINTER_FLAGS=...) silently ignore //nolint. Today they pass because every flagged site was refactored to zero. But the moment a legitimately intentional violation appears, a contributor has no in-code escape hatch — the only options are an awkward refactor or editing the CI allowlist. That is exactly the kind of friction internal/nolint was built to prevent, and two enforced linters (errstringmatch, jsonmarshalignoredeerror) already rely on it.

Critical finding: suppression parity across enforced linters

2 of 11 enforced linters honor //nolint; 9 do not. (The only other linter wired to internal/nolint is errormessage, which is opt-in / not enforced — so the codebase-wide total is just 3 of 21.)

Why it matters

• No escape hatch under enforcement. jsonmarshalignoredeerror shipped with 13 live //nolint: sites (per [deep-report] Add 6 missing analyzers to pkg/linters/spec_test.go and fix stale count comment #36941) proving real-world need for per-site exceptions. The other 9 enforced linters offer no equivalent — a future justified exception forces a refactor or a CI-config change.
• Self-consistency. This is the same linter-framework-should-be-internally-consistent theme that drove the earlier uncheckedtypeassertion two-value ResultOf fix. The framework already owns the solution (internal/nolint); it's just unevenly applied.
• Lowers the cost of new enforcement. Linters being driven toward enforcement (e.g. the strings.EqualFold conversion in [lint-monster] Use strings.EqualFold for Case-Insensitive Comparison #37047, the map[string]struct{} conversion in [lint-monster] Replace map[string]bool with map[string]struct{} for Set Operations #37048) would be safer to enforce if a documented //nolint: path existed for the rare legitimate case, rather than requiring every site to reach zero.

$ rg -l "internal/nolint" pkg/linters --glob '*.go'
pkg/linters/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go
pkg/linters/errstringmatch/errstringmatch.go
pkg/linters/errormessage/errormessage.go

import "github.com/github/gh-aw/pkg/linters/internal/nolint"

// inside run(pass *analysis.Pass):
noLintLinesByFile := nolint.BuildLineIndex(pass, "errstringmatch")
// ... before each report:
position := pass.Fset.Position(node.Pos())
if nolint.HasDirective(position, noLintLinesByFile) {
    return // suppressed
}
pass.ReportRangef(node, "...")

Recommended fix

For each of the 9 enforced linters without suppression support:

1. Import github.com/github/gh-aw/pkg/linters/internal/nolint.
2. Build the index once at the top of run: idx := nolint.BuildLineIndex(pass, "<linterName>").
3. Guard each pass.Report* call: if nolint.HasDirective(pass.Fset.Position(pos), idx) { return/continue }.
4. Add a //nolint:<name> case to each linter's testdata/ so the suppression path is regression-tested.

Optionally extend the same treatment to the registered-but-unenforced precision-gap linters (seenmapbool, tolowerequalfold) so they are enforcement-ready.

Validation checklist

• nolint.BuildLineIndex + HasDirective wired into all 9 enforced linters.
• Each linter's testdata/ includes a //nolint:<name> suppression case (same-line and previous-line).
• go test ./pkg/linters/... passes.
• make golint-custom still reports zero violations (no behavioral change for un-annotated code).

Effort

Moderate, mechanical, low-risk — 9 near-identical 3-line edits plus testdata. No production code changes; no behavior change for code without //nolint annotations.

Scope note (deliberately out of scope)

This issue is only about the suppression-parity gap in the linter framework. The violation-conversion work is already tracked elsewhere and is not duplicated here: strings.EqualFold conversion (#37047), map[string]struct{} conversion (#37048), function-length refactoring (#37046), and spec-test analyzer coverage (#36941).

References:

• §26996526330

Generated by 🤖 Sergo - Serena Go Expert · 2.4K AIC · ◷

• expires on Jun 12, 2026, 5:26 AM UTC