Skip to content

Fix named string round-trip diagnostic guidance - #52963

Merged
pelikhan merged 3 commits into
mainfrom
copilot/stringbytesroundtrip-fix-redundant-message
Aug 15, 2026
Merged

Fix named string round-trip diagnostic guidance#52963
pelikhan merged 3 commits into
mainfrom
copilot/stringbytesroundtrip-fix-redundant-message

Conversation

Copilot AI commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

stringbytesroundtrip matched named string/byte-slice types by underlying type but hardcoded diagnostics as string([]byte(...)) and implied both conversions were always removable.

  • Diagnostics

    • Render actual outer and inner conversion syntax.
    • Recommend a direct outer conversion when named string types require it.
    • Retain “both conversions can be removed” only for exact string inputs/results.
  • Documentation

    • Clarify named-type round trips may require the outer conversion.
  • Coverage

    • Add named outer conversion and string return-context cases.
type myString string

return string([]byte(ms))
// diagnostic: replace it with string(ms)

_ = myString([]byte(ms))
// diagnostic: replace it with myString(ms)

Copilot AI and others added 2 commits August 15, 2026 20:22
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix redundant round-trip message for named types Fix named string round-trip diagnostic guidance Aug 15, 2026
Copilot AI requested a review from pelikhan August 15, 2026 20:25
@github-actions

Copy link
Copy Markdown
Contributor

Great work on the stringbytesroundtrip linter fix! 🎯 This PR improves diagnostic accuracy for named string types, which is exactly what the linter ecosystem needs.

The changes here:

  • Enhanced diagnostics — now render actual conversion syntax and distinguish between exact string types and named string types
  • Accurate guidance — recommends the outer conversion when named types require it
  • Solid test coverage — new test cases cover both the myString([]byte(ms)) pattern and the string([]byte(ms)) return-context case

Everything aligns with the project's linter quality standards: focused, well-documented, properly tested. This looks ready for review and merge. ✅

Generated by ✅ Contribution Check · auto · 57.4 AIC · ⌖ 3.95 AIC · ⊞ 9.1K ·

@pelikhan
pelikhan marked this pull request as ready for review August 15, 2026 21:22
Copilot AI balanced review requested due to automatic review settings August 15, 2026 21:22
@pelikhan
pelikhan merged commit 92c60d5 into main Aug 15, 2026
1 check passed
@pelikhan
pelikhan deleted the copilot/stringbytesroundtrip-fix-redundant-message branch August 15, 2026 21:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves stringbytesroundtrip diagnostics for named string types and clarifies conversion-removal guidance.

Changes:

  • Renders actual conversion syntax in diagnostics.
  • Preserves necessary outer conversions for named types.
  • Adds named-type and return-context coverage.
Show a summary per file
File Description
pkg/linters/stringbytesroundtrip/stringbytesroundtrip.go Updates documentation and diagnostic generation.
pkg/linters/stringbytesroundtrip/testdata/src/stringbytesroundtrip/stringbytesroundtrip.go Adds named conversion test cases.

Review details

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment on lines +147 to +149
func isExactString(t types.Type) bool {
return isStringType(t)
}
Comment on lines +117 to +121
if isExactString(pass.TypesInfo.TypeOf(outer)) && isExactString(pass.TypesInfo.TypeOf(inner.Args[0])) {
pass.ReportRangef(outer,
"%s(%s(%s)) is a redundant round-trip; both conversions can be removed; the inner %s conversion copies the string unnecessarily",
outerText, innerText, argText, innerText,
)
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.87.0

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

Labels

None yet

Projects

None yet

3 participants