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::then -> bool::then_some within unnecessary_lazy_eval can suggest fixes that can panic #9814

Closed
alokedesai opened this issue Nov 7, 2022 · 2 comments · Fixed by #11002
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@alokedesai
Copy link

alokedesai commented Nov 7, 2022

Summary

The bool::then -> bool::then_some suggestions for unnecessary_lazy_eval can be problematic if the underlying body of the closure is not actually trivial. See repro below for a concrete example.

This is especially problematic because cargo clippy --fix will auto-fix this, which can inadvertently cause a new panic.

Lint Name

unnecessary_lazy_eval

Reproducer

I tried this code:

let foo : usize = 0;
let option = (foo > 0).then_some(|| foo - 1)

I saw this happen:

warning: unnecessary closure used with `bool::then`

This is incorrect, since bool:then will panic in a case where then_some would not have.

I expected to see this happen:
No clippy warning suggested.

Version

rustc 1.65.0 (897e37553 2022-11-02)
binary: rustc
commit-hash: 897e37553bba8b42751c67658967889d11ecd120
commit-date: 2022-11-02
host: aarch64-apple-darwin
release: 1.65.0
LLVM version: 15.0.0

Additional Labels

No response

@alokedesai alokedesai added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Nov 7, 2022
@alokedesai alokedesai changed the title bool::then -> bool::then_some within unnecessary_lazy_eval is error-prone bool::then -> bool::then_some within unnecessary_lazy_eval can suggest fixes that can panic Nov 7, 2022
@kraktus
Copy link
Contributor

kraktus commented Nov 9, 2022

Hey, thanks for reporting.

This seem a duplicate of #9422

@alokedesai
Copy link
Author

@kraktus Yes thats correct! Feel free to close to keep that as the canonical issue :) Would love a status on this--we basically have to allow the unnecessary_lazy_evaluations throughout our project because of the concern it could cause panics with an errant fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants