From 90079913db785a1c1d8fccf30cda54d88cef966f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 7 Jul 2026 18:42:06 +0000 Subject: [PATCH 1/6] linters: add writebytestring linter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flags w.Write([]byte(s)) calls where s is a string. These can be replaced with io.WriteString(w, s) which avoids an unnecessary []byte heap allocation — the io.WriteString function checks whether the writer implements io.StringWriter and short-circuits the copy. - New package: pkg/linters/writebytestring/ - Analyzer registered in cmd/linters/main.go - pkg/linters/doc.go count updated (38 → 39) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cmd/linters/main.go | 2 + pkg/linters/doc.go | 3 +- .../src/writebytestring/writebytestring.go | 68 +++++++ .../writebytestring/writebytestring.go | 177 ++++++++++++++++++ .../writebytestring/writebytestring_test.go | 16 ++ 5 files changed, 265 insertions(+), 1 deletion(-) create mode 100644 pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go create mode 100644 pkg/linters/writebytestring/writebytestring.go create mode 100644 pkg/linters/writebytestring/writebytestring_test.go diff --git a/cmd/linters/main.go b/cmd/linters/main.go index ddb71496355..a6eca1b9c31 100644 --- a/cmd/linters/main.go +++ b/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, ) } diff --git a/pkg/linters/doc.go b/pkg/linters/doc.go index f161ce222ff..322d039a1c1 100644 --- a/pkg/linters/doc.go +++ b/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. diff --git a/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go b/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go new file mode 100644 index 00000000000..84fdcaf0a52 --- /dev/null +++ b/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go @@ -0,0 +1,68 @@ +package writebytestring + +import ( + "bytes" + "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 avoid a \[\]byte allocation` + + buf.Write([]byte("world")) // want `buf\.Write\(\[\]byte\("world"\)\) can be replaced with io\.WriteString\(buf, "world"\) to avoid a \[\]byte allocation` +} + +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 avoid a \[\]byte allocation` +} + +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 avoid a \[\]byte allocation` +} + +func badCustomWriter() { + w := &customWriter{} + s := "hello" + w.Write([]byte(s)) // want `w\.Write\(\[\]byte\(s\)\) can be replaced with io\.WriteString\(w, s\) to avoid a \[\]byte allocation` +} + +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)) +} diff --git a/pkg/linters/writebytestring/writebytestring.go b/pkg/linters/writebytestring/writebytestring.go new file mode 100644 index 00000000000..c94ac1be741 --- /dev/null +++ b/pkg/linters/writebytestring/writebytestring.go @@ -0,0 +1,177 @@ +// Package writebytestring implements a Go analysis linter that flags +// w.Write([]byte(s)) calls where s is a string, which can be replaced with +// io.WriteString(w, s) to avoid an unnecessary []byte allocation. +package writebytestring + +import ( + "fmt" + "go/ast" + "go/types" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + + "github.com/github/gh-aw/pkg/linters/internal/astutil" + "github.com/github/gh-aw/pkg/linters/internal/filecheck" + "github.com/github/gh-aw/pkg/linters/internal/nolint" +) + +// Analyzer is the write-byte-string analysis pass. +var Analyzer = &analysis.Analyzer{ + Name: "writebytestring", + Doc: "reports w.Write([]byte(s)) calls where s is a string that can be replaced with io.WriteString(w, s)", + URL: "https://github.com/github/gh-aw/tree/main/pkg/linters/writebytestring", + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (any, error) { + insp, err := astutil.Inspector(pass) + if err != nil { + return nil, err + } + noLintLinesByFile := nolint.BuildLineIndex(pass, "writebytestring") + + nodeFilter := []ast.Node{ + (*ast.CallExpr)(nil), + } + + insp.Preorder(nodeFilter, func(n ast.Node) { + call, ok := n.(*ast.CallExpr) + if !ok { + return + } + + // Match .Write() + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "Write" { + return + } + if len(call.Args) != 1 { + return + } + + pos := pass.Fset.PositionFor(call.Pos(), false) + if filecheck.IsTestFile(pos.Filename) { + return + } + if nolint.HasDirective(pos, noLintLinesByFile) { + return + } + + // The single argument must be a []byte(s) conversion where s is a string. + conv, ok := call.Args[0].(*ast.CallExpr) + if !ok { + return + } + if !isByteSliceConversion(pass, conv) { + return + } + if len(conv.Args) != 1 { + return + } + strArg := conv.Args[0] + if !isStringType(pass, strArg) { + return + } + + // The receiver must implement io.Writer (has a Write([]byte) (int, error) method). + if !implementsWriter(pass, sel.X) { + return + } + + sText := astutil.NodeText(pass.Fset, strArg) + wText := astutil.NodeText(pass.Fset, sel.X) + if sText == "" || wText == "" { + return + } + + pass.Report(analysis.Diagnostic{ + Pos: call.Pos(), + End: call.End(), + Message: fmt.Sprintf("%s.Write([]byte(%s)) can be replaced with io.WriteString(%s, %s) to avoid a []byte allocation", wText, sText, wText, sText), + }) + }) + + return nil, nil +} + +// isByteSliceConversion reports whether conv is a []byte or []uint8 conversion expression. +func isByteSliceConversion(pass *analysis.Pass, conv *ast.CallExpr) bool { + funTypeInfo, ok := pass.TypesInfo.Types[conv.Fun] + if !ok || !funTypeInfo.IsType() { + return false + } + return isByteSlice(pass, conv) +} + +// isByteSlice reports whether expr has type []byte ([]uint8). +func isByteSlice(pass *analysis.Pass, expr ast.Expr) bool { + t := pass.TypesInfo.TypeOf(expr) + if t == nil { + return false + } + sl, ok := t.Underlying().(*types.Slice) + if !ok { + return false + } + elem, ok := sl.Elem().(*types.Basic) + return ok && elem.Kind() == types.Byte +} + +// isStringType reports whether expr has type string (or named string type). +func isStringType(pass *analysis.Pass, expr ast.Expr) bool { + t := pass.TypesInfo.TypeOf(expr) + if t == nil { + return false + } + basic, ok := t.Underlying().(*types.Basic) + return ok && basic.Kind() == types.String +} + +// implementsWriter reports whether expr's type implements io.Writer +// (i.e., has a method Write([]byte) (int, error)). +func implementsWriter(pass *analysis.Pass, expr ast.Expr) bool { + t := pass.TypesInfo.TypeOf(expr) + if t == nil { + return false + } + return hasWriteMethod(t) || hasWriteMethod(types.NewPointer(t)) +} + +// hasWriteMethod reports whether t (or *t) has a Write([]byte)(int,error) method. +func hasWriteMethod(t types.Type) bool { + ms := types.NewMethodSet(t) + sel := ms.Lookup(nil, "Write") + if sel == nil { + return false + } + fn, ok := sel.Obj().(*types.Func) + if !ok { + return false + } + sig, ok := fn.Type().(*types.Signature) + if !ok { + return false + } + params := sig.Params() + results := sig.Results() + if params.Len() != 1 || results.Len() != 2 { + return false + } + // Parameter must be []byte + sl, ok := params.At(0).Type().Underlying().(*types.Slice) + if !ok { + return false + } + elem, ok := sl.Elem().(*types.Basic) + if !ok || elem.Kind() != types.Byte { + return false + } + // Results must be (int, error) + intBasic, ok := results.At(0).Type().Underlying().(*types.Basic) + if !ok || intBasic.Kind() != types.Int { + return false + } + return nolint.ImplementsError(results.At(1).Type()) +} diff --git a/pkg/linters/writebytestring/writebytestring_test.go b/pkg/linters/writebytestring/writebytestring_test.go new file mode 100644 index 00000000000..5e2f458c301 --- /dev/null +++ b/pkg/linters/writebytestring/writebytestring_test.go @@ -0,0 +1,16 @@ +//go:build !integration + +package writebytestring_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/github/gh-aw/pkg/linters/writebytestring" +) + +func TestAnalyzer(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, writebytestring.Analyzer, "writebytestring") +} From fb4ecad2be8636d9a4c7a9143c874f0e69d78f80 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 7 Jul 2026 20:50:35 +0000 Subject: [PATCH 2/6] docs(adr): draft ADR-44101 for writebytestring linter Adds a draft Architecture Decision Record documenting the decision to enforce io.WriteString over w.Write([]byte(s)) via a custom go/analysis linter, consistent with the project's existing 38 custom analyzers. --- ...itebytestring-linter-for-io-writestring.md | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 docs/adr/44101-writebytestring-linter-for-io-writestring.md diff --git a/docs/adr/44101-writebytestring-linter-for-io-writestring.md b/docs/adr/44101-writebytestring-linter-for-io-writestring.md new file mode 100644 index 00000000000..662d61b4594 --- /dev/null +++ b/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 `.Write([]byte())` where `` has type `string` (or a named string type) and `` implements `io.Writer`. The linter reports the diagnostic and suggests replacing the call with `io.WriteString(, )`. 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.* From 66aa948015ee7b02c7b3295748f684c2c38e36c7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 21:31:41 +0000 Subject: [PATCH 3/6] linters: fix writebytestring pointer-receiver suggestion and add to README/spec Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/linters/README.md | 3 +++ pkg/linters/spec_test.go | 6 ++++-- .../testdata/src/writebytestring/writebytestring.go | 6 +++--- pkg/linters/writebytestring/writebytestring.go | 10 +++++++++- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/pkg/linters/README.md b/pkg/linters/README.md index c2bcc4e2d11..617bb42a21e 100644 --- a/pkg/linters/README.md +++ b/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 diff --git a/pkg/linters/spec_test.go b/pkg/linters/spec_test.go index 2e42a1431b1..f011538ad72 100644 --- a/pkg/linters/spec_test.go +++ b/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}, } } diff --git a/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go b/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go index 84fdcaf0a52..0ff6751d6c6 100644 --- a/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go +++ b/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go @@ -12,9 +12,9 @@ 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 avoid a \[\]byte allocation` + buf.Write([]byte(s)) // want `buf\.Write\(\[\]byte\(s\)\) can be replaced with io\.WriteString\(&buf, s\) to avoid a \[\]byte allocation` - buf.Write([]byte("world")) // want `buf\.Write\(\[\]byte\("world"\)\) can be replaced with io\.WriteString\(buf, "world"\) to avoid a \[\]byte allocation` + buf.Write([]byte("world")) // want `buf\.Write\(\[\]byte\("world"\)\) can be replaced with io\.WriteString\(&buf, "world"\) to avoid a \[\]byte allocation` } type myString string @@ -22,7 +22,7 @@ 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 avoid a \[\]byte allocation` + buf.Write([]byte(s)) // want `buf\.Write\(\[\]byte\(s\)\) can be replaced with io\.WriteString\(&buf, s\) to avoid a \[\]byte allocation` } func badFile() { diff --git a/pkg/linters/writebytestring/writebytestring.go b/pkg/linters/writebytestring/writebytestring.go index c94ac1be741..842d126b5d8 100644 --- a/pkg/linters/writebytestring/writebytestring.go +++ b/pkg/linters/writebytestring/writebytestring.go @@ -86,10 +86,18 @@ func run(pass *analysis.Pass) (any, error) { return } + // When the receiver is an addressable value whose Write method lives on + // the pointer type (e.g. var buf bytes.Buffer), io.WriteString requires + // the pointer form so that the interface conversion compiles. + writerArg := wText + if t := pass.TypesInfo.TypeOf(sel.X); t != nil && !hasWriteMethod(t) { + writerArg = "&" + wText + } + pass.Report(analysis.Diagnostic{ Pos: call.Pos(), End: call.End(), - Message: fmt.Sprintf("%s.Write([]byte(%s)) can be replaced with io.WriteString(%s, %s) to avoid a []byte allocation", wText, sText, wText, sText), + Message: fmt.Sprintf("%s.Write([]byte(%s)) can be replaced with io.WriteString(%s, %s) to avoid a []byte allocation", wText, sText, writerArg, sText), }) }) From f5606b261039496107c1b521bd9c6f4fe40ac1f4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 21:32:32 +0000 Subject: [PATCH 4/6] linters: make writebytestring pointer-check explicit Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/linters/writebytestring/writebytestring.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/linters/writebytestring/writebytestring.go b/pkg/linters/writebytestring/writebytestring.go index 842d126b5d8..91a6b7219fc 100644 --- a/pkg/linters/writebytestring/writebytestring.go +++ b/pkg/linters/writebytestring/writebytestring.go @@ -90,7 +90,7 @@ func run(pass *analysis.Pass) (any, error) { // the pointer type (e.g. var buf bytes.Buffer), io.WriteString requires // the pointer form so that the interface conversion compiles. writerArg := wText - if t := pass.TypesInfo.TypeOf(sel.X); t != nil && !hasWriteMethod(t) { + if t := pass.TypesInfo.TypeOf(sel.X); t != nil && !hasWriteMethod(t) && hasWriteMethod(types.NewPointer(t)) { writerArg = "&" + wText } From c390b49e0079e453e581fd7f9839bd099aadaed6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 23:00:23 +0000 Subject: [PATCH 5/6] =?UTF-8?q?linters:=20writebytestring=20=E2=80=94=20Su?= =?UTF-8?q?ggestedFix,=20types.Implements,=20interface=20test,=20wording?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- .../src/writebytestring/writebytestring.go | 16 ++- .../writebytestring/writebytestring.go | 97 ++++++++++--------- 2 files changed, 63 insertions(+), 50 deletions(-) diff --git a/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go b/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go index 0ff6751d6c6..e7da4db459b 100644 --- a/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go +++ b/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go @@ -2,6 +2,7 @@ package writebytestring import ( "bytes" + "io" "os" ) @@ -12,9 +13,9 @@ 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 avoid a \[\]byte allocation` + buf.Write([]byte(s)) // want `buf\.Write\(\[\]byte\(s\)\) can be replaced with io\.WriteString\(&buf, s\) to avoid a \[\]byte allocation when the writer implements io\.StringWriter` - buf.Write([]byte("world")) // want `buf\.Write\(\[\]byte\("world"\)\) can be replaced with io\.WriteString\(&buf, "world"\) to avoid a \[\]byte allocation` + buf.Write([]byte("world")) // want `buf\.Write\(\[\]byte\("world"\)\) can be replaced with io\.WriteString\(&buf, "world"\) to avoid a \[\]byte allocation when the writer implements io\.StringWriter` } type myString string @@ -22,7 +23,7 @@ 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 avoid a \[\]byte allocation` + buf.Write([]byte(s)) // want `buf\.Write\(\[\]byte\(s\)\) can be replaced with io\.WriteString\(&buf, s\) to avoid a \[\]byte allocation when the writer implements io\.StringWriter` } func badFile() { @@ -32,13 +33,18 @@ func badFile() { } defer f.Close() msg := "hello" - f.Write([]byte(msg)) // want `f\.Write\(\[\]byte\(msg\)\) can be replaced with io\.WriteString\(f, msg\) to avoid a \[\]byte allocation` + f.Write([]byte(msg)) // want `f\.Write\(\[\]byte\(msg\)\) can be replaced with io\.WriteString\(f, msg\) to avoid a \[\]byte allocation when 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 avoid a \[\]byte allocation` + w.Write([]byte(s)) // want `w\.Write\(\[\]byte\(s\)\) can be replaced with io\.WriteString\(w, s\) to avoid a \[\]byte allocation when 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 avoid a \[\]byte allocation when the writer implements io\.StringWriter` } func goodBytes() { diff --git a/pkg/linters/writebytestring/writebytestring.go b/pkg/linters/writebytestring/writebytestring.go index 91a6b7219fc..ba0ff5cce54 100644 --- a/pkg/linters/writebytestring/writebytestring.go +++ b/pkg/linters/writebytestring/writebytestring.go @@ -6,6 +6,7 @@ package writebytestring import ( "fmt" "go/ast" + "go/token" "go/types" "golang.org/x/tools/go/analysis" @@ -16,6 +17,26 @@ import ( "github.com/github/gh-aw/pkg/linters/internal/nolint" ) +// writerIface is a synthetic *types.Interface matching io.Writer: +// +// Write(p []byte) (n int, err error) +// +// Built once at package init so it can be reused across analysis passes. +var writerIface = func() *types.Interface { + byteSlice := types.NewSlice(types.Typ[types.Byte]) + errType := types.Universe.Lookup("error").Type() + params := types.NewTuple(types.NewVar(token.NoPos, nil, "p", byteSlice)) + results := types.NewTuple( + types.NewVar(token.NoPos, nil, "n", types.Typ[types.Int]), + types.NewVar(token.NoPos, nil, "err", errType), + ) + sig := types.NewSignatureType(nil, nil, nil, params, results, false) + method := types.NewFunc(token.NoPos, nil, "Write", sig) + iface := types.NewInterfaceType([]*types.Func{method}, nil) + iface.Complete() + return iface +}() + // Analyzer is the write-byte-string analysis pass. var Analyzer = &analysis.Analyzer{ Name: "writebytestring", @@ -67,15 +88,12 @@ func run(pass *analysis.Pass) (any, error) { if !isByteSliceConversion(pass, conv) { return } - if len(conv.Args) != 1 { - return - } strArg := conv.Args[0] if !isStringType(pass, strArg) { return } - // The receiver must implement io.Writer (has a Write([]byte) (int, error) method). + // The receiver must implement io.Writer. if !implementsWriter(pass, sel.X) { return } @@ -90,14 +108,15 @@ func run(pass *analysis.Pass) (any, error) { // the pointer type (e.g. var buf bytes.Buffer), io.WriteString requires // the pointer form so that the interface conversion compiles. writerArg := wText - if t := pass.TypesInfo.TypeOf(sel.X); t != nil && !hasWriteMethod(t) && hasWriteMethod(types.NewPointer(t)) { + if t := pass.TypesInfo.TypeOf(sel.X); t != nil && !types.Implements(t, writerIface) { writerArg = "&" + wText } pass.Report(analysis.Diagnostic{ - Pos: call.Pos(), - End: call.End(), - Message: fmt.Sprintf("%s.Write([]byte(%s)) can be replaced with io.WriteString(%s, %s) to avoid a []byte allocation", wText, sText, writerArg, sText), + Pos: call.Pos(), + End: call.End(), + Message: fmt.Sprintf("%s.Write([]byte(%s)) can be replaced with io.WriteString(%s, %s) to avoid a []byte allocation when the writer implements io.StringWriter", wText, sText, writerArg, sText), + SuggestedFixes: buildFix(call, writerArg, sText), }) }) @@ -137,49 +156,37 @@ func isStringType(pass *analysis.Pass, expr ast.Expr) bool { return ok && basic.Kind() == types.String } -// implementsWriter reports whether expr's type implements io.Writer -// (i.e., has a method Write([]byte) (int, error)). +// implementsWriter reports whether expr's type implements io.Writer. +// It uses types.Implements against a synthetic io.Writer interface so the check +// is idiomatic and avoids manually re-implementing the signature comparison. +// Only T and *T are tried; **T is never constructed so pointer types are not +// double-wrapped. func implementsWriter(pass *analysis.Pass, expr ast.Expr) bool { t := pass.TypesInfo.TypeOf(expr) if t == nil { return false } - return hasWriteMethod(t) || hasWriteMethod(types.NewPointer(t)) -} - -// hasWriteMethod reports whether t (or *t) has a Write([]byte)(int,error) method. -func hasWriteMethod(t types.Type) bool { - ms := types.NewMethodSet(t) - sel := ms.Lookup(nil, "Write") - if sel == nil { - return false - } - fn, ok := sel.Obj().(*types.Func) - if !ok { - return false - } - sig, ok := fn.Type().(*types.Signature) - if !ok { - return false - } - params := sig.Params() - results := sig.Results() - if params.Len() != 1 || results.Len() != 2 { - return false + if types.Implements(t, writerIface) { + return true } - // Parameter must be []byte - sl, ok := params.At(0).Type().Underlying().(*types.Slice) - if !ok { - return false - } - elem, ok := sl.Elem().(*types.Basic) - if !ok || elem.Kind() != types.Byte { + // Only add a pointer wrapper when t is not already a pointer, to avoid + // constructing a semantically meaningless **T type. + if _, isPtr := t.Underlying().(*types.Pointer); isPtr { return false } - // Results must be (int, error) - intBasic, ok := results.At(0).Type().Underlying().(*types.Basic) - if !ok || intBasic.Kind() != types.Int { - return false - } - return nolint.ImplementsError(results.At(1).Type()) + return types.Implements(types.NewPointer(t), writerIface) +} + +// buildFix returns a SuggestedFix rewriting w.Write([]byte(s)) to io.WriteString(w, s). +// 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), + }}, + }} } From 3bcd03eceaaae9adad64a10a68d55e9a43a67a2e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 23:05:43 +0000 Subject: [PATCH 6/6] =?UTF-8?q?linters:=20writebytestring=20=E2=80=94=20de?= =?UTF-8?q?fensive=20&T=20check,=20accurate=20message=20wording,=20variabl?= =?UTF-8?q?e=20rename?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- .../testdata/src/writebytestring/writebytestring.go | 12 ++++++------ pkg/linters/writebytestring/writebytestring.go | 8 +++++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go b/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go index e7da4db459b..fafdaa60ec6 100644 --- a/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go +++ b/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go @@ -13,9 +13,9 @@ 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 avoid a \[\]byte allocation when the writer implements io\.StringWriter` + 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 avoid a \[\]byte allocation when 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 @@ -23,7 +23,7 @@ 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 avoid a \[\]byte allocation when the writer implements io\.StringWriter` + 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() { @@ -33,18 +33,18 @@ func badFile() { } defer f.Close() msg := "hello" - f.Write([]byte(msg)) // want `f\.Write\(\[\]byte\(msg\)\) can be replaced with io\.WriteString\(f, msg\) to avoid a \[\]byte allocation when the writer implements io\.StringWriter` + 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 avoid a \[\]byte allocation when the writer implements io\.StringWriter` + 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 avoid a \[\]byte allocation when the writer implements io\.StringWriter` + 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() { diff --git a/pkg/linters/writebytestring/writebytestring.go b/pkg/linters/writebytestring/writebytestring.go index ba0ff5cce54..932d3c0b200 100644 --- a/pkg/linters/writebytestring/writebytestring.go +++ b/pkg/linters/writebytestring/writebytestring.go @@ -108,14 +108,16 @@ func run(pass *analysis.Pass) (any, error) { // the pointer type (e.g. var buf bytes.Buffer), io.WriteString requires // the pointer form so that the interface conversion compiles. writerArg := wText - if t := pass.TypesInfo.TypeOf(sel.X); t != nil && !types.Implements(t, writerIface) { + if t := pass.TypesInfo.TypeOf(sel.X); t != nil && + !types.Implements(t, writerIface) && + types.Implements(types.NewPointer(t), writerIface) { writerArg = "&" + wText } pass.Report(analysis.Diagnostic{ Pos: call.Pos(), End: call.End(), - Message: fmt.Sprintf("%s.Write([]byte(%s)) can be replaced with io.WriteString(%s, %s) to avoid a []byte allocation when the writer implements io.StringWriter", wText, sText, writerArg, sText), + Message: fmt.Sprintf("%s.Write([]byte(%s)) can be replaced with io.WriteString(%s, %s) to potentially avoid a []byte allocation if the writer implements io.StringWriter", wText, sText, writerArg, sText), SuggestedFixes: buildFix(call, writerArg, sText), }) }) @@ -171,7 +173,7 @@ func implementsWriter(pass *analysis.Pass, expr ast.Expr) bool { } // Only add a pointer wrapper when t is not already a pointer, to avoid // constructing a semantically meaningless **T type. - if _, isPtr := t.Underlying().(*types.Pointer); isPtr { + if _, alreadyPtr := t.Underlying().(*types.Pointer); alreadyPtr { return false } return types.Implements(types.NewPointer(t), writerIface)