Add new lint stacked_if_match

CHANGELOG.md

@@ -5931,6 +5931,7 @@ Released 2018-09-13
 [`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next
 [`slow_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#slow_vector_initialization
 [`stable_sort_primitive`]: https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive
+[`stacked_if_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#stacked_if_match
 [`std_instead_of_alloc`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_alloc
 [`std_instead_of_core`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_core
 [`str_split_at_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_split_at_newline

clippy_lints/src/declared_lints.rs

@@ -662,6 +662,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT_INFO,
     crate::size_of_ref::SIZE_OF_REF_INFO,
     crate::slow_vector_initialization::SLOW_VECTOR_INITIALIZATION_INFO,
+    crate::stacked_if_match::STACKED_IF_MATCH_INFO,
     crate::std_instead_of_core::ALLOC_INSTEAD_OF_CORE_INFO,
     crate::std_instead_of_core::STD_INSTEAD_OF_ALLOC_INFO,
     crate::std_instead_of_core::STD_INSTEAD_OF_CORE_INFO,

clippy_lints/src/lib.rs

@@ -335,6 +335,7 @@ mod single_range_in_vec_init;
 mod size_of_in_element_count;
 mod size_of_ref;
 mod slow_vector_initialization;
+mod stacked_if_match;
 mod std_instead_of_core;
 mod string_patterns;
 mod strings;
@@ -942,5 +943,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| Box::new(manual_div_ceil::ManualDivCeil::new(conf)));
     store.register_late_pass(|_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo));
     store.register_late_pass(|_| Box::new(non_zero_suggestions::NonZeroSuggestions));
+    store.register_late_pass(|_| Box::new(stacked_if_match::StackedIfMatch));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

clippy_lints/src/stacked_if_match.rs

