Skip to content

Commit

Permalink
Auto merge of #11028 - BenWiederhake:dev-ifnotelse_neqzero, r=llogiq
Browse files Browse the repository at this point in the history
Skip if_not_else lint for '!= 0'-style checks

Currently, clippy makes unhelpful suggestions such as this:

```
warning: unnecessary `!=` operation
   --> src/vm.rs:598:36
    |
598 |                     *destination = if source & 0x8000 != 0 { 0xFFFF } else { 0 };
    |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: change to `==` and swap the blocks of the `if`/`else`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#if_not_else
    = note: `-W clippy::if-not-else` implied by `-W clippy::pedantic`
```

Bit tests often take on the form `if foo & 0x1234 != 0 { … } else { … }`, and the `!= 0` part reads as "has any bits set". Therefore, this code already has the "correct" order, and shouldn't be changed.

This PR disables the lint for these cases, and in fact all cases where the condition is "foo is non-zero".

I did my homework:
- \[X] Followed [lint naming conventions][lint_naming] → Not applicable, this PR fixes an existing lint
- \[X] Added passing UI tests (including committed `.stderr` file) → Yes, `tests/ui/if_not_else_bittest.rs`
- \[X] `cargo test` passes locally
- \[X] Executed `cargo dev update_lints`
- \[X] Added lint documentation → Not applicable, this PR fixes an existing lint
- \[X] Run `cargo dev fmt`

changelog: Fix [`if_not_else`] false positive when something like `bitflags != 0` is used
  • Loading branch information
bors committed Oct 23, 2023
2 parents 56c8235 + 5e17f9f commit f942470
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 1 deletion.
12 changes: 11 additions & 1 deletion clippy_lints/src/if_not_else.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! lint on if branches that could be swapped so no `!` operation is necessary
//! on the condition
use clippy_utils::consts::{constant_simple, Constant};
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::is_else_clause;
use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp};
Expand Down Expand Up @@ -47,6 +48,13 @@ declare_clippy_lint! {

declare_lint_pass!(IfNotElse => [IF_NOT_ELSE]);

fn is_zero_const(expr: &Expr<'_>, cx: &LateContext<'_>) -> bool {
if let Some(value) = constant_simple(cx, cx.typeck_results(), expr) {
return Constant::Int(0) == value;
}
false
}

impl LateLintPass<'_> for IfNotElse {
fn check_expr(&mut self, cx: &LateContext<'_>, item: &Expr<'_>) {
// While loops will be desugared to ExprKind::If. This will cause the lint to fire.
Expand All @@ -72,7 +80,9 @@ impl LateLintPass<'_> for IfNotElse {
"remove the `!` and swap the blocks of the `if`/`else`",
);
},
ExprKind::Binary(ref kind, _, _) if kind.node == BinOpKind::Ne => {
ExprKind::Binary(ref kind, _, lhs) if kind.node == BinOpKind::Ne && !is_zero_const(lhs, cx) => {
// Disable firing the lint on `… != 0`, as these are likely to be bit tests.
// For example, `if foo & 0x0F00 != 0 { … } else { … }` already is in the "proper" order.
span_lint_and_help(
cx,
IF_NOT_ELSE,
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/if_not_else_bittest.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#![deny(clippy::if_not_else)]

fn show_permissions(flags: u32) {
if flags & 0x0F00 != 0 {
println!("Has the 0x0F00 permission.");
} else {
println!("The 0x0F00 permission is missing.");
}
}

fn main() {}

0 comments on commit f942470

Please sign in to comment.