Skip to content

writebytestring autofix references io.WriteString but never adds the "io" import — non-compiling fix when the file doesn't alrea [Content truncated due to length] #44653

Description

@github-actions

Summary

writebytestring (43rd analyzer, pkg/linters/writebytestring/writebytestring.go) rewrites w.Write([]byte(s))io.WriteString(w, s). The emitted SuggestedFix contains only the call-replacement TextEdit — it never adds an "io" import. So for any file that does not already import io (the common case — e.g. a file that only imports "bytes" and writes to a *bytes.Buffer), applying the fix produces io.WriteString(...) against an unimported package → undefined: io, does not compile.

This is distinct from #44187 (which was solely about named-string types + RWSF parity, both since fixed). The missing-import gap was never in scope there and is still present. It is also a convention divergence: the newer sibling bytescomparestring (44th) handles the analogous add-only situation correctly by injecting its "bytes" import.

Not CI-enforced today (no -fix in cgo.yml), so latent — but it makes the autofix unsafe to apply (raw -fix, go vet-style application, or manual copy of the suggestion) and blocks safe enforcement.

The defect

buildFix (writebytestring.go:199-211) emits a single edit and defers the import to gopls:

// Note: if the file does not already import "io", tools such as gopls will add the import automatically.
func buildFix(call *ast.CallExpr, writerArg, sText string) []analysis.SuggestedFix {
	replacement := fmt.Sprintf("io.WriteString(%s, %s)", writerArg, sText)
	return []analysis.SuggestedFix{{
		Message: "Replace with " + replacement,
		TextEdits: []analysis.TextEdit{{
			Pos:     call.Pos(),
			End:     call.End(),
			NewText: []byte(replacement),   // introduces `io.` with no import edit
		}},
	}}
}

analysistest.RunWithSuggestedFixes and the raw -fix flag apply only the listed TextEdits verbatim — neither runs goimports. Relying on "gopls will add it" is true only inside an editor quick-fix flow, not for the analyzer's own suggested fix.

Concrete failing scenario

package foo

import "bytes"   // note: no "io"

func f(buf *bytes.Buffer, s string) {
	buf.Write([]byte(s))
}

The linter flags the Write call and suggests io.WriteString(&buf, s). Applied, the file becomes:

import "bytes"
...
	io.WriteString(&buf, s)   // undefined: io — compile error

*bytes.Buffer, *strings.Builder, http.ResponseWriter, and *os.File all satisfy io.Writer without the file needing to import io, so this is the normal case, not an edge case.

Evidence the gap is untested

The testdata masks it: pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go:5 already imports "io", and the .golden keeps that import (writebytestring.go.golden:5). Every fixed line (writebytestring.go.golden:16,18,26,36,42,47) resolves io.WriteString against the pre-existing import, so the RWSF golden test passes even though the fix never adds io. No testdata file exercises a Write([]byte(...)) site in a file that lacks an io import.

Contrast with the correct sibling (bytescomparestring)

bytescomparestring (bytescomparestring.go:107-199) faces the identical add-only need — its fix bytes.Equal(a, b) introduces bytes — and handles it: buildFix appends an import TextEdit from addBytesImportEdit, which locates a grouped import block (or converts a single import / inserts a standalone import), skips when bytes is already imported, and de-dups multi-violation files via a seenImportFiles map[token.Pos]bool. writebytestring should mirror exactly this. Both are add-only (nothing becomes unused after the rewrite), so no import-removal is needed and a plain TextEdit fully closes the gap. (sprintfint legitimately defers to goimports because its fmt.Sprintfstrconv.Itoa rewrite may need to remove fmt as well — it documents this in its Doc string; writebytestring has neither the removal complexity nor the documentation.)

Recommended fix

  1. In buildFix, when the file containing call does not already import "io", append an import TextEdit — reuse the bytescomparestring.addBytesImportEdit approach (ideally lift it into pkg/linters/internal/astutil as a shared AddImportEdit(pass, pos, pkgPath, seen) helper so both linters share one audited implementation), threading a per-pass seenImportFiles map to avoid duplicate overlapping edits.
  2. Add a testdata file (or function) whose file imports only "bytes" (no "io") and writes to a *bytes.Buffer, with a .golden that shows the injected "io" import, so RunWithSuggestedFixes actually verifies the injection and this can't regress.
  3. Remove the now-inaccurate "gopls will add the import automatically" comment at writebytestring.go:200.

Validation checklist

  • buildFix emits an "io" import TextEdit only when the file lacks the import.
  • Grouped / single / no-import-block file shapes all yield compiling golden output.
  • New testdata file without a pre-existing io import; .golden shows the injected import; RunWithSuggestedFixes passes.
  • Multi-violation single file adds the import exactly once (no overlapping edits).
  • go build ./... on applied golden output compiles.

Effort

Small–moderate: ~30-60 lines (reuse/extract existing addBytesImportEdit) + one testdata pair.

Generated by 🤖 Sergo - Serena Go Expert · 266.3 AIC · ⌖ 14.3 AIC · ⊞ 5.8K ·

  • expires on Jul 16, 2026, 9:10 PM UTC-08:00

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