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.

Sign up for GitHub

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

Jump to bottom

feat: add manual_is_variant_and lint #11865

Merged
merged 1 commit into from Dec 30, 2023
Merged

feat: add manual_is_variant_and lint #11865

merged 1 commit into from Dec 30, 2023

Conversation

yuxqiu
Copy link
Contributor

@yuxqiu yuxqiu commented Nov 25, 2023 •
edited by Jarcho

changelog: add a new lint [manual_is_variant_and].

  • Replace option.map(f).unwrap_or_default() and result.map(f).unwrap_or_default() with option.is_some_and(f) and result.is_ok_and(f) where f is a function or closure that returns bool.
  • MSRV is set to 1.70.0 for this lint; when is_some_and and is_ok_and was stabilised

For example, for the following code:

let opt = Some(0);
opt.map(|x| x > 1).unwrap_or_default();

It suggests to instead write:

let opt = Some(0);
opt.is_some_and(|x| x > 1)

@rustbot
Copy link
Collaborator

rustbot commented Nov 25, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Jarcho (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

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

bors commented Nov 25, 2023

☔ The latest upstream changes (presumably #11866) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Collaborator

bors commented Dec 16, 2023

☔ The latest upstream changes (presumably #11977) made this pull request unmergeable. Please resolve the merge conflicts.

Jarcho
Jarcho reviewed Dec 17, 2023
View reviewed changes
Copy link
Contributor

@Jarcho Jarcho left a comment

Choose a reason for hiding this comment

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

Sorry for the long delay. The implementation is fine, just have some small nits about it.

Can you change the name to something like manual_is_variant_and. Otherwise we'll end up with a pile of lints that do basically the same thing.

clippy_config/src/msrvs.rs Show resolved Hide resolved
clippy_lints/src/methods/map_unwrap_or_default.rs Outdated Show resolved Hide resolved
clippy_lints/src/methods/map_unwrap_or_default.rs Outdated Show resolved Hide resolved
clippy_lints/src/methods/map_unwrap_or_default.rs Outdated Show resolved Hide resolved
clippy_lints/src/methods/map_unwrap_or_default.rs Outdated Show resolved Hide resolved
@yuxqiu
Copy link
Contributor Author

yuxqiu commented Dec 19, 2023

Sorry for the long delay. The implementation is fine, just have some small nits about it.

Can you change the name to something like manual_is_variant_and. Otherwise we'll end up with a pile of lints that do basically the same thing.

No worries! Thank you for your review Jarcho! I have tried to resolved the problems you raised. Can you please see if the new code looks good?

@yuxqiu yuxqiu changed the title feat: add map_unwrap_or_default lint feat: add manual_is_variant_and lint Dec 19, 2023
@yuxqiu yuxqiu requested a review from Jarcho December 21, 2023 16:34
@yuxqiu yuxqiu requested a review from Jarcho December 25, 2023 01:08
Jarcho
Jarcho approved these changes Dec 25, 2023
View reviewed changes
Copy link
Contributor

@Jarcho Jarcho left a comment

Choose a reason for hiding this comment

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

Things look good. Just need to squash down the commits and then it's good.

@yuxqiu
Copy link
Contributor Author

yuxqiu commented Dec 26, 2023

Things look good. Just need to squash down the commits and then it's good.

All done! Have a good holiday!

@bors
Copy link
Collaborator

bors commented Dec 26, 2023

☔ The latest upstream changes (presumably #12004) made this pull request unmergeable. Please resolve the merge conflicts.

@Jarcho
Copy link
Contributor

Jarcho commented Dec 30, 2023

Thank you. @bors r+

@bors
Copy link
Collaborator

bors commented Dec 30, 2023

📌 Commit c4a80f2 has been approved by Jarcho

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Dec 30, 2023

⌛ Testing commit c4a80f2 with merge c6aeb28...

@bors
Copy link
Collaborator

bors commented Dec 30, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Jarcho
Pushing c6aeb28 to master...

@bors bors merged commit c6aeb28 into rust-lang:master Dec 30, 2023
8 checks passed
@yuxqiu yuxqiu deleted the map_unwrap_or_default branch December 30, 2023 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jarcho Jarcho Jarcho approved these changes

Assignees

@Jarcho Jarcho

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.

None yet
4 participants
@yuxqiu @rustbot @bors @Jarcho