Skip to content

mapclearloop enforce-readiness: clean, type-resolved, autofix-verified, zero production violations — but its already-enforced si [Content truncated due to length] #46341

Description

@github-actions

Summary

mapclearloop (range-delete loop over a map → clear(m)) is production-ready for CI enforcement but is absent from the enforced analyzer list. Its sibling mapdeletecheck — same package family, same author style, added in the same wave — is already enforced (.github/workflows/cgo.yml:1208 and the wasm gate at :1211). mapclearloop is arguably the more careful of the two (it has a comment-preservation guard that mapdeletecheck still lacks, see #46130), yet it is the one left out of the gate.

Evidence

Enforced list today (cgo.yml:1208) — mapdeletecheck present, mapclearloop absent
make golint-custom LINTER_FLAGS="-errstringmatch -panicinlibrarycode -manualmutexunlock \
  -osexitinlibrary -rawloginlib -logfatallibrary -regexpcompileinfunction -fprintlnsprintf \
  -strconvparseignorederror -jsonmarshalignoredeerror -uncheckedtypeassertion -fmterrorfnoverbs \
  -tolowerequalfold -httpnoctx -timeafterleak -errortypeassertion -execcommandwithoutcontext \
  -sprintfint -stringsindexcontains -stringscountcontains -bytesbufferstring -ioutildeprecated \
  -mapdeletecheck -test=false"

mapclearloop appears nowhere in cgo.yml (prod or wasm gate).

Correctness of the analyzer (pkg/linters/mapclearloop/mapclearloop.go):

  • Type-resolved throughout — no syntactic package/name matching. Map type is verified via pass.TypesInfo.TypeOf(rangeStmt.X).Underlying().(*types.Map) (mapclearloop.go:59-65); delete is confirmed to be the builtin via pass.TypesInfo.Uses[delIdent].(*types.Builtin) (:104-107); the map and key are matched by object identity (sameObject, :190-208).
  • Shadow-safe fix: clear is only emitted after builtinVisibleAtPos(pass.Pkg, rangeStmt.Pos(), "clear") confirms the clear builtin is not shadowed at the site (:131, :157-171). This is the correct guard for emitting a builtin the source did not already contain.
  • Comment-safe fix: the SuggestedFix is suppressed (diagnostic still reported) when a comment overlaps the replaced span, via hasOverlappingComment(pass.Files, rangeStmt.Pos(), rangeStmt.End()) (:140-149). This is exactly the guard mapdeletecheck: autofix silently deletes comments inside the if body — no comment guard, unlike its sibling mapclearloop #46130 asks mapdeletecheck to adopt — mapclearloop already has it.
  • Wires generated-file skip (filecheck.ShouldSkipFilename) and per-line suppression (nolint.HasDirectiveForLinter), consistent with the rest of the family.
  • Autofix is golden-verified: mapclearloop_test.go:15 uses analysistest.RunWithSuggestedFixes (not the weaker analysistest.Run), so the emitted clear(...) text is checked against .golden. This closes the verification-parity class from Autofix verification-parity gap: 3 of 9 SuggestedFix linters (stringsindexcontains, sprintfint, stringreplaceminusone) have zero [Content truncated due to length] #43313.

Zero production violations (the enforce-readiness bar):
A repo-wide search for the exact shape it flags — a range-over-map loop whose sole body statement is delete(m, k) — returns only testdata under pkg/linters/mapclearloop/testdata/. The nearest production hit, pkg/cli/devcontainer.go:316-319, is not a violation: its loop body is a guarded if ... { delete(existing, key) }, not a bare unconditional delete, so mapclearloop (which requires len(rangeStmt.Body.List) == 1 and the single statement to be the delete call, :88-110) correctly does not flag it. Adding -mapclearloop to the gate therefore turns green immediately and locks in the invariant going forward (clear(m) is the Go 1.21+ idiom and also correctly clears NaN-keyed entries that a delete-loop cannot reach).

Impact

  • Consistency gap: the map-family pair is split across the enforcement boundary for no principled reason — the less guarded member is enforced, the more guarded one is not.
  • Regression risk: without the gate, a future for k := range m { delete(m, k) } can land in production un-flagged even though the analyzer that catches it already exists and is green.

Recommendation

Add -mapclearloop to both LINTER_FLAGS invocations in .github/workflows/cgo.yml (the prod gate at :1208 and the wasm gate at :1211), alongside -mapdeletecheck.

Before (cgo.yml:1208):

... -bytesbufferstring -ioutildeprecated -mapdeletecheck -test=false

After:

... -bytesbufferstring -ioutildeprecated -mapclearloop -mapdeletecheck -test=false

Validation checklist

Effort: trivial (two one-token edits in cgo.yml); the analyzer and its RWSF golden already exist and pass.

Non-duplicate note

Prior enforce-readiness issues covered different linters: #45629 (bytesbufferstring / mapdeletecheck / ioutildeprecated), #45186 (stringsindexcontains / stringscountcontains). mapclearloop was in none of them and appears in no open or closed sergo issue title — this gap is genuinely untracked. This is distinct from #46130, which is about mapdeletecheck's missing comment guard, not enforcement of mapclearloop.

Generated by 🤖 Sergo - Serena Go Expert · 364.2 AIC · ⌖ 14 AIC · ⊞ 5.8K ·

  • expires on Jul 24, 2026, 8:52 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

cookieIssue Monster Loves Cookies!sergo

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions