Summary
The three import-adding autofix linters all share one latent defect in how they reconcile the package qualifier they emit with the import they (do or don't) add:
pkg/linters/sprintfint/sprintfint.go — emits strconv.Itoa(x)pkg/linters/writebytestring/writebytestring.go — emits io.WriteString(w, s)pkg/linters/bytescomparestring/bytescomparestring.go — emits bytes.Equal(a, b)
Each decides whether it needs to add an import by scanning file.Imports and comparing only the import path (imp.Path.Value == "\"strconv\"" etc.). None ever consults the import's local binding name (imp.Name). But the replacement text hardcodes the default package name (strconv., io., bytes.).
When the target package is imported under an alias, the path-keyed check reports it as already present, so no import edit is added — and the emitted call references the default name, which is undefined in that file. The applied fix does not compile.
Evidence (code)
bytescomparestring — path-only presence check + hardcoded bytes.
// bytescomparestring.go:88 / :95 — replacement text hardcodes "bytes."
SuggestedFixes: buildFix(pass, bin, fmt.Sprintf("bytes.Equal(%s, %s)", lText, rText), seenImportFiles),
// ...
// addBytesImportEdit — bytescomparestring.go:145-149
for _, imp := range file.Imports {
if imp.Path.Value == `"`+bytesPkg+`"` { // keyed on PATH only, ignores imp.Name
return analysis.TextEdit{}, false // "already imported" -> no edit added
}
}
writebytestring — same shape (io), sprintfint — same shape (strconv)
// writebytestring.go:207 — replacement hardcodes "io."
replacement := fmt.Sprintf("io.WriteString(%s, %s)", writerArg, sText)
// writebytestring.go:247
if imp.Path.Value == `"`+ioPkg+`"` { return analysis.TextEdit{}, false }
// sprintfint.go:115 — replacement hardcodes "strconv."
NewText: []byte("strconv.Itoa(" + argText + ")"),
// sprintfint.go:148-150
case `"`+strconvPkg+`"`:
strconvImported = true // path-only; alias name never capturedgrep -n imp.Name across all three files returns nothing — none resolve the local binding.
Failure scenario (concrete)
Input file that already imports bytes under an alias:
package p
import bx "bytes"
var _ = bx.Buffer{}
func eq(a, b []byte) bool {
return string(a) == string(b) // flagged
}Applied suggested fix:
return bytes.Equal(a, b) // undefined: bytes -> DOES NOT COMPILE
The file binds the package to bx; bytes is undefined. The linter added no import (path "bytes" was seen as present), and the emitted qualifier is wrong. The identical failure reproduces for io→io.WriteString and strconv→strconv.Itoa.
A closely related second trigger: even when the linter does add import "bytes", it emits bytes. — if a local identifier shadows the package name at the call site (e.g. a parameter bytes []byte, which already exists at pkg/console/progress_shared.go:7 via scaleBinaryBytes(bytes, ...)), the qualifier resolves to the local value and the fix fails to compile (bytes.Equal = method on []byte).
Why the tests don't catch it
All three linters test with analysistest.RunWithSuggestedFixes + .golden, but RWSF only text-compares the applied edits against the golden file — it never recompiles the result, and every fixture imports the package under its default name. No fixture exercises an aliased or shadowed import, so the golden blesses whatever the linter emits. (Same verification-parity blind spot noted in #44187 / #43313, here on a distinct axis.)
Distinct from prior issues
Recommendation
When the target package is already imported, derive the qualifier from the actual binding instead of hardcoding the default name:
- In the presence scan, capture the local name: if
imp.Name != nil use imp.Name.Name, else the package's default name. Emit <localName>.Equal(...) / .WriteString(...) / .Itoa(...).
- Or resolve it semantically via
pass.TypesInfo (a *types.PkgName whose Imported().Path() matches) at the fix site — this also lets you detect a shadowing local binding and skip the SuggestedFix (still report the diagnostic) when a safe qualifier can't be guaranteed.
stringscountcontains already does the right thing for the reuse case — it takes the qualifier from the flagged call's own selector text (pkgText), preserving aliases; appendbytestring sidesteps it entirely by emitting no qualifier. These are the correct references.
Add RWSF fixtures with (a) an aliased import and (b) a shadowing local identifier, with goldens that keep the aliased/qualified form, so the fix is verified against these shapes.
Scope / severity
Latent (no aliased io/bytes/strconv import exists in the repo today; a shadowing bytes parameter does). None of the three linters is CI-enforced, and enforcement only reports diagnostics, so this affects --fix consumers rather than CI. Low blast radius, but a real non-compiling-autofix defect shared across three linters with a single root cause and a single fix pattern.
Method
Serena/LSP semantic scan of `pkg/linters/`. Registry delta 43->45 since last run (new: `bytescomparestring` 44th, `stringscountcontains` 45th); both audited clean on precision (type-resolved matching via `astutil.IsPkgSelector` + `TypesInfo`, complete comparison-operator sets, nolint + `IsTestFile` wired, RWSF+golden tests). This finding surfaced from the cross-linter SuggestedFix-body probe.
Generated by 🤖 Sergo - Serena Go Expert · 301.2 AIC · ⌖ 14.4 AIC · ⊞ 5.8K · ◷
Summary
The three import-adding autofix linters all share one latent defect in how they reconcile the package qualifier they emit with the import they (do or don't) add:
pkg/linters/sprintfint/sprintfint.go— emitsstrconv.Itoa(x)pkg/linters/writebytestring/writebytestring.go— emitsio.WriteString(w, s)pkg/linters/bytescomparestring/bytescomparestring.go— emitsbytes.Equal(a, b)Each decides whether it needs to add an import by scanning
file.Importsand comparing only the import path (imp.Path.Value == "\"strconv\""etc.). None ever consults the import's local binding name (imp.Name). But the replacement text hardcodes the default package name (strconv.,io.,bytes.).When the target package is imported under an alias, the path-keyed check reports it as already present, so no import edit is added — and the emitted call references the default name, which is undefined in that file. The applied fix does not compile.
Evidence (code)
bytescomparestring — path-only presence check + hardcoded
bytes.writebytestring — same shape (io), sprintfint — same shape (strconv)
grep -n imp.Nameacross all three files returns nothing — none resolve the local binding.Failure scenario (concrete)
Input file that already imports
bytesunder an alias:Applied suggested fix:
The file binds the package to
bx;bytesis undefined. The linter added no import (path"bytes"was seen as present), and the emitted qualifier is wrong. The identical failure reproduces forio→io.WriteStringandstrconv→strconv.Itoa.A closely related second trigger: even when the linter does add
import "bytes", it emitsbytes.— if a local identifier shadows the package name at the call site (e.g. a parameterbytes []byte, which already exists atpkg/console/progress_shared.go:7viascaleBinaryBytes(bytes, ...)), the qualifier resolves to the local value and the fix fails to compile (bytes.Equal= method on[]byte).Why the tests don't catch it
All three linters test with
analysistest.RunWithSuggestedFixes+.golden, but RWSF only text-compares the applied edits against the golden file — it never recompiles the result, and every fixture imports the package under its default name. No fixture exercises an aliased or shadowed import, so the golden blesses whatever the linter emits. (Same verification-parity blind spot noted in #44187 / #43313, here on a distinct axis.)Distinct from prior issues
Recommendation
When the target package is already imported, derive the qualifier from the actual binding instead of hardcoding the default name:
imp.Name != niluseimp.Name.Name, else the package's default name. Emit<localName>.Equal(...)/.WriteString(...)/.Itoa(...).pass.TypesInfo(a*types.PkgNamewhoseImported().Path()matches) at the fix site — this also lets you detect a shadowing local binding and skip the SuggestedFix (still report the diagnostic) when a safe qualifier can't be guaranteed.stringscountcontainsalready does the right thing for the reuse case — it takes the qualifier from the flagged call's own selector text (pkgText), preserving aliases;appendbytestringsidesteps it entirely by emitting no qualifier. These are the correct references.Add RWSF fixtures with (a) an aliased import and (b) a shadowing local identifier, with goldens that keep the aliased/qualified form, so the fix is verified against these shapes.
Scope / severity
Latent (no aliased
io/bytes/strconvimport exists in the repo today; a shadowingbytesparameter does). None of the three linters is CI-enforced, and enforcement only reports diagnostics, so this affects--fixconsumers rather than CI. Low blast radius, but a real non-compiling-autofix defect shared across three linters with a single root cause and a single fix pattern.Method
Serena/LSP semantic scan of `pkg/linters/`. Registry delta 43->45 since last run (new: `bytescomparestring` 44th, `stringscountcontains` 45th); both audited clean on precision (type-resolved matching via `astutil.IsPkgSelector` + `TypesInfo`, complete comparison-operator sets, nolint + `IsTestFile` wired, RWSF+golden tests). This finding surfaced from the cross-linter SuggestedFix-body probe.