first finished stacked_if_match

Author: promptkp

clippy_lints/src/stacked_if_match.rs

@@ -1,13 +1,19 @@
+use clippy_utils::source::snippet;
+use rustc_middle::lint::in_external_macro;
 use rustc_hir::*;
-use rustc_lint::{LateContext, LateLintPass};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_session::declare_lint_pass;
+use rustc_errors::Applicability;
+use clippy_utils::visitors::{for_each_expr, Descend};
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use std::ops::ControlFlow;
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Checks for stacked `if` and `match`, e.g., `if if`.
+    /// Checks for `if if` and `match match`.
     ///
     /// ### Why is this bad?
-    /// Stacked `if`'s and `match`'s are hard to read.
+    /// `if if` and `match match` are hard to read.
     ///
     /// ### Example
     /// ```no_run
@@ -17,23 +23,19 @@ declare_clippy_lint! {
     ///     e == f
     /// } {
     ///     println!("true");
-    /// } else {
-    ///     println!("false");
     /// }
     /// ```
     ///
     /// Use instead:
     /// ```no_run
-    /// let cond = if a == b {
+    /// let result = if a == b {
     ///     c == d
     /// } else {
     ///     e == f
     /// };
     ///
-    /// if cond {
+    /// if result {
     ///     println!("true");
-    /// } else {
-    ///     println!("false");
     /// }
     /// ```
     #[clippy::version = "1.82.0"]
@@ -46,6 +48,48 @@ declare_lint_pass!(StackedIfMatch => [STACKED_IF_MATCH]);
 
 impl<'tcx> LateLintPass<'tcx> for StackedIfMatch {
    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        if expr.span.from_expansion() || in_external_macro(cx.sess(), expr.span) {
+            return;
+        }
+
+        let Some((cond, keyword)) = (match expr.kind {
+            ExprKind::If(if_expr, _, _) => Some((if_expr, "if")),
+            ExprKind::Match(match_expr, _, MatchSource::Normal) => Some((match_expr, "match")),
+            _ => None,
+        }) else {
+            return;
+        };
+
+        let cond_snippet = snippet(cx, cond.span, "");
+        if !cond_snippet.starts_with("if") && !cond_snippet.starts_with("match") {
+            return;
+        }
+
+        for_each_expr(cx, cond, |sub_expr| {
+            if matches!(sub_expr.kind, ExprKind::DropTemps(..)) {
+                return ControlFlow::Continue(Descend::Yes);
+            }
+
+            if !sub_expr.span.eq_ctxt(expr.span) || sub_expr.span.lo() != cond.span.lo() {
+                return ControlFlow::Continue(Descend::No);
+            }
 
+            if (keyword == "if" && matches!(sub_expr.kind, ExprKind::If(..)))
+                || (keyword == "match" && matches!(sub_expr.kind, ExprKind::Match(.., MatchSource::Normal))) {
+                let inner_snippet = snippet(cx, sub_expr.span, "..");
+                span_lint_and_sugg(
+                    cx,
+                    STACKED_IF_MATCH,
+                    expr.span.with_hi(sub_expr.span.hi()),
+                    format!("avoid using `{keyword} {keyword}`"),
+                    format!("try binding inner `{keyword}` with `let`"),
+                    format!("let result = {inner_snippet}; {keyword} result"),
+                    Applicability::MachineApplicable,
+                );
+                ControlFlow::Break(())
+            } else {
+                ControlFlow::Continue(Descend::Yes)
+            }
+        });
     }
 }

tests/ui/stacked_if_match.rs

@@ -1,5 +1,132 @@
 #![warn(clippy::stacked_if_match)]
+#![allow(unused)]
 
-fn main() {
-    // test code goes here
+
+fn stacked_if() {
+    let x = 0;
+    if if x == 1 {
+        x == 2
+    } else {
+        x == 3
+    } {
+        println!("true");
+    }
+
+    if if x == 1 {
+        2
+    } else {
+        3
+    } == 4 {
+        println!("true");
+    }
+}
+
+fn stacked_match() {
+    let x = 0;
+    match match x {
+        1 => 2,
+        _ => 3,
+    } {
+        1 => {},
+        2 => {},
+        _ => {},
+    }
+
+    match match x {
+        1 => 2,
+        _ => 3,
+    } + 1 {
+        1 => {},
+        2 => {},
+        _ => {},
+    }
+}
+
+fn if_no_lint() {
+    let x = 0;
+
+    if (if x == 1 {
+        x == 2
+    } else {
+        x == 3
+    }) {
+        println!("true");
+    }
+
+    if 1 == if x == 1 {
+        1
+    } else {
+        2
+    } {
+        println!("true");
+    }
+}
+
+fn match_no_lint() {
+    let x = 0;
+    match (match x {
+        1 => 2,
+        _ => 3,
+    }) {
+        1 => {},
+        2 => {},
+        _ => {},
+    }
+
+    match 1 + match x {
+        1 => 2,
+        _ => 3,
+    } {
+        1 => {},
+        2 => {},
+        _ => {},
+    }
 }
+
+macro_rules! if_macro {
+    ($var:ident) => {
+        if $var == 1 {
+            true
+        } else {
+            false
+        }
+    };
+}
+
+macro_rules! match_macro {
+    ($var:ident) => {
+        match $var {
+            1 => 2,
+            _ => 3,
+        }
+    };
+}
+
+macro_rules! if_if_macro {
+    () => {
+        if if true {
+            true
+        } else {
+            false
+        } {
+            println!("true");
+        }
+    };
+}
+
+fn macro_no_lint() {
+    let x = 0;
+    if if_macro!(x) {
+        println!("true");
+    }
+
+    match match_macro!(x) {
+        1 => {},
+        2 => {},
+        _ => {},
+    }
+
+    if_if_macro!();
+}
+
+fn main() {}