Summary
bytescomparestring (pkg/linters/bytescomparestring/bytescomparestring.go) flags string(a) == string(b) / != comparisons where a/b are []byte, and suggests bytes.Equal(a, b). The type-match logic in extractByteSliceStringConv (lines 219-250) accepts any conversion whose result type's Underlying() is *types.Basic with Kind() == types.String — not just the exact predeclared string type. That includes named string types, e.g. type Password string.
The diagnostic message, however, hardcodes the literal text "string(...)" regardless of what conversion the source actually used:
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),So code like:
type Password string
if Password(a) == Password(b) { ... }produces the misleading diagnostic: string(a) == string(b) is a []byte comparison written the long way; use bytes.Equal(a, b) for clearer intent — even though the source never wrote string(...) at all.
This is the same bug class already confirmed and fixed for a sibling linter in #52826 (stringbytesroundtrip: "redundant-round-trip message hardcodes 'string(...)'/'[]byte(...)' and ignores named-type assignability"), but bytescomparestring was not covered by that fix and has no test for this case.
Note: the SuggestedFix itself (bytes.Equal(a, b)) remains semantically correct and compiles fine here — only the human-facing diagnostic message text is wrong. This is a message-accuracy bug, not a compile-correctness bug (contrast with #aw_sg60a1/writebytestring, which was a compile bug).
Evidence
pkg/linters/bytescomparestring/bytescomparestring.go:238-241 — matches on Underlying(), not exact identity:
basic, ok := resultInfo.Type.Underlying().(*types.Basic)
if !ok || basic.Kind() != types.String {
return nil, false
}pkg/linters/bytescomparestring/bytescomparestring.go:93,104 — message hardcodes "string(%s)" literally rather than using the actual conversion's type name.- Confirmed via
pkg/linters/bytescomparestring/testdata/src/bytescomparestring/: existing golden fixtures (bytescomparestring.go, aliased_import.go, noimport.go, shadow.go, singleimport.go) contain no named-string-type conversion case, so this gap is untested.
Impact
Low-medium. Purely a message-quality bug (misleading lint text pointing at code that doesn't exist verbatim in the file), which can confuse a developer trying to locate the flagged expression, especially in an editor/CI annotation that quotes the message. No compile or autofix-correctness risk.
Recommendation
Reuse the actual conversion type name in the message instead of the hardcoded literal, e.g. render the type's String() (or fall back to "string" only when the type is the exact predeclared string) so the message quotes what the source really wrote. Add a testdata case with a named string type on one or both sides to lock in the fix, mirroring the approach used to close #52826.
Validation checklist
Effort
Small — message string construction change + one testdata fixture pair.
Generated by 🤖 Sergo - Serena Go Expert · agent · 165 AIC · ⌖ 4.99 AIC · ⊞ 6.3K · ◷
Summary
bytescomparestring(pkg/linters/bytescomparestring/bytescomparestring.go) flagsstring(a) == string(b)/!=comparisons wherea/bare[]byte, and suggestsbytes.Equal(a, b). The type-match logic inextractByteSliceStringConv(lines 219-250) accepts any conversion whose result type'sUnderlying()is*types.BasicwithKind() == types.String— not just the exact predeclaredstringtype. That includes named string types, e.g.type Password string.The diagnostic message, however, hardcodes the literal text
"string(...)"regardless of what conversion the source actually used:So code like:
produces the misleading diagnostic:
string(a) == string(b) is a []byte comparison written the long way; use bytes.Equal(a, b) for clearer intent— even though the source never wrotestring(...)at all.This is the same bug class already confirmed and fixed for a sibling linter in #52826 (
stringbytesroundtrip: "redundant-round-trip message hardcodes 'string(...)'/'[]byte(...)' and ignores named-type assignability"), butbytescomparestringwas not covered by that fix and has no test for this case.Note: the
SuggestedFixitself (bytes.Equal(a, b)) remains semantically correct and compiles fine here — only the human-facing diagnostic message text is wrong. This is a message-accuracy bug, not a compile-correctness bug (contrast with #aw_sg60a1/writebytestring, which was a compile bug).Evidence
pkg/linters/bytescomparestring/bytescomparestring.go:238-241— matches onUnderlying(), not exact identity:pkg/linters/bytescomparestring/bytescomparestring.go:93,104— message hardcodes"string(%s)"literally rather than using the actual conversion's type name.pkg/linters/bytescomparestring/testdata/src/bytescomparestring/: existing golden fixtures (bytescomparestring.go,aliased_import.go,noimport.go,shadow.go,singleimport.go) contain no named-string-type conversion case, so this gap is untested.Impact
Low-medium. Purely a message-quality bug (misleading lint text pointing at code that doesn't exist verbatim in the file), which can confuse a developer trying to locate the flagged expression, especially in an editor/CI annotation that quotes the message. No compile or autofix-correctness risk.
Recommendation
Reuse the actual conversion type name in the message instead of the hardcoded literal, e.g. render the type's
String()(or fall back to"string"only when the type is the exact predeclared string) so the message quotes what the source really wrote. Add a testdata case with a named string type on one or both sides to lock in the fix, mirroring the approach used to close #52826.Validation checklist
"string")type X string; X(a) == X(b)wherea,bare[]byteSuggestedFixoutput is unaffected (still emitsbytes.Equal(a, b))go test ./pkg/linters/bytescomparestring/...passesEffort
Small — message string construction change + one testdata fixture pair.