Add suggestion to `.to_owned()` used on `Cow` when borrowing#144925

Periodic1911 · 2025-08-04T20:04:08Z

Adds a suggestion when using .to_borrow() on Cow where .into_borrow() should be used, and makes a test that explicitly checks if it is shown.

rustbot · 2025-08-04T20:04:13Z

r? @davidtwco

rustbot has assigned @davidtwco.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2025-08-04T20:04:16Z

This PR modifies tests/ui/issues/. If this PR is adding new tests to tests/ui/issues/,
please refrain from doing so, and instead add it to more descriptive subdirectories.

Periodic1911 · 2025-08-04T20:07:31Z

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

+                && adtdef.did() == cow
+            {
+                if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(return_span) {
+                    if let Some(pos) = snippet.rfind(".to_owned") {


I'm not happy about detecting the use of .to_owned by way of string comparison, but since this check is in the MIR I don't know how to do it better. Constructive criticism would be greatly appreciated!

There are other diagnostics code here that does similar things, so happy to call it pre-existing until there's a better solution for this part of the compiler

Kivooeo

Some test adjustments

We have a bit of a gap in our test coverage, so I'm trying to keep a close eye on new tests

Kivooeo · 2025-08-04T22:22:43Z

tests/ui/errors/cow-to-owned.rs

I'd personally move this test to suggestions/ or borrowck/ and about name... I think name is fine but we can do better right? So i'd suggest something like cow-into-owned-suggestion.rs

Kivooeo · 2025-08-04T22:25:55Z

tests/ui/errors/cow-to-owned.rs

@@ -0,0 +1,7 @@
+// issue #144792


It's better to use full links and special comment //! Regression test for: https://github.com/rust-lang/rust/issues/144792 just because it will be easy to follow with just copy past link

Kivooeo · 2025-08-04T22:29:19Z

tests/ui/errors/cow-to-owned.rs

+// issue #144792
+
+fn main() {
+    _ = std::env::var_os("RUST_LOG").map_or("warn".into(), |x| x.to_string_lossy().to_owned());


It might be fine but I don't like it's env dependable, can we have more interesting test like below and with more coverage

// More robust - doesn't depend on environment fn test_cow_suggestion() -> String { let os_string = std::ffi::OsString::from("test"); os_string.to_string_lossy().to_owned() } // Test multiple Cow scenarios fn test_cow_from_str() -> String { let s = "hello"; let cow = std::borrow::Cow::from(s); cow.to_owned() // Should suggest into_owned() } // Test with different Cow types fn test_cow_bytes() -> Vec<u8> { let bytes = b"hello"; let cow = std::borrow::Cow::from(&bytes[..]); cow.to_owned() // Should suggest into_owned() }

I'd adopt this as the test

davidtwco · 2025-08-13T15:53:17Z

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

            }
+
+            if let Some(cow) = tcx.get_diagnostic_item(sym::Cow)
+                && let ty::Adt(adtdef, _) = return_ty.kind()


Suggested change

&& let ty::Adt(adtdef, _) = return_ty.kind()

&& let ty::Adt(adt_def, _) = return_ty.kind()

nit: more common name for variables of AdtDef

davidtwco · 2025-08-13T15:53:33Z

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

                );
            }
+
+            if let Some(cow) = tcx.get_diagnostic_item(sym::Cow)


Suggested change

if let Some(cow) = tcx.get_diagnostic_item(sym::Cow)

if let Some(cow_did) = tcx.get_diagnostic_item(sym::Cow)

nit: likewise, more typical naming convention

davidtwco · 2025-08-13T16:00:34Z

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

+                && adtdef.did() == cow
+            {
+                if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(return_span) {
+                    if let Some(pos) = snippet.rfind(".to_owned") {


There are other diagnostics code here that does similar things, so happy to call it pre-existing until there's a better solution for this part of the compiler

davidtwco · 2025-08-13T16:01:16Z

tests/ui/errors/cow-to-owned.rs

+// issue #144792
+
+fn main() {
+    _ = std::env::var_os("RUST_LOG").map_or("warn".into(), |x| x.to_string_lossy().to_owned());


I'd adopt this as the test

Dylan-DPC · 2025-10-10T15:08:26Z

@Periodic1911 any updates on this? thanks

reddevilmidzy · 2026-03-28T02:18:21Z

@Periodic1911

Triage: I'm closing this due to inactivity. Please reopen when you are ready to continue with this. Thanks for your contribution.
@rustbot label: +S-inactive

rustbot assigned davidtwco Aug 4, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 4, 2025

Periodic1911 commented Aug 4, 2025

View reviewed changes

Add suggestion to .to_owned() used on Cow when borrowing

3403c4f

Periodic1911 force-pushed the 144792-cow-diag branch from a89614d to 3403c4f Compare August 4, 2025 20:14

Kivooeo reviewed Aug 4, 2025

View reviewed changes

davidtwco requested changes Aug 13, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 13, 2025

Noratrieb mentioned this pull request Sep 1, 2025

Improve diagnostic for Cow::to_owned() usage. #146058

Closed

reddevilmidzy closed this Mar 28, 2026

rustbot added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 28, 2026

m4rch3n1ng mentioned this pull request Mar 31, 2026

Add suggestion to .to_owned() used on Cow when borrowing #154646

Open

	&& let ty::Adt(adtdef, _) = return_ty.kind()
	&& let ty::Adt(adt_def, _) = return_ty.kind()

	if let Some(cow) = tcx.get_diagnostic_item(sym::Cow)
	if let Some(cow_did) = tcx.get_diagnostic_item(sym::Cow)

Uh oh!

Conversation

Periodic1911 commented Aug 4, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Aug 4, 2025

Uh oh!

rustbot commented Aug 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kivooeo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dylan-DPC commented Oct 10, 2025

Uh oh!

reddevilmidzy commented Mar 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Periodic1911 commented Aug 4, 2025 •

edited by rustbot

Loading