stringbytesroundtrip: redundant-round-trip message hardcodes "string(...)"/"[]byte(...)" and ignores named-type assignability, g [Content truncated due to length]

[github-actions]

Summary

pkg/linters/stringbytesroundtrip/stringbytesroundtrip.go flags string([]byte(s)) as "redundant" and its package doc claims "the result is value-identical to s and both conversions can be removed" (lines 1-4). This claim, and the diagnostic message text itself, are only accurate when both conversions target the exact predeclared string/[]byte types. The type check (roundTripUnderlyingTypes, lines 90-106) only compares .Underlying(), so it also matches named types — but the reported message hardcodes the literal text "string([]byte(%s))" regardless of what the actual outer/inner conversion targets are.

Evidence

• reportRedundantRoundTrip (stringbytesroundtrip.go:108-118) builds its message with the literal words string(...) / []byte(...):

  pass.ReportRangef(outer,
      "string([]byte(%s)) is a redundant round-trip; the inner []byte conversion copies the string unnecessarily",
      argText,
  )

  but outer/inner are only checked via .Underlying() (line 105: return outerType.Underlying(), innerType.Underlying(), innerArgType.Underlying(), true), so a call like myString([]byte(ms)) (where myString is type myString string) is flagged with the message "string([]byte(ms)) is a redundant round-trip" — text that doesn't match the actual source (myString([]byte(ms))), and which advises removing conversions that Go's assignability rules do not allow to be dropped bare: ms has type myString, not string, so replacing myString([]byte(ms)) with plain ms fails to compile in any context expecting a string (or vice versa), exactly the class of bug already fixed once in this repo for writebytestring (isExactString check, buildStringExpr wraps named types in string(...)) and already the subject of closed issue stringbytesroundtrip: the []byte(string(b)) arm mislabels the defensive-copy idiom as a "redundant round-trip" — a false-positiv [Content truncated due to length] #47704's remediation note for this exact arm ("or string(s) for a named string") — advice the current diagnostic text never gives.
• testdata/src/stringbytesroundtrip/stringbytesroundtrip.go:30-36 (badNamedTypes) only exercises _ = string([]byte(ms)), assigning to the blank identifier — it never uses the "redundant" result in a context (e.g. a string-typed return or assignment to a string-typed variable) where the literal removal the doc/message implies would actually fail to compile, and it never exercises a named outer conversion (e.g. myString([]byte(ms))), so the message-text mismatch never gets golden-file coverage.
• Compare to the sibling writebytestring linter (already fixed for the identical root cause via isExactString/buildStringExpr); stringbytesroundtrip's own isStringType/isByteSliceType helpers (lines 131-143) are copies of that same Underlying()-based pattern applied here without the exact-type distinction. This arm has no SuggestedFix to catch the bug at compile time, so it only surfaces as an inaccurate diagnostic message rather than a broken autofix.

Impact

Not CI-enforced (absent from .github/workflows/cgo.yml LINTER_FLAGS), so no build breakage today. But the diagnostic actively misleads: (1) the message text can literally misquote the source when named types are involved, and (2) a developer trusting the package doc's "both conversions can be removed" framing and manually deleting both conversions on a named-type round-trip introduces a compile error — exactly the outcome closed issue #47704 flagged for the sibling []byte(string(b)) arm (that issue explicitly noted the string([]byte(s)) arm needs string(s) wrapping for named types; this issue is that the code and message still don't reflect that).

Suggested fix

In reportRedundantRoundTrip, use the actual outer/inner conversion type names (via astutil.NodeText on outer.Fun/inner.Fun, mirroring how argText is derived) instead of hardcoding "string(...)"/"[]byte(...)", and qualify the removal advice when the outer/inner/arg types are named rather than exact string/[]byte (same distinction as writebytestring.isExactString). Add a badNamedTypes testdata case with a named outer conversion (myString([]byte(ms))) to lock in the correct message text.

---

Sergo automated finding. temporary_id: aw_sg61a1

Generated by 🤖 Sergo - Serena Go Expert · agent · 255 AIC · ⌖ 2.25 AIC · ⊞ 5.9K · ◷

• expires on Aug 21, 2026, 8:13 PM UTC-08:00