mapdeletecheck: autofix silently deletes comments inside the if body — no comment guard, unlike its sibling mapclearloop

[github-actions]

Summary

mapdeletecheck emits its SuggestedFix unconditionally, replacing the entire if _, ok := m[k]; ok { delete(m, k) } statement (ifStmt.Pos() → ifStmt.End()) with the bare text delete(m, k). Any comment that lives inside that range — a leading comment above the delete, a trailing comment on the delete line, or a comment after the { — is inside [ifStmt.Pos(), ifStmt.End()) and is therefore silently discarded when the fix is applied.

Its sibling linter mapclearloop (same package family, same author style, same range-replacement shape) guards against exactly this: it computes hasOverlappingComment(pass.Files, rangeStmt.Pos(), rangeStmt.End()) and, when a comment overlaps the replaced span, suppresses the SuggestedFix while still reporting the diagnostic. mapdeletecheck has no such guard — a clear inconsistency within the same linter family.

Location

• pkg/linters/mapdeletecheck/mapdeletecheck.go:119-131 — the diagnostic always carries a SuggestedFixes entry whose single TextEdit spans ifStmt.Pos()→ifStmt.End() with NewText: "delete(mText, kText)".
• Compare pkg/linters/mapclearloop/mapclearloop.go:140-149 (hasOverlappingComment guard) and its helper mapclearloop.go:173-186.

Failure scenario (comment loss on autofix)

if _, ok := cache[key]; ok {
	// evict the stale entry before refetching
	delete(cache, key)
}

go vet -fix / analysistest.RunWithSuggestedFixes rewrites this to:

delete(cache, key)

The explanatory comment // evict the stale entry before refetching is gone. The same loss occurs for a trailing comment (delete(cache, key) // evict) or a comment after the opening brace (if _, ok := cache[key]; ok { // guard), since all fall within [ifStmt.Pos(), ifStmt.End()).

Why the existing test does not catch it

pkg/linters/mapdeletecheck/mapdeletecheck_test.go:15 uses analysistest.RunWithSuggestedFixes, so the golden file is verified — but the testdata only contains comments that sit between statements (outside any flagged if), e.g. mapdeletecheck.go.golden:9,17,20,.... No flagged case has a comment inside the if body, so the golden never exercises the comment-dropping path. The bug is latent, not caught.

Impact

• Autofix safety / behavior: applying the suggested fix silently destroys developer intent captured in comments. This is exactly the class of autofix-quality problem the maintainers already guard against in mapclearloop.
• Not CI-enforced today, so this is latent — but it becomes user-facing the moment mapdeletecheck is wired into go vet -fix or an editor "apply fix" action.

Recommendation

Adopt the established mapclearloop pattern: before attaching the SuggestedFix, check whether any comment overlaps [ifStmt.Pos(), ifStmt.End()) and, if so, report the diagnostic without a fix (the developer applies it manually and preserves the comment). Concretely, lift hasOverlappingComment into a shared internal/astutil helper (both linters need the identical logic) and gate SuggestedFixes on it.

Before (current):

pass.Report(analysis.Diagnostic{
	Pos: ifStmt.Pos(), End: ifStmt.End(), Message: ...,
	SuggestedFixes: []analysis.SuggestedFix{{ ... }},
})

After (intent):

diag := analysis.Diagnostic{Pos: ifStmt.Pos(), End: ifStmt.End(), Message: ...}
if !astutil.HasOverlappingComment(pass.Files, ifStmt.Pos(), ifStmt.End()) {
	diag.SuggestedFixes = []analysis.SuggestedFix{{ ... }}
}
pass.Report(diag)

Validation checklist

• Add a testdata case with (a) a leading comment above delete, (b) a trailing comment on the delete line, both inside a flagged if; assert the diagnostic still fires but the .golden leaves the code (and comment) unchanged.
• Confirm mapclearloop's guard behavior is mirrored (diagnostic yes, fix no) when a comment overlaps.
• Optionally de-duplicate the guard into internal/astutil so both linters share one implementation.

Effort: small (single-file change + one testdata case; optional tiny astutil extraction).

Generated by 🤖 Sergo - Serena Go Expert · 367.5 AIC · ⌖ 14.7 AIC · ⊞ 5.8K · ◷

• expires on Jul 23, 2026, 9:03 PM UTC-08:00

[github-actions]

🍪 Issue Monster selected this for Copilot

I've identified this issue as a good candidate for automated resolution and requested assignment to the Copilot coding agent.

If assignment succeeds, the Copilot coding agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

🍪 Om nom nom by Issue Monster · 1.2 AIC · ⌖ 10.8 AIC · ⊞ 7.1K · ◷