Skip to content

Autofix verification-parity gap: 3 of 9 SuggestedFix linters (stringsindexcontains, sprintfint, stringreplaceminusone) have zero [Content truncated due to length] #43313

Description

@github-actions

Summary

9 custom analyzers emit analysis.SuggestedFix text edits (batch-applied under -fix), but only 6 verify the emitted fix output via analysistest.RunWithSuggestedFixes + a .golden file. 3 emit fixes whose generated source is never checked by any test:

Linter Emits SuggestedFix Fix builder Test entrypoint .golden?
stringsindexcontains (new, R56 registry 40→41) yes buildContainsFix (stringsindexcontains.go:226-242) analysistest.Run (stringsindexcontains_test.go:15) none
sprintfint yes buildItoaFix (sprintfint.go:94-107) analysistest.Run (sprintfint_test.go:16) none
stringreplaceminusone yes buildReplaceAllFix (stringreplaceminusone.go:96-116) analysistest.Run (stringreplaceminusone_test.go:15) none

The 6 that DO verify: ctxbackground, execcommandwithoutcontext, fprintlnsprintf, lenstringsplit, lenstringzero, tolowerequalfold (8 .golden files total under pkg/linters/).

Why this matters

analysistest.Run only checks diagnostic positions/messages (the // want comments). It never applies the TextEdits, so the reconstructed replacement string is completely unexercised. For an autofix linter that is -fix-applied in batch, a defect in the fix builder ships silently — the diagnostic still lands on the right line, but the rewritten code is wrong.

This is not hypothetical: R55 found a real semantic-change bug in a fix builder (fprintlnsprintf buildFprintfFix dropped a trailing newline). That linter did have golden coverage, which is how such regressions are meant to be caught. The 3 linters above have no such safety net, so any future edit to their fix builders (or a latent defect today) would not be detected.

Concrete latent-risk examples in the uncovered set:

  • sprintfint documents in its own Analyzer.Doc that fixes "may require goimports to add/remove imports" — the strconv.Itoa(...) rewrite (sprintfint.go:105) leaves strconv unimported / fmt possibly unused. Import-management behavior is precisely the kind of thing a golden file pins.
  • stringsindexcontains reconstructs the replacement from astutil.NodeText (go/printer reprint of the arg AST, astutil.go:117), e.g. "!" + pkg + ".Contains(" + sText + ", " + subText + ")" (stringsindexcontains.go:229). The reprinted-arg + negation path is untested.

(Note: I audited stringsindexcontains's detection and fix logic and both are semantically correct today — all 6 comparison forms + yoda variants map correctly to strings.Contains/!strings.Contains, args preserved in call order. This issue is about the missing verification harness, not a known miscompile.)

Evidence

Grep reconciliation
# emit SuggestedFix (non-test):
pkg/linters/{ctxbackground,execcommandwithoutcontext,fprintlnsprintf,lenstringsplit,
            lenstringzero,sprintfint,stringreplaceminusone,stringsindexcontains,
            tolowerequalfold}/*.go   → 9 linters

# use RunWithSuggestedFixes (test):
pkg/linters/{ctxbackground,execcommandwithoutcontext,fprintlnsprintf,lenstringsplit,
            lenstringzero,tolowerequalfold}/*_test.go   → 6 linters

# *.golden under pkg/linters/ → 8 files (all belong to the 6 verified linters)
# stringsindexcontains / sprintfint / stringreplaceminusone → 0 golden, analysistest.Run only

Recommended fix (uniform, per linter)

For each of the 3 linters:

  1. Add a testdata/src/<name>/<name>.go.golden capturing the expected post-fix source for each // want case (contains + negated + yoda variants; for sprintfint, the strconv.Itoa rewrite).
  2. Switch the test from analysistest.Run(...) to analysistest.RunWithSuggestedFixes(t, testdata, <pkg>.Analyzer, "<name>") — matching the pattern already used by fprintlnsprintf_test.go, lenstringzero_test.go, etc.

Validation checklist

  • go test ./pkg/linters/stringsindexcontains/... ./pkg/linters/sprintfint/... ./pkg/linters/stringreplaceminusone/... passes with RunWithSuggestedFixes.
  • Each .golden reflects the exact reprinted replacement (verify sprintfint golden documents the import-management caveat, or gate the linter out of -fix batches until goimports is wired).
  • All 9 SuggestedFix linters now covered → grep parity: RunWithSuggestedFixes count == SuggestedFix-emitting count.

Effort: small–medium (test-only; 3 golden files + 3 one-line test swaps). No production analyzer changes required.

References: run §28695406176

Generated by 🤖 Sergo - Serena Go Expert · 387.7 AIC · ⌖ 14 AIC · ⊞ 5.9K ·

  • expires on Jul 10, 2026, 9:07 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