Skip to content

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

Description

@github-actions

Summary

The new 57th linter stringbytesroundtrip (pkg/linters/stringbytesroundtrip/stringbytesroundtrip.go) flags both directions of a string/[]byte round-trip with the same "redundant round-trip ... produces a wasteful intermediate copy" framing. But the two directions are not semantically symmetric:

Expression Result vs. input Safe to drop?
string([]byte(s)), s string value-identical to s (strings immutable) ✅ yes — genuinely redundant
[]byte(string(b)), b []byte a fresh, non-aliasing copy of b ❌ no — this is the defensive-copy idiom

[]byte(string(b)) is the long-standing Go idiom for producing an independent byte slice that does not share b's backing array (the pre-slices.Clone/bytes.Clone clone). Per the Go spec both conversions copy, so the result provably never aliases b. Calling this a "redundant round-trip" (the doc even says it "leave[s] the caller with the same underlying type as the input") is misleading: the whole point is that it is a distinct copy. A developer who trusts the "redundant" label and rewrites []byte(string(b))b introduces an aliasing bug — subsequent mutations of the result would now alias b.

This is the same behavior-not-preserving class as #44865 (tolowerequalfold) and #46130 (mapdeletecheck): the diagnostic's stated premise is wrong for a legitimate idiom.

Evidence

  • stringbytesroundtrip.go:118-125 — the []byte(string(b)) arm reports "[]byte(string(%s)) is a redundant round-trip; the inner string conversion copies the bytes unnecessarily".
  • Package doc stringbytesroundtrip.go:1-5 and Analyzer.Doc:23: "leave the caller with the same underlying type as the input" — untrue for the []byte(string(b)) direction (same type, but a different, non-aliasing value).
  • testdata/src/stringbytesroundtrip/stringbytesroundtrip.go:27,36 encode this as intended behavior (_ = []byte(string(b)) // want ...redundant round-trip, _ = []byte(string(mb)) // want ...), so the FP framing is locked in by the golden test rather than an oversight.
  • The string([]byte(s)) arm (:108-116) is correct: string([]byte(s)) == s always, so simplifying to s (or string(s) for a named string) is safe.

Impact

  • False-positive class + enforce-blocker. Production currently has zero real []byte(string(x)) sites (all 5 grep hits are this linter's own doc/comments), so there is no live failure today — but the linter is not yet CI-enforced (cgo.yml:1217 enforces 26 analyzers; this one is absent). If it is added to CI as-is (it is otherwise clean and type-resolved, a natural enforce candidate), the first legitimate defensive copy — []byte(string(b)), or a future slices.Clone refactor target — will be reported as "redundant," pushing the author toward an aliasing bug or a nolint.
  • Misleading remediation. Even where a caller does want a copy, the advice implied by "redundant" (remove it) is wrong; the right advice is "replace the two-copy round-trip with a single-copy clone" (slices.Clone(b) / bytes.Clone(b)).

Recommendation

Split the two arms so the []byte(string(b)) diagnostic reflects its real semantics. Options, cheapest first:

  1. Reword the []byte(string(b)) arm to describe it as a wasteful two-copy clone, not a redundant round-trip, and recommend slices.Clone(b)/bytes.Clone(b) (one copy) rather than removal. Keep the string([]byte(s)) arm's "redundant" wording (it is correct).
  2. Or drop the []byte(string(b)) arm entirely and keep only the provably-redundant string([]byte(s)) direction, which is the safe, enforce-ready half.

Before → after intent (message only, option 1):

  • before: []byte(string(b)) is a redundant round-trip; the inner string conversion copies the bytes unnecessarily
  • after: []byte(string(b)) makes two copies to clone b; use slices.Clone(b) (or bytes.Clone(b)) for a single-copy independent slice

Validation checklist

  • []byte(string(b)) diagnostic no longer implies removal / value-equality with b.
  • string([]byte(s)) arm unchanged (still flagged; still want-matched).
  • Update testdata/.../stringbytesroundtrip.go:27,36 want regexps + package Doc string (:23) and header comment (:1-5) to drop "same underlying ... as the input" for the byte-slice direction.
  • If the linter is subsequently added to cgo.yml enforcement, confirm the reworded []byte(string(b)) rule is acceptable to enforce (or enforce only the string([]byte(s)) half).

Effort

Small — message + doc + golden-want edits in one package; no traversal or type-logic change. The type analysis (Underlying()-based, IsType()-gated) is otherwise correct and alias/named-type safe.

Generated by 🤖 Sergo - Serena Go Expert · age00 · 227.5 AIC · ⌖ 13.7 AIC · ⊞ 5.8K ·

  • expires on Jul 30, 2026, 8:58 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

cookieIssue Monster Loves Cookies!sergo

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions