From 534ceb3c0a9d0f1e6c15ccd9c8afbcf0b8ed1a40 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Jul 2026 05:22:06 +0000 Subject: [PATCH 1/4] Initial plan From a7495657ada89f47bb2be2696393b1b546305731 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Jul 2026 05:40:19 +0000 Subject: [PATCH 2/4] Fix bytescomparestring rationale and single-import suggested fix Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../bytescomparestring/bytescomparestring.go | 29 +++++++++++++++---- .../bytescomparestring/bytescomparestring.go | 6 ++-- .../bytescomparestring.go.golden | 6 ++-- .../src/bytescomparestring/noimport.go | 4 +-- .../src/bytescomparestring/noimport.go.golden | 4 +-- .../src/bytescomparestring/singleimport.go | 8 +++++ .../bytescomparestring/singleimport.go.golden | 11 +++++++ pkg/linters/doc.go | 2 +- 8 files changed, 54 insertions(+), 16 deletions(-) create mode 100644 pkg/linters/bytescomparestring/testdata/src/bytescomparestring/singleimport.go create mode 100644 pkg/linters/bytescomparestring/testdata/src/bytescomparestring/singleimport.go.golden diff --git a/pkg/linters/bytescomparestring/bytescomparestring.go b/pkg/linters/bytescomparestring/bytescomparestring.go index 99ad1ac8cb9..3a06111d4f4 100644 --- a/pkg/linters/bytescomparestring/bytescomparestring.go +++ b/pkg/linters/bytescomparestring/bytescomparestring.go @@ -1,7 +1,7 @@ // Package bytescomparestring implements a Go analysis linter that flags // string(a) == string(b) and string(a) != string(b) comparisons where both -// a and b are []byte values, which should use bytes.Equal(a, b) instead to -// avoid unnecessary allocations. +// a and b are []byte values, which should use bytes.Equal(a, b) instead for +// clearer intent. package bytescomparestring import ( @@ -75,19 +75,18 @@ func run(pass *analysis.Pass) (any, error) { return } - op := bin.Op.String() if bin.Op == token.EQL { pass.Report(analysis.Diagnostic{ Pos: bin.Pos(), End: bin.End(), - Message: fmt.Sprintf("string(%s) == string(%s) allocates; use bytes.Equal(%s, %s) instead", lText, rText, lText, rText), + Message: fmt.Sprintf("string(%s) == string(%s) is a []byte comparison written the long way; use bytes.Equal(%s, %s) for clearer intent", lText, rText, lText, rText), SuggestedFixes: buildFix(pass, bin, fmt.Sprintf("bytes.Equal(%s, %s)", lText, rText)), }) } else { pass.Report(analysis.Diagnostic{ Pos: bin.Pos(), End: bin.End(), - Message: fmt.Sprintf("string(%s) %s string(%s) allocates; use !bytes.Equal(%s, %s) instead", lText, op, rText, lText, rText), + Message: fmt.Sprintf("string(%s) != string(%s) is a []byte comparison written the long way; use !bytes.Equal(%s, %s) for clearer intent", lText, rText, lText, rText), SuggestedFixes: buildFix(pass, bin, fmt.Sprintf("!bytes.Equal(%s, %s)", lText, rText)), }) } @@ -149,6 +148,26 @@ func addBytesImportEdit(pass *analysis.Pass, pos token.Pos) (analysis.TextEdit, }, true } + // If the file has a single non-grouped import declaration, convert it to a + // grouped import block while adding "bytes". + for _, decl := range file.Decls { + genDecl, ok := decl.(*ast.GenDecl) + if !ok || genDecl.Tok != token.IMPORT || genDecl.Lparen.IsValid() || len(genDecl.Specs) != 1 { + continue + } + + specText := astutil.NodeText(pass.Fset, genDecl.Specs[0]) + if specText == "" { + continue + } + + return analysis.TextEdit{ + Pos: genDecl.Pos(), + End: genDecl.End(), + NewText: []byte("import (\n\t" + specText + "\n\t\"" + bytesPkg + "\"\n)"), + }, true + } + // No grouped import block; insert a standalone import after the package name. return analysis.TextEdit{ Pos: file.Name.End(), diff --git a/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/bytescomparestring.go b/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/bytescomparestring.go index 6c4b87dbc8c..4ec2fcf2a63 100644 --- a/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/bytescomparestring.go +++ b/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/bytescomparestring.go @@ -3,17 +3,17 @@ package bytescomparestring import "bytes" func badEqual(a, b []byte) bool { - return string(a) == string(b) // want `string\(a\) == string\(b\) allocates; use bytes\.Equal\(a, b\) instead` + return string(a) == string(b) // want `string\(a\) == string\(b\) is a \[\]byte comparison written the long way; use bytes\.Equal\(a, b\) for clearer intent` } func badNotEqual(a, b []byte) bool { - return string(a) != string(b) // want `string\(a\) != string\(b\) allocates; use !bytes\.Equal\(a, b\) instead` + return string(a) != string(b) // want `string\(a\) != string\(b\) is a \[\]byte comparison written the long way; use !bytes\.Equal\(a, b\) for clearer intent` } type myBytes []byte func badNamedType(a, b myBytes) bool { - return string(a) == string(b) // want `string\(a\) == string\(b\) allocates; use bytes\.Equal\(a, b\) instead` + return string(a) == string(b) // want `string\(a\) == string\(b\) is a \[\]byte comparison written the long way; use bytes\.Equal\(a, b\) for clearer intent` } func goodBytesEqual(a, b []byte) bool { diff --git a/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/bytescomparestring.go.golden b/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/bytescomparestring.go.golden index 398552177cb..0d34c21d82d 100644 --- a/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/bytescomparestring.go.golden +++ b/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/bytescomparestring.go.golden @@ -3,17 +3,17 @@ package bytescomparestring import "bytes" func badEqual(a, b []byte) bool { - return bytes.Equal(a, b) // want `string\(a\) == string\(b\) allocates; use bytes\.Equal\(a, b\) instead` + return bytes.Equal(a, b) // want `string\(a\) == string\(b\) is a \[\]byte comparison written the long way; use bytes\.Equal\(a, b\) for clearer intent` } func badNotEqual(a, b []byte) bool { - return !bytes.Equal(a, b) // want `string\(a\) != string\(b\) allocates; use !bytes\.Equal\(a, b\) instead` + return !bytes.Equal(a, b) // want `string\(a\) != string\(b\) is a \[\]byte comparison written the long way; use !bytes\.Equal\(a, b\) for clearer intent` } type myBytes []byte func badNamedType(a, b myBytes) bool { - return bytes.Equal(a, b) // want `string\(a\) == string\(b\) allocates; use bytes\.Equal\(a, b\) instead` + return bytes.Equal(a, b) // want `string\(a\) == string\(b\) is a \[\]byte comparison written the long way; use bytes\.Equal\(a, b\) for clearer intent` } func goodBytesEqual(a, b []byte) bool { diff --git a/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/noimport.go b/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/noimport.go index 9a7a119f8fd..0ce81214cc0 100644 --- a/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/noimport.go +++ b/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/noimport.go @@ -1,9 +1,9 @@ package bytescomparestring func noImportEqual(a, b []byte) bool { - return string(a) == string(b) // want `string\(a\) == string\(b\) allocates; use bytes\.Equal\(a, b\) instead` + return string(a) == string(b) // want `string\(a\) == string\(b\) is a \[\]byte comparison written the long way; use bytes\.Equal\(a, b\) for clearer intent` } func noImportNotEqual(a, b []byte) bool { - return string(a) != string(b) // want `string\(a\) != string\(b\) allocates; use !bytes\.Equal\(a, b\) instead` + return string(a) != string(b) // want `string\(a\) != string\(b\) is a \[\]byte comparison written the long way; use !bytes\.Equal\(a, b\) for clearer intent` } diff --git a/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/noimport.go.golden b/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/noimport.go.golden index e42adc1baf1..6da48c240f4 100644 --- a/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/noimport.go.golden +++ b/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/noimport.go.golden @@ -3,9 +3,9 @@ package bytescomparestring import "bytes" func noImportEqual(a, b []byte) bool { - return bytes.Equal(a, b) // want `string\(a\) == string\(b\) allocates; use bytes\.Equal\(a, b\) instead` + return bytes.Equal(a, b) // want `string\(a\) == string\(b\) is a \[\]byte comparison written the long way; use bytes\.Equal\(a, b\) for clearer intent` } func noImportNotEqual(a, b []byte) bool { - return !bytes.Equal(a, b) // want `string\(a\) != string\(b\) allocates; use !bytes\.Equal\(a, b\) instead` + return !bytes.Equal(a, b) // want `string\(a\) != string\(b\) is a \[\]byte comparison written the long way; use !bytes\.Equal\(a, b\) for clearer intent` } diff --git a/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/singleimport.go b/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/singleimport.go new file mode 100644 index 00000000000..f047cf777dd --- /dev/null +++ b/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/singleimport.go @@ -0,0 +1,8 @@ +package bytescomparestring + +import "fmt" + +func singleImportEqual(a, b []byte) bool { + fmt.Println("compare") + return string(a) == string(b) // want `string\(a\) == string\(b\) is a \[\]byte comparison written the long way; use bytes\.Equal\(a, b\) for clearer intent` +} diff --git a/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/singleimport.go.golden b/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/singleimport.go.golden new file mode 100644 index 00000000000..8b6af862389 --- /dev/null +++ b/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/singleimport.go.golden @@ -0,0 +1,11 @@ +package bytescomparestring + +import ( + "fmt" + "bytes" +) + +func singleImportEqual(a, b []byte) bool { + fmt.Println("compare") + return bytes.Equal(a, b) // want `string\(a\) == string\(b\) is a \[\]byte comparison written the long way; use bytes\.Equal\(a, b\) for clearer intent` +} diff --git a/pkg/linters/doc.go b/pkg/linters/doc.go index e4752598579..8c8fb89cecf 100644 --- a/pkg/linters/doc.go +++ b/pkg/linters/doc.go @@ -3,7 +3,7 @@ // All 44 active analyzers: // // - appendbytestring — flags append(b, []byte(s)...) calls where s is a string that can be simplified to append(b, s...) -// - bytescomparestring — flags string(a) == string(b) and string(a) != string(b) comparisons where a and b are []byte values that should use bytes.Equal instead +// - bytescomparestring — flags string(a) == string(b) and string(a) != string(b) comparisons where a and b are []byte values and recommends bytes.Equal for clearer intent // - contextcancelnotdeferred — flags context cancel functions called directly instead of deferred // - ctxbackground — flags context.Background() inside functions that already receive a context // - deferinloop — flags defer statements placed directly inside for or range loop bodies From af87de58195a863a626b68f939e83f4d0418ad57 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Jul 2026 07:33:37 +0000 Subject: [PATCH 3/4] Address review feedback: fix Analyzer.Doc, import order, single-import guard, and != coverage Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- .../bytescomparestring/bytescomparestring.go | 38 ++++++++++--------- .../src/bytescomparestring/singleimport.go | 5 +++ .../bytescomparestring/singleimport.go.golden | 7 +++- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/pkg/linters/bytescomparestring/bytescomparestring.go b/pkg/linters/bytescomparestring/bytescomparestring.go index 3a06111d4f4..7de889f4d7f 100644 --- a/pkg/linters/bytescomparestring/bytescomparestring.go +++ b/pkg/linters/bytescomparestring/bytescomparestring.go @@ -23,7 +23,7 @@ const bytesPkg = "bytes" // Analyzer is the bytes-compare-string analysis pass. var Analyzer = &analysis.Analyzer{ Name: "bytescomparestring", - Doc: "reports string(a) == string(b) and string(a) != string(b) comparisons where a and b are []byte values that should use bytes.Equal instead", + Doc: "flags string(a) == string(b) and string(a) != string(b) as []byte comparisons written the long way; use bytes.Equal for clearer intent", URL: "https://github.com/github/gh-aw/tree/main/pkg/linters/bytescomparestring", Requires: []*analysis.Analyzer{inspect.Analyzer}, Run: run, @@ -148,24 +148,26 @@ func addBytesImportEdit(pass *analysis.Pass, pos token.Pos) (analysis.TextEdit, }, true } - // If the file has a single non-grouped import declaration, convert it to a - // grouped import block while adding "bytes". - for _, decl := range file.Decls { - genDecl, ok := decl.(*ast.GenDecl) - if !ok || genDecl.Tok != token.IMPORT || genDecl.Lparen.IsValid() || len(genDecl.Specs) != 1 { - continue + // If the file has exactly one import (non-grouped), convert it to a grouped + // import block while adding "bytes" before it (alphabetical order). + if len(file.Imports) == 1 { + for _, decl := range file.Decls { + genDecl, ok := decl.(*ast.GenDecl) + if !ok || genDecl.Tok != token.IMPORT || genDecl.Lparen.IsValid() || len(genDecl.Specs) != 1 { + continue + } + + specText := astutil.NodeText(pass.Fset, genDecl.Specs[0]) + if specText == "" { + continue + } + + return analysis.TextEdit{ + Pos: genDecl.Pos(), + End: genDecl.End(), + NewText: []byte("import (\n\t\"" + bytesPkg + "\"\n\t" + specText + "\n)"), + }, true } - - specText := astutil.NodeText(pass.Fset, genDecl.Specs[0]) - if specText == "" { - continue - } - - return analysis.TextEdit{ - Pos: genDecl.Pos(), - End: genDecl.End(), - NewText: []byte("import (\n\t" + specText + "\n\t\"" + bytesPkg + "\"\n)"), - }, true } // No grouped import block; insert a standalone import after the package name. diff --git a/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/singleimport.go b/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/singleimport.go index f047cf777dd..7ebb0d3be2c 100644 --- a/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/singleimport.go +++ b/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/singleimport.go @@ -6,3 +6,8 @@ func singleImportEqual(a, b []byte) bool { fmt.Println("compare") return string(a) == string(b) // want `string\(a\) == string\(b\) is a \[\]byte comparison written the long way; use bytes\.Equal\(a, b\) for clearer intent` } + +func singleImportNotEqual(a, b []byte) bool { + fmt.Println("compare") + return string(a) != string(b) // want `string\(a\) != string\(b\) is a \[\]byte comparison written the long way; use !bytes\.Equal\(a, b\) for clearer intent` +} diff --git a/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/singleimport.go.golden b/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/singleimport.go.golden index 8b6af862389..33c405e55cd 100644 --- a/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/singleimport.go.golden +++ b/pkg/linters/bytescomparestring/testdata/src/bytescomparestring/singleimport.go.golden @@ -1,11 +1,16 @@ package bytescomparestring import ( - "fmt" "bytes" + "fmt" ) func singleImportEqual(a, b []byte) bool { fmt.Println("compare") return bytes.Equal(a, b) // want `string\(a\) == string\(b\) is a \[\]byte comparison written the long way; use bytes\.Equal\(a, b\) for clearer intent` } + +func singleImportNotEqual(a, b []byte) bool { + fmt.Println("compare") + return !bytes.Equal(a, b) // want `string\(a\) != string\(b\) is a \[\]byte comparison written the long way; use !bytes\.Equal\(a, b\) for clearer intent` +} From c0b4778c28b9bd26dd6aa23fa0d0a8dd98dce5fa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Jul 2026 15:11:38 +0000 Subject: [PATCH 4/4] fix: deduplicate bytes import TextEdit across multiple violations in same file Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- .../bytescomparestring/bytescomparestring.go | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/pkg/linters/bytescomparestring/bytescomparestring.go b/pkg/linters/bytescomparestring/bytescomparestring.go index 7de889f4d7f..452223afe39 100644 --- a/pkg/linters/bytescomparestring/bytescomparestring.go +++ b/pkg/linters/bytescomparestring/bytescomparestring.go @@ -36,6 +36,11 @@ func run(pass *analysis.Pass) (any, error) { } noLintLinesByFile := nolint.BuildLineIndex(pass, "bytescomparestring") + // seenImportFiles tracks files that have already received a bytes import + // TextEdit in this pass, preventing duplicate overlapping edits when a + // single file contains multiple flagged comparisons. + seenImportFiles := make(map[token.Pos]bool) + nodeFilter := []ast.Node{ (*ast.BinaryExpr)(nil), } @@ -80,14 +85,14 @@ func run(pass *analysis.Pass) (any, error) { Pos: bin.Pos(), End: bin.End(), Message: fmt.Sprintf("string(%s) == string(%s) is a []byte comparison written the long way; use bytes.Equal(%s, %s) for clearer intent", lText, rText, lText, rText), - SuggestedFixes: buildFix(pass, bin, fmt.Sprintf("bytes.Equal(%s, %s)", lText, rText)), + SuggestedFixes: buildFix(pass, bin, fmt.Sprintf("bytes.Equal(%s, %s)", lText, rText), seenImportFiles), }) } else { pass.Report(analysis.Diagnostic{ Pos: bin.Pos(), End: bin.End(), Message: fmt.Sprintf("string(%s) != string(%s) is a []byte comparison written the long way; use !bytes.Equal(%s, %s) for clearer intent", lText, rText, lText, rText), - SuggestedFixes: buildFix(pass, bin, fmt.Sprintf("!bytes.Equal(%s, %s)", lText, rText)), + SuggestedFixes: buildFix(pass, bin, fmt.Sprintf("!bytes.Equal(%s, %s)", lText, rText), seenImportFiles), }) } }) @@ -97,13 +102,15 @@ func run(pass *analysis.Pass) (any, error) { // buildFix returns the SuggestedFix for rewriting bin to replacement, adding a // "bytes" import TextEdit when the file containing bin does not yet import it. -func buildFix(pass *analysis.Pass, bin *ast.BinaryExpr, replacement string) []analysis.SuggestedFix { +// seenImportFiles tracks files that have already received an import edit in +// this pass so that multi-violation files do not get duplicate overlapping edits. +func buildFix(pass *analysis.Pass, bin *ast.BinaryExpr, replacement string, seenImportFiles map[token.Pos]bool) []analysis.SuggestedFix { edits := []analysis.TextEdit{{ Pos: bin.Pos(), End: bin.End(), NewText: []byte(replacement), }} - if importEdit, ok := addBytesImportEdit(pass, bin.Pos()); ok { + if importEdit, ok := addBytesImportEdit(pass, bin.Pos(), seenImportFiles); ok { edits = append(edits, importEdit) } return []analysis.SuggestedFix{{ @@ -113,9 +120,11 @@ func buildFix(pass *analysis.Pass, bin *ast.BinaryExpr, replacement string) []an } // addBytesImportEdit returns a TextEdit that inserts an import for "bytes" into -// the file containing pos, unless "bytes" is already imported in that file. +// the file containing pos, unless "bytes" is already imported in that file or +// an import edit for this file has already been emitted in this pass +// (tracked via seenImportFiles to prevent duplicate overlapping edits). // Returns (TextEdit{}, false) when no edit is needed. -func addBytesImportEdit(pass *analysis.Pass, pos token.Pos) (analysis.TextEdit, bool) { +func addBytesImportEdit(pass *analysis.Pass, pos token.Pos, seenImportFiles map[token.Pos]bool) (analysis.TextEdit, bool) { var file *ast.File for _, f := range pass.Files { if f.Pos() <= pos && pos <= f.End() { @@ -127,6 +136,11 @@ func addBytesImportEdit(pass *analysis.Pass, pos token.Pos) (analysis.TextEdit, return analysis.TextEdit{}, false } + // Skip if an import edit for this file was already emitted in this pass. + if seenImportFiles[file.Pos()] { + return analysis.TextEdit{}, false + } + // Check if "bytes" is already imported in this file. for _, imp := range file.Imports { if imp.Path.Value == `"`+bytesPkg+`"` { @@ -134,6 +148,9 @@ func addBytesImportEdit(pass *analysis.Pass, pos token.Pos) (analysis.TextEdit, } } + // Compute the edit to add and mark the file so subsequent violations in the + // same pass do not emit a duplicate overlapping TextEdit. + // Find an existing grouped import declaration to add into. for _, decl := range file.Decls { genDecl, ok := decl.(*ast.GenDecl) @@ -141,6 +158,7 @@ func addBytesImportEdit(pass *analysis.Pass, pos token.Pos) (analysis.TextEdit, continue } // Insert "bytes" before the closing paren of the import block. + seenImportFiles[file.Pos()] = true return analysis.TextEdit{ Pos: genDecl.Rparen, End: genDecl.Rparen, @@ -162,6 +180,7 @@ func addBytesImportEdit(pass *analysis.Pass, pos token.Pos) (analysis.TextEdit, continue } + seenImportFiles[file.Pos()] = true return analysis.TextEdit{ Pos: genDecl.Pos(), End: genDecl.End(), @@ -171,6 +190,7 @@ func addBytesImportEdit(pass *analysis.Pass, pos token.Pos) (analysis.TextEdit, } // No grouped import block; insert a standalone import after the package name. + seenImportFiles[file.Pos()] = true return analysis.TextEdit{ Pos: file.Name.End(), End: file.Name.End(),