@@ -0,0 +1,102 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet;
+use clippy_utils::visitors::{for_each_expr, Descend};
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, MatchSource};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::lint::in_external_macro;
+use rustc_session::declare_lint_pass;
+use std::ops::ControlFlow;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `if if` and `match match`.
+    ///
+    /// ### Why is this bad?
+    /// `if if` and `match match` are hard to read.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # let (a, b, c, d, e, f) = (1, 2, 3, 4, 5, 6);
+    /// if if a == b {
+    ///     c == d
+    /// } else {
+    ///     e == f
+    /// } {
+    ///     println!("true");
+    /// }
+    /// ```
+    ///
+    /// Use instead:
+    /// ```no_run
+    /// # let (a, b, c, d, e, f) = (1, 2, 3, 4, 5, 6);
+    /// let result = if a == b {
+    ///     c == d
+    /// } else {
+    ///     e == f
+    /// };
+    ///
+    /// if result {
+    ///     println!("true");
+    /// }
+    /// ```
+    #[clippy::version = "1.82.0"]
+    pub STACKED_IF_MATCH,
+    style,
+    "`if if` and `match match` that can be eliminated"
+}
+
+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);
+            }
+
+            let sub_keyword = match sub_expr.kind {
+                ExprKind::If(..) => "if",
+                ExprKind::Match(.., MatchSource::Normal) => "match",
+                _ => "",
+            };
+
+            if keyword == sub_keyword {
+                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}` by binding inner `{keyword}` with `let`"),
+                    "try",
+                    format!("let result = {inner_snippet}; {keyword} result"),
+                    Applicability::MachineApplicable,
+                );
+                ControlFlow::Break(())
+            } else {
+                ControlFlow::Continue(Descend::Yes)
+            }
+        });
+    }
+}

tests/ui/match_single_binding.fixed

@@ -5,7 +5,8 @@
     clippy::no_effect,
     clippy::toplevel_ref_arg,
     clippy::uninlined_format_args,
-    clippy::useless_vec
+    clippy::useless_vec,
+    clippy::stacked_if_match
 )]
 
 struct Point {

tests/ui/match_single_binding.rs

@@ -5,7 +5,8 @@
     clippy::no_effect,
     clippy::toplevel_ref_arg,
     clippy::uninlined_format_args,
-    clippy::useless_vec
+    clippy::useless_vec,
+    clippy::stacked_if_match
 )]
 
 struct Point {

tests/ui/match_single_binding.stderr

@@ -1,5 +1,5 @@
 error: this match could be written as a `let` statement
-  --> tests/ui/match_single_binding.rs:33:5
+  --> tests/ui/match_single_binding.rs:34:5
    |
 LL | /     match (a, b, c) {
 LL | |         (x, y, z) => {
@@ -19,7 +19,7 @@ LL +     }
    |
 
 error: this match could be written as a `let` statement
-  --> tests/ui/match_single_binding.rs:39:5
+  --> tests/ui/match_single_binding.rs:40:5
    |
 LL | /     match (a, b, c) {
 LL | |         (x, y, z) => println!("{} {} {}", x, y, z),
@@ -33,15 +33,15 @@ LL +     println!("{} {} {}", x, y, z);
    |
 
 error: this match could be replaced by its body itself
-  --> tests/ui/match_single_binding.rs:56:5
+  --> tests/ui/match_single_binding.rs:57:5
    |
 LL | /     match a {
 LL | |         _ => println!("whatever"),
 LL | |     }
    | |_____^ help: consider using the match body instead: `println!("whatever");`
 
 error: this match could be replaced by its body itself
-  --> tests/ui/match_single_binding.rs:60:5
+  --> tests/ui/match_single_binding.rs:61:5
    |
 LL | /     match a {
 LL | |         _ => {
@@ -60,7 +60,7 @@ LL +     }
    |
 
 error: this match could be replaced by its body itself
-  --> tests/ui/match_single_binding.rs:67:5
+  --> tests/ui/match_single_binding.rs:68:5
    |
 LL | /     match a {
 LL | |         _ => {
@@ -82,7 +82,7 @@ LL +     }
    |
 
 error: this match could be written as a `let` statement
-  --> tests/ui/match_single_binding.rs:77:5
+  --> tests/ui/match_single_binding.rs:78:5
    |
 LL | /     match p {
 LL | |         Point { x, y } => println!("Coords: ({}, {})", x, y),
@@ -96,7 +96,7 @@ LL +     println!("Coords: ({}, {})", x, y);
    |
 
 error: this match could be written as a `let` statement
-  --> tests/ui/match_single_binding.rs:81:5
+  --> tests/ui/match_single_binding.rs:82:5
    |
 LL | /     match p {
 LL | |         Point { x: x1, y: y1 } => println!("Coords: ({}, {})", x1, y1),
@@ -110,7 +110,7 @@ LL +     println!("Coords: ({}, {})", x1, y1);
    |
 
 error: this match could be written as a `let` statement
-  --> tests/ui/match_single_binding.rs:86:5
+  --> tests/ui/match_single_binding.rs:87:5
    |
 LL | /     match x {
 LL | |         ref r => println!("Got a reference to {}", r),
@@ -124,7 +124,7 @@ LL +     println!("Got a reference to {}", r);
    |
 
 error: this match could be written as a `let` statement
-  --> tests/ui/match_single_binding.rs:91:5
+  --> tests/ui/match_single_binding.rs:92:5
    |
 LL | /     match x {
 LL | |         ref mut mr => println!("Got a mutable reference to {}", mr),
@@ -138,7 +138,7 @@ LL +     println!("Got a mutable reference to {}", mr);
    |
 
 error: this match could be written as a `let` statement
-  --> tests/ui/match_single_binding.rs:95:5
+  --> tests/ui/match_single_binding.rs:96:5
    |
 LL | /     let product = match coords() {
 LL | |         Point { x, y } => x * y,
@@ -152,7 +152,7 @@ LL +     let product = x * y;
    |
 
 error: this match could be written as a `let` statement
-  --> tests/ui/match_single_binding.rs:103:18
+  --> tests/ui/match_single_binding.rs:104:18
    |
 LL |           .map(|i| match i.unwrap() {
    |  __________________^
@@ -169,7 +169,7 @@ LL ~         })
    |
 
 error: this match could be replaced by its body itself
-  --> tests/ui/match_single_binding.rs:129:5
+  --> tests/ui/match_single_binding.rs:130:5
    |
 LL | /     match x {
 LL | |         // =>
@@ -178,7 +178,7 @@ LL | |     }
    | |_____^ help: consider using the match body instead: `println!("Not an array index start")`
 
 error: this assignment could be simplified
-  --> tests/ui/match_single_binding.rs:138:5
+  --> tests/ui/match_single_binding.rs:139:5
    |
 LL | /     val = match val.split_at(idx) {
 LL | |         (pre, suf) => {
@@ -198,7 +198,7 @@ LL ~     };
    |
 
 error: this match could be replaced by its scrutinee and body
-  --> tests/ui/match_single_binding.rs:151:16
+  --> tests/ui/match_single_binding.rs:152:16
    |
 LL |       let _ = || match side_effects() {
    |  ________________^
@@ -215,7 +215,7 @@ LL ~     };
    |
 
 error: this match could be written as a `let` statement
-  --> tests/ui/match_single_binding.rs:157:5
+  --> tests/ui/match_single_binding.rs:158:5
    |
 LL | /     match r {
 LL | |         x => match x {
@@ -240,15 +240,15 @@ LL ~     };
    |
 
 error: this match could be replaced by its body itself
-  --> tests/ui/match_single_binding.rs:170:5
+  --> tests/ui/match_single_binding.rs:171:5
    |
 LL | /     match 1 {
 LL | |         _ => (),
 LL | |     }
    | |_____^ help: consider using the match body instead: `();`
 
 error: this match could be replaced by its body itself
-  --> tests/ui/match_single_binding.rs:174:13
+  --> tests/ui/match_single_binding.rs:175:13
    |
 LL |       let a = match 1 {
    |  _____________^
@@ -257,15 +257,15 @@ LL | |     };
    | |_____^ help: consider using the match body instead: `()`
 
 error: this match could be replaced by its body itself
-  --> tests/ui/match_single_binding.rs:178:5
+  --> tests/ui/match_single_binding.rs:179:5
    |
 LL | /     match 1 {
 LL | |         _ => side_effects(),
 LL | |     }
    | |_____^ help: consider using the match body instead: `side_effects();`
 
 error: this match could be replaced by its body itself
-  --> tests/ui/match_single_binding.rs:182:13
+  --> tests/ui/match_single_binding.rs:183:13
    |
 LL |       let b = match 1 {
    |  _____________^
@@ -274,15 +274,15 @@ LL | |     };
    | |_____^ help: consider using the match body instead: `side_effects()`
 
 error: this match could be replaced by its body itself
-  --> tests/ui/match_single_binding.rs:186:5
+  --> tests/ui/match_single_binding.rs:187:5
    |
 LL | /     match 1 {
 LL | |         _ => println!("1"),
 LL | |     }
    | |_____^ help: consider using the match body instead: `println!("1");`
 
 error: this match could be replaced by its body itself
-  --> tests/ui/match_single_binding.rs:190:13
+  --> tests/ui/match_single_binding.rs:191:13
    |
 LL |       let c = match 1 {
    |  _____________^
@@ -291,23 +291,23 @@ LL | |     };
    | |_____^ help: consider using the match body instead: `println!("1")`
 
 error: this match could be replaced by its body itself
-  --> tests/ui/match_single_binding.rs:195:9
+  --> tests/ui/match_single_binding.rs:196:9
    |
 LL | /         match 1 {
 LL | |             _ => (),
 LL | |         },
    | |_________^ help: consider using the match body instead: `()`
 
 error: this match could be replaced by its body itself
-  --> tests/ui/match_single_binding.rs:198:9
+  --> tests/ui/match_single_binding.rs:199:9
    |
 LL | /         match 1 {
 LL | |             _ => side_effects(),
 LL | |         },
    | |_________^ help: consider using the match body instead: `side_effects()`
 
 error: this match could be replaced by its body itself
-  --> tests/ui/match_single_binding.rs:201:9
+  --> tests/ui/match_single_binding.rs:202:9
    |
 LL | /         match 1 {
 LL | |             _ => println!("1"),

tests/ui/match_single_binding2.fixed

@@ -1,6 +1,7 @@
 #![warn(clippy::match_single_binding)]
 #![allow(unused_variables)]
 #![allow(clippy::uninlined_format_args)]
+#![allow(clippy::stacked_if_match)]
 
 fn main() {
     // Lint (additional curly braces needed, see #6572)

tests/ui/match_single_binding2.rs

@@ -1,6 +1,7 @@
 #![warn(clippy::match_single_binding)]
 #![allow(unused_variables)]
 #![allow(clippy::uninlined_format_args)]
+#![allow(clippy::stacked_if_match)]
 
 fn main() {
     // Lint (additional curly braces needed, see #6572)