[linter-miner] linters: add writebytestring — flag w.Write([]byte(s)) calls that should use io.WriteString

cmd/linters/main.go

@@ -58,6 +58,7 @@ import (
 	"github.com/github/gh-aw/pkg/linters/tolowerequalfold"
 	"github.com/github/gh-aw/pkg/linters/uncheckedtypeassertion"
 	"github.com/github/gh-aw/pkg/linters/wgdonenotdeferred"
+	"github.com/github/gh-aw/pkg/linters/writebytestring"
 )
 
 func main() {
@@ -104,5 +105,6 @@ func main() {
 		tolowerequalfold.Analyzer,
 		uncheckedtypeassertion.Analyzer,
 		wgdonenotdeferred.Analyzer,
+		writebytestring.Analyzer,
 	)
 }

docs/adr/44101-writebytestring-linter-for-io-writestring.md

@@ -0,0 +1,51 @@
+# ADR-44101: Enforce io.WriteString Over w.Write([]byte(s)) via a Custom Static Analysis Linter
+
+**Date**: 2026-07-07
+**Status**: Draft
+**Deciders**: Unknown (generated by Linter Miner automation — review required)
+
+---
+
+### Context
+
+Across the codebase, several call sites use `w.Write([]byte(s))` where `s` is a string value. This pattern incurs an unnecessary heap allocation: the runtime must copy the string's bytes into a new `[]byte` slice before passing it to the writer. Go's standard library provides `io.WriteString(w, s)`, which checks whether `w` also implements `io.StringWriter`; if it does, the underlying writer can accept the string directly, eliminating the allocation. The codebase already has 38 custom `go/analysis` linters catching similar Go anti-patterns (e.g., `appendbytestring` for `append(b, []byte(s)...)`), so a new linter fits the established pattern-enforcement approach. Six real instances of this anti-pattern were identified in production code under `pkg/` and `cmd/` (including `pkg/logger/logger.go`, `pkg/workflow/strings.go`, `pkg/parser/schedule_fuzzy_scatter.go`, and others).
+
+### Decision
+
+We will add a custom `go/analysis` linter named `writebytestring` to `pkg/linters/writebytestring/` that diagnoses any call of the form `<w>.Write([]byte(<s>))` where `<s>` has type `string` (or a named string type) and `<w>` implements `io.Writer`. The linter reports the diagnostic and suggests replacing the call with `io.WriteString(<w>, <s>)`. It skips test files and lines annotated with `//nolint:writebytestring`. This keeps enforcement automatic, consistent with all other custom linters, and visible to CI without requiring manual code review vigilance.
+
+### Alternatives Considered
+
+#### Alternative 1: Rely on an external or upstream linter (e.g., staticcheck, golangci-lint SA1006)
+
+staticcheck's `S1030` rule covers a subset of this pattern, but the project uses its own custom linter binary (`cmd/linters`) rather than `staticcheck` or `golangci-lint`. Adopting an external tool solely for this check would introduce a new dependency and configuration surface, diverge from the established internal linter approach, and require integrating a separate analysis pipeline. Rejected in favour of consistency with the existing 38 custom analyzers.
+
+#### Alternative 2: Enforce via code review guidelines and documentation only
+
+Adding a style guide entry instructing developers to prefer `io.WriteString` would avoid tooling overhead but relies on human vigilance. The existing anti-pattern instances survived code review for an unknown period, showing that documentation-only approaches are insufficient for mechanical patterns. This also creates inconsistency given that structurally similar patterns (`appendbytestring`, `tolowerequalfold`) are already enforced by linters. Rejected.
+
+#### Alternative 3: Fix all existing instances without adding a linter
+
+One-time remediation of the six known call sites would eliminate current debt but not prevent recurrence. Without a linter, new instances would be silently introduced. Rejected as an incomplete solution.
+
+### Consequences
+
+#### Positive
+- Eliminates an unnecessary `[]byte` heap allocation at call sites where the underlying writer implements `io.StringWriter` (e.g., `bytes.Buffer`, `os.File`), reducing GC pressure in hot paths such as hashing and logging.
+- Prevents recurrence: all future `w.Write([]byte(s))` anti-patterns are caught at static analysis time in CI, with no ongoing manual review cost.
+- Consistent with the existing linter infrastructure: the new analyzer integrates via the same `go/analysis` framework, `//nolint:writebytestring` escape hatch, and test-data pattern as all other custom analyzers.
+- Raises the count of active analyzers from 38 to 39 and is documented in `pkg/linters/doc.go`, preserving discoverability.
+
+#### Negative
+- Adds a maintenance burden: the linter must be updated if internal helper packages (`astutil`, `filecheck`, `nolint`) change their APIs or semantics.
+- `io.WriteString` only avoids an allocation when the concrete writer also implements `io.StringWriter`; for writers that do not, the call is functionally equivalent to `Write([]byte(s))` with no performance benefit, making some diagnostics semantically neutral rather than strictly improvements.
+- Developers unfamiliar with the linter collection must learn the `//nolint:writebytestring` suppression mechanism for intentional `[]byte` conversions.
+
+#### Neutral
+- The linter was generated by the Linter Miner automated workflow; human review of the detection logic (particularly `isByteSliceConversion`, `implementsWriter`, and the `[]uint8` handling) is needed before this ADR moves to Accepted.
+- The change increments the documented analyzer count in `pkg/linters/doc.go` from 38 to 39; teams maintaining integration lists of analyzer names should update their inventories.
+- Test coverage uses the standard `analysistest` framework and includes bad/good/named-type/suppressed cases, but does not exercise the linter against the six real call sites identified in the codebase — a follow-up PR to fix those instances should be expected.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/linters/README.md

