13 linters build SuggestedFix TextEdits without HasOverlappingComment guard (silent comment deletion on -fix)

[github-actions]

Overview

astutil.HasOverlappingComment (pkg/linters/internal/astutil/astutil.go:348-367) exists specifically to stop a SuggestedFix from silently deleting a comment that falls inside the replaced span. Issue #57844 already fixed this gap for 5 linters. This run found the same bug class recurring in 13 more linters that build full-span TextEdits (node.Pos()..node.End()) with no call to the guard anywhere in the file.

Confirmed (full file read)

Linter	buildFix location	Span replaced
writebytestring	pkg/linters/writebytestring/writebytestring.go:189-227 (edit at 214-218)	call.Pos()..call.End()
appendbytestring	pkg/linters/appendbytestring/appendbytestring.go:104-122	conv.Pos()..conv.End()
bytescomparestring	pkg/linters/bytescomparestring/bytescomparestring.go:140-153	bin.Pos()..bin.End()

Example failure scenario (writebytestring): w.Write(/* flush pending */ []byte(s)) — the fix replaces the whole call span including the comment, so -fix silently drops /* flush pending */ with no warning.

Likely affected (grep-confirmed: full-span TextEdit, no HasOverlappingComment call in file; not yet individually re-verified with a full read this run)

• tolowerequalfold (buildEqualFoldFix)
• stringsindexhasprefix
• stringreplaceminusone (buildReplaceAllFix)
• sprintfint (buildItoaFix)
• lenstringzero
• lenstringsplit (buildCountFix)
• fprintlnsprintf (buildFprintfFix)
• execcommandwithoutcontext
• ctxbackground
• bytesbufferstring

Why this matters now

A repo-wide grep for HasOverlappingComment returns only 8 hits: timenowsub, stringsjoinone, sprintfbool, mapdeletecheck, mapclearloop, appendoneelement (direct guard) plus stringsindexcontains/stringscountcontains (guarded indirectly via the shared astutil.BuildContainsFix helper, which already bails to nil on overlap). mapdeletecheck.go:84 is the correct reference pattern.

Of the 13 files above, all but lenstringzero are enforced on production code in CI (.github/workflows/cgo.yml, LINTER_FLAGS with -test=false, both native and GOOS=js GOARCH=wasm jobs), so -fix running against real source can hit this today, not just in theory.

Recommendation

1. For each confirmed/likely file, add the same guard used in mapdeletecheck.go:84 before constructing the TextEdits: if astutil.HasOverlappingComment(pass, span) is true, return nil SuggestedFixes (diagnostic still reported, just no unsafe autofix).
2. Since this is the second time this exact bug class has appeared across a double-digit number of linters (first sergo: 5 linters (appendoneelement, timenowsub, stringsjoinone, stringscountcontains, stringsindexcontains) silently delete comm #57844, now this), consider a structural fix instead of continuing to patch file-by-file: either (a) a shared astutil.SafeTextEdit-style constructor that all buildFix functions call instead of building analysis.TextEdit literals directly, or (b) a spec_test.go check that flags any analyzer producing full-span SuggestedFixes without a corresponding HasOverlappingComment guard reference in its source.

Validation checklist

• Add a comment-overlap testdata fixture per fixed linter (none of the 13 currently have one — confirmed via testdata/ grep for comment//*)
• go test ./pkg/linters/... passes after the guard is added
• -fix no longer removes a comment sitting inside the replaced span

Labels: sergo

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• api.anthropic.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "api.anthropic.com"

See Network Configuration for more information.

Generated by 🤖 Sergo - Serena Go Expert · claude · agent · 336.3 AIC · ⌖ 7.47 AIC · ⊞ 6.8K · ◷

• expires on Sep 10, 2026, 8:04 PM UTC-08:00