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

DON'T MERGE[ci skip]: demonstrate if_let_rescope auto migration #4192

Closed
wants to merge 2 commits into from

Conversation

rami3l
Copy link
Member

@rami3l rami3l commented Feb 22, 2025

Complements #4191 to show the if-let migrations that I didn't do.

For the sake of simplicity, the first commit of #4191 has also been cherry-picked into this branch. After that, a cargo clippy --fix is performed after adjusting the Cargo.toml to activate the if_let_rescope lint.

Notably, according to rust-lang/rust#133167 (comment), if let Some() and other similar cases where the else drop is not significant are not affected by the Edition change.

Please feel free to point out if I have missed a significant drop somewhere in #4191.

You can get a clearer version of the PR diff via semanticdiff.

Following
<https://users.rust-lang.org/t/rust-2024-compatibility-lint-and-the-if-let-temporary-scope/125969/5>,
I have manually audited all `if_let_rescope` cases and found out that
certain existing cases can be rewritten to make it clearer semantically.
So these migrations have been made in advance so that the only cases of
`if-let` remaining are false positives of this lint.

The crux of determining false positives here is that, according to
<rust-lang/rust#133167>, `if let Some()`
and other similar cases where the `else` drop is not significant are
not affected by the Edition change.
@rami3l rami3l force-pushed the x/2024-if-let-demo branch from d84e07a to 41f0298 Compare February 22, 2025 07:52
@rami3l rami3l closed this Feb 23, 2025
@rami3l rami3l deleted the x/2024-if-let-demo branch February 23, 2025 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant