Skip to content

Add extra symbol check for .to_owned()#156074

Open
EvoPot wants to merge 1 commit intorust-lang:mainfrom
EvoPot:cow-diag-item
Open

Add extra symbol check for .to_owned()#156074
EvoPot wants to merge 1 commit intorust-lang:mainfrom
EvoPot:cow-diag-item

Conversation

@EvoPot
Copy link
Copy Markdown

@EvoPot EvoPot commented May 2, 2026

Follow up of #154646

Previously when adding a suggestion for using Cow::into_owned() instead of ToOwned::to_owned(), the compiler would just convert the methods Span into a String and do checks on that String. This PR adds an extra guard to that suggestion by checking if the method is sym::to_owned_method.

r? @davidtwco since you added the review to the previous PR

@rustbot 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 May 2, 2026
@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 2, 2026

The Clippy subtree was changed

cc @rust-lang/clippy

@rustbot rustbot added the T-clippy Relevant to the Clippy team. label May 2, 2026
&& tcx.is_diagnostic_item(sym::to_owned_method, def_id)
&& let Ok(snippet) = tcx.sess.source_map().span_to_snippet(return_span)
{
if let Some(pos) = snippet.rfind(".to_owned") {
Copy link
Copy Markdown
Member

@chenyukang chenyukang May 2, 2026

Choose a reason for hiding this comment

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

if we already used is_diagnostic_item to check the method name, there is no need to check snippet?

View changes since the review

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The previous PR used that to crop the span of the method, so I kept it.

@EvoPot EvoPot requested a review from chenyukang May 2, 2026 06:55
Comment on lines +3546 to +3550
to_owned_ident.span.shrink_to_lo(),
"try using `.into_owned()` if you meant to convert a `Cow<'_, T>` to an owned `T`",
"in",
Applicability::MaybeIncorrect,
);
Copy link
Copy Markdown
Contributor

@ada4a ada4a May 2, 2026

Choose a reason for hiding this comment

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

the indentation here seems off?..

View changes since the review

Previously when adding a suggestion for using `Cow::into_owned()`
instead of `ToOwned::to_owned()`, the compiler would just convert the
methods `Span` into a `String` and do checks on that `String`. This PR
adds an extra guard to that suggestion by checking if the method is
`sym::to_owned_method`.
@EvoPot EvoPot requested a review from ada4a May 2, 2026 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants