Skip to content
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

manual_split_once: lint manual iteration of SplitN #8717

Merged
merged 1 commit into from
Apr 22, 2022

Conversation

Alexendoo
Copy link
Member

changelog: manual_split_once: lint manual iteration of SplitN

Now lints:

let mut iter = "a.b.c".splitn(2, '.');
let first = iter.next().unwrap();
let second = iter.next().unwrap();

let mut iter = "a.b.c".splitn(2, '.');
let first = iter.next()?;
let second = iter.next()?;

let mut iter = "a.b.c".rsplitn(2, '.');
let first = iter.next().unwrap();
let second = iter.next().unwrap();

let mut iter = "a.b.c".rsplitn(2, '.');
let first = iter.next()?;
let second = iter.next()?;

It suggests (minus leftover whitespace):

let (first, second) = "a.b.c".split_once('.').unwrap();

let (first, second) = "a.b.c".split_once('.')?;

let (second, first) = "a.b.c".rsplit_once('.').unwrap();

let (second, first) = "a.b.c".rsplit_once('.')?;

Currently only lints if the statements are next to each other, as detecting the various kinds of shadowing was tricky, so the following won't lint

let mut iter = "a.b.c".splitn(2, '.');
let something_else = 1;
let first = iter.next()?;
let second = iter.next()?;

@rust-highfive
Copy link

r? @xFrednet

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 18, 2022
Copy link
Member

@dswij dswij left a comment

Choose a reason for hiding this comment

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

LGTM! Just a question and a suggestion to add the limitations to docs, otherwise the changes look great.

tests/ui/manual_split_once.rs Show resolved Hide resolved
clippy_lints/src/methods/str_splitn.rs Show resolved Hide resolved
Copy link
Member

@xFrednet xFrednet left a comment

Choose a reason for hiding this comment

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

This version already looks good to me. I like the suggestion to add a limitations section to the docs, and then we can merge this. 👍

Thank you very much @dswij for the review!

tests/ui/manual_split_once.rs Show resolved Hide resolved
@Alexendoo Alexendoo force-pushed the manual-split-once-manual-iter branch from 5825cc0 to c206098 Compare April 21, 2022 17:33
@Alexendoo Alexendoo force-pushed the manual-split-once-manual-iter branch from c206098 to 4424aa4 Compare April 21, 2022 17:33
@xFrednet
Copy link
Member

Looks good to me, thank you for the update! 👍

@bors r=dswij,xFrednet

@bors
Copy link
Contributor

bors commented Apr 22, 2022

📌 Commit 4424aa4 has been approved by dswij,xFrednet

@bors
Copy link
Contributor

bors commented Apr 22, 2022

⌛ Testing commit 4424aa4 with merge ed22428...

@bors
Copy link
Contributor

bors commented Apr 22, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: dswij,xFrednet
Pushing ed22428 to master...

@bors bors merged commit ed22428 into rust-lang:master Apr 22, 2022
@Alexendoo Alexendoo deleted the manual-split-once-manual-iter branch April 22, 2022 10:16
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.

5 participants