sprintfint autofix omits the strconv import and orphans fmt — non-compiling fix for single-use files; the RWSF golden masks it

[github-actions]

Summary

sprintfint (pkg/linters/sprintfint/sprintfint.go) rewrites fmt.Sprintf("%d", n) → strconv.Itoa(n). The emitted SuggestedFix (buildItoaFix, lines 94-109) contains only the call-replacement TextEdit — it never adds a "strconv" import, and it does nothing about the now-possibly-orphaned "fmt" import. For the common case of a file whose only strconv/fmt use is the flagged call, applying the fix produces code that fails to compile in two ways:

1. undefined: strconv — strconv.Itoa is emitted but strconv is not imported.
2. "fmt" imported and not used — the flagged call was the last fmt reference.

This is the same self-containment defect that #44653 filed against writebytestring (missing io import) — which has since been retrofitted with an in-fix import edit (writebytestring.go:213 addIOImportEdit), and the 44th linter bytescomparestring was written to add its bytes import in-fix (bytescomparestring.go:113 addBytesImportEdit). sprintfint is now the only import-introducing SuggestedFix linter that does not self-add its import, and it additionally has an orphaned-import problem the other two do not (they add a package; sprintfint both adds strconv and removes the last fmt use).

Evidence

pkg/linters/sprintfint/sprintfint.go:94-109   // buildItoaFix: single TextEdit, no import management
pkg/linters/sprintfint/sprintfint.go:22       // Doc: "suggested fixes may require goimports to add/remove imports"

The analyzer Doc string explicitly acknowledges the goimports dependency, but the autofix is applied verbatim by any non-gopls driver (e.g. go vet -vettool=... -fix, singlechecker -fix, multichecker -fix), none of which run goimports. Only editor-integrated gopls rescues it via format-on-save.

The RWSF golden hides the bug

sprintfint_test.go:16 uses analysistest.RunWithSuggestedFixes, but the fixture is hand-constructed so the import transition is never exercised:

pkg/linters/sprintfint/testdata/src/sprintfint/sprintfint.go(.golden)
  import ( "fmt"; "strconv" )   // strconv PRE-imported -> no "undefined: strconv"
  func goodInt64(n int64) ...    // keeps other fmt.Sprintf uses alive -> fmt never orphaned

Because the fixture pre-imports strconv and preserves other fmt uses, the golden compiles and the test passes — giving false confidence that the autofix is compile-safe. analysistest does not compile the golden, so a single-use fixture would expose it.

Concrete failing input

package foo
import "fmt"
func label(n int) string { return fmt.Sprintf("%d", n) }

After -fix:

package foo
import "fmt"                                   // "fmt" imported and not used
func label(n int) string { return strconv.Itoa(n) }  // undefined: strconv

Impact

• Any bulk/CI application of the suggested fix (without a trailing goimports pass) yields non-compiling code for single-use files — the majority case for a targeted fmt/strconv call.
• Inconsistent with the suite's now-established convention (writebytestring retrofit + bytescomparestring) that import-introducing fixes are self-contained.

Recommendation

Adopt the same in-fix import handling used by bytescomparestring/writebytestring:

1. Add a "strconv" import TextEdit (reuse the grouped/single/standalone insertion logic + per-file dedup already implemented in those two linters — factor it into pkg/linters/internal/astutil to avoid a third copy).
2. When the flagged call is the file's last fmt reference, also remove the now-unused "fmt" import (or, if that analysis is too costly, keep the Doc caveat but at minimum add the strconv import so the result is at worst an unused-import warning, not an undefined error).
3. Add a testdata fixture whose file imports only fmt and uses fmt.Sprintf("%d", n) exactly once, so the RWSF golden exercises both the strconv-add and the fmt-removal.

Validation checklist

• New fixture file (single fmt import, one flagged call) — golden shows strconv imported and fmt removed, and the golden compiles.
• Existing multi-use fixture still passes (fmt retained where other uses exist).
• go build ./... on a scratch module after applying the fix to a single-use file.

Effort: Small–Medium (mostly reuse of the existing import-edit helper; the fmt-removal is the only genuinely new logic). NOT CI-enforced today, so this is latent quality, not an active break.

Generated by 🤖 Sergo - Serena Go Expert · 333.5 AIC · ⌖ 11.7 AIC · ⊞ 5.8K · ◷

• expires on Jul 17, 2026, 8:59 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 · 2.09 AIC · ⌖ 5.37 AIC · ⊞ 8.4K · ◷