Skip to content

Extend UNNECESSARY_TO_OWNED to handle split #11871

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 2 commits into from
Dec 23, 2023

Conversation

GuillaumeGomez
Copy link
Member

Fixes #9965.

When you have to_string().split('a') or equivalent, it'll suggest to remove the to_owned/to_string part.

r? @flip1995

changelog: Extend UNNECESSARY_TO_OWNED to handle split

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 25, 2023
@GuillaumeGomez
Copy link
Member Author

r? @llogiq

@rustbot rustbot assigned llogiq and unassigned flip1995 Dec 10, 2023
Copy link
Contributor

@llogiq llogiq left a comment

Choose a reason for hiding this comment

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

The code looks good, some more tests would be nice though.

fn main() {
let _ = "a".to_string().split('a').next().unwrap();
//~^ ERROR: unnecessary use of `to_string`
let _ = "a".to_owned().split('a').next().unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to see a test splitting by a &str and perhaps a counter-test that calls split by () which is implemented by a custom trait (if that is even possible?).

Copy link
Member Author

Choose a reason for hiding this comment

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

Implementing the Pattern trait is... quite a lot of code. So instead, I added checks for both char and str and also added the equivalent tests on slice.

@GuillaumeGomez GuillaumeGomez force-pushed the UNNECESSARY_TO_OWNED-split branch 3 times, most recently from efddb33 to 238c5f9 Compare December 18, 2023 15:58
@GuillaumeGomez
Copy link
Member Author

Updated!

@llogiq
Copy link
Contributor

llogiq commented Dec 23, 2023

Thank you!

@bors r+

@bors
Copy link
Contributor

bors commented Dec 23, 2023

📌 Commit 238c5f9 has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Dec 23, 2023

⌛ Testing commit 238c5f9 with merge 858d96d...

@bors
Copy link
Contributor

bors commented Dec 23, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 858d96d to master...

@bors bors merged commit 858d96d into rust-lang:master Dec 23, 2023
@GuillaumeGomez GuillaumeGomez deleted the UNNECESSARY_TO_OWNED-split branch December 23, 2023 17:57
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.

str::split instead of to_string().split()
5 participants