@@ -48,6 +48,7 @@ This package currently provides custom Go analyzers in the following subpackages
 - `tolowerequalfold` — reports case-insensitive string comparisons using `strings.ToLower`/`ToUpper` that should use `strings.EqualFold`.
 - `uncheckedtypeassertion` — reports single-value type assertions where unchecked panics are possible.
 - `wgdonenotdeferred` — reports non-deferred `sync.WaitGroup.Done()` calls that can deadlock on panics or early returns.
+- `writebytestring` — reports `w.Write([]byte(s))` calls where `s` is a string, which can be replaced with `io.WriteString` to avoid an unnecessary `[]byte` allocation.
 - `internal` — shared helper packages for analyzers (file checks and `nolint` handling).
 
 ## Public API
@@ -98,6 +99,7 @@ This package currently provides custom Go analyzers in the following subpackages
 | `tolowerequalfold` | Custom `go/analysis` analyzer that flags case-insensitive comparisons via `strings.ToLower`/`ToUpper` that should use `strings.EqualFold` |
 | `uncheckedtypeassertion` | Custom `go/analysis` analyzer that flags unchecked single-value type assertions |
 | `wgdonenotdeferred` | Custom `go/analysis` analyzer that flags non-deferred `sync.WaitGroup.Done()` calls |
+| `writebytestring` | Custom `go/analysis` analyzer that flags `w.Write([]byte(s))` calls where `s` is a string that can be replaced with `io.WriteString` |
 | `internal` | Shared helper subpackages used by analyzers (`internal/filecheck`, `internal/nolint`) |
 
 ### Namespace exports
@@ -208,6 +210,7 @@ _ = timesleepnocontext.Analyzer
 - `github.com/github/gh-aw/pkg/linters/tolowerequalfold` — to-lower-equal-fold analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/uncheckedtypeassertion` — unchecked-type-assertion analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/wgdonenotdeferred` — wg-done-not-deferred analyzer subpackage
+- `github.com/github/gh-aw/pkg/linters/writebytestring` — write-byte-string analyzer subpackage
 
 **Transitive / Internal helpers**:
 - `github.com/github/gh-aw/pkg/linters/internal/filecheck` — shared file-path filtering helpers used by multiple analyzers

pkg/linters/doc.go

@@ -1,6 +1,6 @@
 // Package linters is a namespace for gh-aw's custom Go analysis linters.
 //
-// The actual analyzers are implemented in subpackages. All 38 active analyzers:
+// The actual analyzers are implemented in subpackages. All 39 active analyzers:
 //
 //   - appendbytestring — flags append(b, []byte(s)...) calls where s is a string that can be simplified to append(b, s...)
 //   - contextcancelnotdeferred — flags context cancel functions called directly instead of deferred
@@ -40,6 +40,7 @@
 //   - tolowerequalfold — flags case-insensitive comparisons via ToLower/ToUpper that should use EqualFold
 //   - uncheckedtypeassertion — flags unchecked single-value type assertions
 //   - wgdonenotdeferred — flags non-deferred sync.WaitGroup.Done() calls
+//   - writebytestring — flags w.Write([]byte(s)) calls where s is a string that can be replaced with io.WriteString(w, s)
 //
 // The package also exposes a compatibility alias (ErrorMessageAnalyzer) that
 // points to the errormessage subpackage analyzer.

pkg/linters/spec_test.go

@@ -49,6 +49,7 @@ import (
 	"github.com/github/gh-aw/pkg/linters/tolowerequalfold"
 	"github.com/github/gh-aw/pkg/linters/uncheckedtypeassertion"
 	"github.com/github/gh-aw/pkg/linters/wgdonenotdeferred"
+	"github.com/github/gh-aw/pkg/linters/writebytestring"
 )
 
 // TestSpec tests derive from pkg/linters/README.md. They enforce the documented
