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

Add new lint obfuscated_if_else #9148

Merged
merged 1 commit into from
Jul 18, 2022
Merged

Add new lint obfuscated_if_else #9148

merged 1 commit into from
Jul 18, 2022

Conversation

arieluy
Copy link
Contributor

@arieluy arieluy commented Jul 10, 2022 •
edited
Loading

part of #9100, additional commits could make it work with then and unwrap_or_else as well

changelog: Add new lint obfuscated_if_else

@rust-highfive
Copy link

r? @flip1995

(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 Jul 10, 2022
@Jarcho
Copy link
Contributor

Jarcho commented Jul 11, 2022

Just going to nitpick the name here. Basically the same lint applies to any combinations of [then, then_some] and [unwrap_or, unwrap_or_default, unwrap_or_else, map_or, map_or_else, is_some_and].

@arieluy
Copy link
Contributor Author

arieluy commented Jul 13, 2022

I was trying to follow the naming guidelines of stating the thing that is wrong. What do you think about obfuscated_if_else, or do you have any other suggestions?

@Jarcho
Copy link
Contributor

Jarcho commented Jul 17, 2022

I think obfuscated_if_else should be fine.

@arieluy
Add new lint obfuscated_if_else
9ff7c91 
New lint suggests using `if .. else ..` instead of
`.then_some(..).unwrap_or(..)`.
@arieluy arieluy force-pushed the then_some_unwrap_or branch from ee5e1da to 9ff7c91 Compare July 18, 2022 01:49
@arieluy arieluy changed the title Add new lint then_some_unwrap_or Add new lint obfuscated_if_else Jul 18, 2022
@Jarcho
Copy link
Contributor

Jarcho commented Jul 18, 2022

Looks good. Thank you.

@bors r+

@bors
Copy link
Contributor

bors commented Jul 18, 2022

📌 Commit 9ff7c91 has been approved by Jarcho

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jul 18, 2022

⌛ Testing commit 9ff7c91 with merge cce6171...

@bors
Copy link
Contributor

bors commented Jul 18, 2022

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

@bors bors merged commit cce6171 into rust-lang:master Jul 18, 2022
This was referenced Jul 18, 2022
Merged
Merged
@arieluy
Copy link
Contributor Author

arieluy commented Jul 28, 2022

Thanks @Jarcho !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@flip1995 flip1995

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.
5 participants
@arieluy @rust-highfive @Jarcho @bors @flip1995