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

New lint: comparing option to Some(true) #4987

Open
ghost opened this issue Jan 3, 2020 · 1 comment
Open

New lint: comparing option to Some(true) #4987

ghost opened this issue Jan 3, 2020 · 1 comment
Labels
A-lint Area: New lints

Comments

@ghost
Copy link

ghost commented Jan 3, 2020

Suggest replacing the expression x == Some(true) with x.unwrap_or(false). To me the second form better expresses what's going on: use the value in x otherwise default to false.

This is similar to the bool_comparison lint but I guess it could be more controversial. I'm OK if it's pedantic or restriction depending on what other people think.

@lolbinarycat
Copy link

the latter is longer, uses a method readers may not be very familiar with (in comparison, every programmer should immediately know what == means), only works with booleans (whereas == Some(x) works on any PartialEq type) and may have worse performance on debug builds (function call instead of a simple bitwise comparison).

if this is implemented, i think it should definitely be restriction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

No branches or pull requests

2 participants