@@ -64,7 +65,7 @@ type docAnalyzer struct {
 }
 
 // documentedAnalyzers returns the analyzer subpackages documented in the README
-// "Public API > Subpackages" table. The README documents 38 analyzers
+// "Public API > Subpackages" table. The README documents 39 analyzers
 // subpackages (the non-analyzer `internal` helper subpackage is excluded because
 // it exposes no Analyzer).
 //
@@ -76,7 +77,7 @@ type docAnalyzer struct {
 //	manualmutexunlock, osexitinlibrary, ossetenvlibrary, panic-in-library-code, rawloginlib,
 //	regexpcompileinfunction, seenmapbool, sortslice, sprintferrdot, sprintferrorsnew, ssljson,
 //	strconvparseignorederror, stringreplaceminusone, stringsindexcontains, timeafterleak, timesleepnocontext,
-//	tolowerequalfold, uncheckedtypeassertion, wgdonenotdeferred
+//	tolowerequalfold, uncheckedtypeassertion, wgdonenotdeferred, writebytestring
 func documentedAnalyzers() []docAnalyzer {
 	return []docAnalyzer{
 		{"appendbytestring", appendbytestring.Analyzer},
@@ -117,6 +118,7 @@ func documentedAnalyzers() []docAnalyzer {
 		{"tolowerequalfold", tolowerequalfold.Analyzer},
 		{"uncheckedtypeassertion", uncheckedtypeassertion.Analyzer},
 		{"wgdonenotdeferred", wgdonenotdeferred.Analyzer},
+		{"writebytestring", writebytestring.Analyzer},
 	}
 }
 

pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go

@@ -0,0 +1,74 @@
+package writebytestring
+
+import (
+	"bytes"
+	"io"
+	"os"
+)
+
+type customWriter struct{}
+
+func (c *customWriter) Write(p []byte) (int, error) { return len(p), nil }
+
+func bad() {
+	var buf bytes.Buffer
+	s := "hello"
+	buf.Write([]byte(s)) // want `buf\.Write\(\[\]byte\(s\)\) can be replaced with io\.WriteString\(&buf, s\) to potentially avoid a \[\]byte allocation if the writer implements io\.StringWriter`
+
+	buf.Write([]byte("world")) // want `buf\.Write\(\[\]byte\("world"\)\) can be replaced with io\.WriteString\(&buf, "world"\) to potentially avoid a \[\]byte allocation if the writer implements io\.StringWriter`
+}
+
+type myString string
+
+func badNamedString() {
+	var buf bytes.Buffer
+	s := myString("hello")
+	buf.Write([]byte(s)) // want `buf\.Write\(\[\]byte\(s\)\) can be replaced with io\.WriteString\(&buf, s\) to potentially avoid a \[\]byte allocation if the writer implements io\.StringWriter`
+}
+
+func badFile() {
+	f, err := os.Create("/tmp/test.txt")
+	if err != nil {
+		return
+	}
+	defer f.Close()
+	msg := "hello"
+	f.Write([]byte(msg)) // want `f\.Write\(\[\]byte\(msg\)\) can be replaced with io\.WriteString\(f, msg\) to potentially avoid a \[\]byte allocation if the writer implements io\.StringWriter`
+}
+
+func badCustomWriter() {
+	w := &customWriter{}
+	s := "hello"
+	w.Write([]byte(s)) // want `w\.Write\(\[\]byte\(s\)\) can be replaced with io\.WriteString\(w, s\) to potentially avoid a \[\]byte allocation if the writer implements io\.StringWriter`
+}
+
+func badInterfaceWriter(w io.Writer) {
+	s := "hello"
+	w.Write([]byte(s)) // want `w\.Write\(\[\]byte\(s\)\) can be replaced with io\.WriteString\(w, s\) to potentially avoid a \[\]byte allocation if the writer implements io\.StringWriter`
+}
+
+func goodBytes() {
+	var buf bytes.Buffer
+	b := []byte("hello")
+	buf.Write(b) // already []byte — no conversion
+}
+
+func goodWriteString() {
+	// Using io.WriteString is the idiomatic form.
+	var buf bytes.Buffer
+	s := "hello"
+	buf.WriteString(s)
+}
+
+func goodNotString() {
+	var buf bytes.Buffer
+	n := 42
+	buf.Write([]byte{byte(n)})
+}
+
+func suppressed() {
+	var buf bytes.Buffer
+	s := "hello"
+	//nolint:writebytestring
+	buf.Write([]byte(s))
+}