Skip to content

[unnecessary_to_owned]: check that the adjusted type matches target #10913

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

y21
Copy link
Member

@y21 y21 commented Jun 9, 2023

Fixes #10033.

Before this change, the lint would assume that removing the .to_string() in f(&x.to_string()) would be ok if x is of some type that implements Deref<Target = str> and f takes a &str.
This turns out to not actually be ok if the to_string call is some method that exists on x directly, which happens if it implements Display/ToString itself.

changelog: [unnecessary_to_owned]: only lint if the adjusted receiver type actually matches

@rustbot
Copy link
Collaborator

rustbot commented Jun 9, 2023

r? @Manishearth

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 9, 2023
@Manishearth
Copy link
Member

r? @Centri3 before I review this as well

@rustbot

This comment was marked as outdated.

Copy link
Member

@Centri3 Centri3 left a comment

Choose a reason for hiding this comment

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

This looks fine, thanks!

@Manishearth
Copy link
Member

@bors r=Manishearth,Centri3

thanks!

@bors
Copy link
Contributor

bors commented Jun 9, 2023

📌 Commit dd08494 has been approved by Manishearth,Centri3

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 9, 2023

⌛ Testing commit dd08494 with merge 05de787...

@bors
Copy link
Contributor

bors commented Jun 9, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Manishearth,Centri3
Pushing 05de787 to master...

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

erroneous "unnecessary use of to_string"
5 participants