Skip to content

new lint: manual_while_let_some#10647

Merged
bors merged 7 commits intorust-lang:masterfrom
y21:while_pop_unwrap
Apr 29, 2023
Merged

new lint: manual_while_let_some#10647
bors merged 7 commits intorust-lang:masterfrom
y21:while_pop_unwrap

Conversation

@y21
Copy link
Member

@y21 y21 commented Apr 15, 2023 •
edited
Loading

This PR implements the lint I suggested on zulip.
It looks for while loops like these:

let mut numbers = vec![0, 1, 2];
while !numbers.is_empty() {
  let number = numbers.pop().unwrap();
  // use `number`
}

and suggests replacing it with a while-let loop, like this:

let mut numbers = vec![0, 1, 2];
while let Some(number) = numbers.pop() {
  // use `number`
}

... which is more concise and idiomatic.

It only looks for Vec::pop() calls in the first statement of the loop body in an attempt to not trigger FPs (as pop might only be called conditionally).

changelog: new lint [manual_while_let_some]

@rustbot
Copy link
Collaborator

rustbot commented Apr 15, 2023

r? @llogiq

(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 Apr 15, 2023
llogiq
llogiq requested changes Apr 17, 2023
View reviewed changes
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.

This is a good starting point! 👍

For a style lint, I'd like to have a real working suggestion. Otherwise I'd be fine with putting the lint in nursery for the time being, and work on it in follow-up PRs. Otherwise a few more tests would be great.

@llogiq
Copy link
Contributor

llogiq commented Apr 19, 2023

I like it! 👍 The only thing I think we might improve is the name. while_pop_unwrap looks a bit out of place as a lint name. How about manual_while_let_some (we have a good number of manual_* lints already)?

@y21
Copy link
Member Author

y21 commented Apr 19, 2023

I agree, manual_while_let_some sounds better. Changing.

@y21 y21 changed the title new lint: while_pop_unwrap new lint: manual_while_let_some Apr 19, 2023
@y21 y21 force-pushed the while_pop_unwrap branch from 82a8626 to 8d8178f Compare April 29, 2023 17:00
@y21 y21 requested a review from llogiq April 29, 2023 17:19
@llogiq
Copy link
Contributor

llogiq commented Apr 29, 2023

Thank you!

@bors r+

@bors
Copy link
Contributor

bors commented Apr 29, 2023

📌 Commit 9613ea8 has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Apr 29, 2023

⌛ Testing commit 9613ea8 with merge 7bc3da9...

@bors
Copy link
Contributor

bors commented Apr 29, 2023

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

@bors bors merged commit 7bc3da9 into rust-lang:master Apr 29, 2023
This was referenced Apr 29, 2023
Merged
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@llogiq llogiq Awaiting requested review from llogiq

Assignees

@llogiq llogiq

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@y21 @rustbot @llogiq @bors