Refactor `if_not_else`:

* Check HIR tree first.
* Merge lint calls.

1 files changed, 26 insertions, 37 deletions

diff --git a/clippy_lints/src/if_not_else.rs b/clippy_lints/src/if_not_else.rs
index 4dc1ff83771..2f6daeeb90d 100644
--- a/clippy_lints/src/if_not_else.rs
+++ b/clippy_lints/src/if_not_else.rs
@@ -56,44 +56,33 @@ fn is_zero_const(expr: &Expr<'_>, cx: &LateContext<'_>) -> bool {
 }
 
 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.
-        // To fix this, return early if this span comes from a macro or desugaring.
-        if item.span.from_expansion() {
-            return;
-        }
-        if let ExprKind::If(cond, _, Some(els)) = item.kind {
-            if let ExprKind::Block(..) = els.kind {
-                // Disable firing the lint in "else if" expressions.
-                if is_else_clause(cx.tcx, item) {
-                    return;
-                }
+    fn check_expr(&mut self, cx: &LateContext<'_>, e: &Expr<'_>) {
+        if let ExprKind::If(cond, _, Some(els)) = e.kind
+            && let ExprKind::DropTemps(cond) = cond.kind
+            && let ExprKind::Block(..) = els.kind
+        {
+            let (msg, help) = match cond.kind {
+                ExprKind::Unary(UnOp::Not, _) => (
+                    "unnecessary boolean `not` operation",
+                    "remove the `!` and swap the blocks of the `if`/`else`",
+                ),
+                // Don't lint on `… != 0`, as these are likely to be bit tests.
+                // For example, `if foo & 0x0F00 != 0 { … } else { … }` is already in the "proper" order.
+                ExprKind::Binary(op, _, rhs) if op.node == BinOpKind::Ne && !is_zero_const(rhs, cx) => (
+                    "unnecessary `!=` operation",
+                    "change to `==` and swap the blocks of the `if`/`else`",
+                ),
+                _ => return,
+            };
 
-                match cond.peel_drop_temps().kind {
-                    ExprKind::Unary(UnOp::Not, _) => {
-                        span_lint_and_help(
-                            cx,
-                            IF_NOT_ELSE,
-                            item.span,
-                            "unnecessary boolean `not` operation",
-                            None,
-                            "remove the `!` and swap the blocks of the `if`/`else`",
-                        );
-                    },
-                    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,
-                            item.span,
-                            "unnecessary `!=` operation",
-                            None,
-                            "change to `==` and swap the blocks of the `if`/`else`",
-                        );
-                    },
-                    _ => (),
-                }
+            // `from_expansion` will also catch `while` loops which appear in the HIR as:
+            // ```rust
+            // loop {
+            //     if cond { ... } else { break; }
+            // }
+            // ```
+            if !e.span.from_expansion() && !is_else_clause(cx.tcx, e) {
+                span_lint_and_help(cx, IF_NOT_ELSE, e.span, msg, None, help);
             }
         }
     }