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_comparison should not fire when testing for false #1043

Open
svmnotn opened this issue Jun 26, 2016 · 4 comments
Open

bool_comparison should not fire when testing for false #1043

svmnotn opened this issue Jun 26, 2016 · 4 comments
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy L-style Lint: Belongs in the style lint group S-needs-discussion Status: Needs further discussion before merging or work can be started

Comments

@svmnotn
Copy link

svmnotn commented Jun 26, 2016

I think that the bool_comparison lint should not fire when the comparison is var == false because, at least I, find that version much easier to see when quickly scrolling through the code than the !var as it is too simple to miss that single !.
However, I'm just putting this here for discussion I'll go turn the lint off in that specific place.

Enjoy your rusty endeavors.

@Manishearth
Copy link
Member

Listed in #533

@oli-obk
Copy link
Contributor

oli-obk commented Jun 27, 2016

@svmnotn I think turning it off for a specific project is the right way to go. But I'd go even further in that case: write a (restriction) lint that forbids !var for booleans. This way you can be sure that your code base doesn't contain the opposite.

@mcarton mcarton added the S-needs-discussion Status: Needs further discussion before merging or work can be started label Sep 5, 2016
@golddranks
Copy link

I'd rather have exp == false than !exp, because ! is visually hard to see. The blog post https://eev.ee/blog/2016/12/01/lets-stop-copying-c/#_1 makes excellent point against that.

But even if the style guidelines end up supporting ! over == false, I think that Clippy should lint if there is not whitespace around ! – that at least helps a bit visually.

@oli-obk oli-obk added L-style Lint: Belongs in the style lint group good-first-issue These issues are a good way to get started with Clippy A-lint Area: New lints labels Jan 24, 2017
@workingjubilee
Copy link
Contributor

I came here to add to the lamentations because it is my experience that very often Clippy recommends the ! on expressions like ident.is_cond_true() which is directly against the flow of the overall reading, and in general negative reasoning is harder for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy L-style Lint: Belongs in the style lint group S-needs-discussion Status: Needs further discussion before merging or work can be started
Projects
None yet
Development

No branches or pull requests

7 participants