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

bool_to_int_with_if inverse case patch #9476

Merged
merged 4 commits into from
Sep 14, 2022
Merged

bool_to_int_with_if inverse case patch #9476

merged 4 commits into from
Sep 14, 2022

Conversation

Xaeroxe
Copy link
Contributor

@Xaeroxe Xaeroxe commented Sep 14, 2022

Enhances bool_to_int_with_if such that it can also catch an inverse bool int conversion scenario, and makes the right suggestion for converting to int with a prefixed negation operator.

changelog: [bool_to_int_with_if]: Now correctly detects the inverse case, if bool { 0 } else { 1 }

@rust-highfive
Copy link

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

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 14, 2022
Xaeroxe added a commit to Xaeroxe/futures-rs that referenced this pull request Sep 14, 2022
Comment on lines 74 to 75
let invert = if inverted { "!" } else { "" };
let need_parens = should_have_parentheses(check);
Copy link
Contributor

Choose a reason for hiding this comment

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

Clippy has an helper struct that will already handle parenthesis for you, including when negating the expression: clippy_utils::sugg::Sugg

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That fixed a bug in the lint, and in turn fixed a bug in clippy_utils::sugg::Sugg, so thanks!

bug in Sugg

@bors
Copy link
Contributor

bors commented Sep 14, 2022

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

@xFrednet
Copy link
Member

This looks good to me, thank you for this enhancement! I hope you had fun getting into Clippy! Furthermore, thanks to @kraktus for the review and suggestion. 🙃

@bors r+

@bors
Copy link
Contributor

bors commented Sep 14, 2022

📌 Commit dd97c1e has been approved by xFrednet

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Sep 14, 2022

⌛ Testing commit dd97c1e with merge bae4699...

@Xaeroxe
Copy link
Contributor Author

Xaeroxe commented Sep 14, 2022

I hope you had fun getting into Clippy!

I did! Thank you

@bors
Copy link
Contributor

bors commented Sep 14, 2022

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

@bors bors merged commit bae4699 into rust-lang:master Sep 14, 2022
@Xaeroxe Xaeroxe deleted the bool-to-int-inverted branch September 14, 2022 20:15
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