about summary refs log tree commit diff
diff options
context:
space:
mode:
authorThibsG <thibsg@pm.me>2021-03-20 16:11:19 +0100
committerThibsG <thibsg@pm.me>2021-03-20 16:11:19 +0100
commit00a2d7ad7e1a041e47f618a019702bfb37eca680 (patch)
treed7b7adcbd5c147fcd8453c25151b79d115be1eec
parent1d3c539fbbdddc2c0ab2c512e70f4f6ea2d37c5e (diff)
downloadrust-00a2d7ad7e1a041e47f618a019702bfb37eca680.tar.gz
rust-00a2d7ad7e1a041e47f618a019702bfb37eca680.zip
Fix bad suggestion that needs curly braces for `match_single_binding` lint
-rw-r--r--clippy_lints/src/matches.rs14
-rw-r--r--tests/ui/match_single_binding.fixed29
-rw-r--r--tests/ui/match_single_binding.rs29
-rw-r--r--tests/ui/match_single_binding.stderr33
4 files changed, 103 insertions, 2 deletions
diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
index d43cb32ee51..1a8ec9c6ec3 100644
--- a/clippy_lints/src/matches.rs
+++ b/clippy_lints/src/matches.rs
@@ -1353,6 +1353,7 @@ fn find_bool_lit(ex: &ExprKind<'_>, desugared: bool) -> Option<bool> {
     }
 }
 
+#[allow(clippy::too_many_lines)]
 fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], expr: &Expr<'_>) {
     if in_macro(expr.span) || arms.len() != 1 || is_refutable(cx, arms[0].pat) {
         return;
@@ -1427,7 +1428,18 @@ fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[A
                         indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
                         cbrace_start = format!("{{\n{}", indent);
                     }
-                };
+                }
+                // If the parent is already an arm, and the body is another match statement,
+                // we need curly braces around suggestion
+                let parent_node_id = cx.tcx.hir().get_parent_node(expr.hir_id);
+                if let Node::Arm(arm) = &cx.tcx.hir().get(parent_node_id) {
+                    if let ExprKind::Match(..) = arm.body.kind {
+                        cbrace_end = format!("\n{}}}", indent);
+                        // Fix body indent due to the match
+                        indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
+                        cbrace_start = format!("{{\n{}", indent);
+                    }
+                }
                 (
                     expr.span,
                     format!(
diff --git a/tests/ui/match_single_binding.fixed b/tests/ui/match_single_binding.fixed
index 526e94b10bd..4709b5b0157 100644
--- a/tests/ui/match_single_binding.fixed
+++ b/tests/ui/match_single_binding.fixed
@@ -115,4 +115,33 @@ fn main() {
         // =>
         _ => println!("Not an array index start"),
     }
+    // Lint (additional curly braces needed, see #6572)
+    struct AppendIter<I>
+    where
+        I: Iterator,
+    {
+        inner: Option<(I, <I as Iterator>::Item)>,
+    }
+
+    #[allow(dead_code)]
+    fn size_hint<I: Iterator>(iter: &AppendIter<I>) -> (usize, Option<usize>) {
+        match &iter.inner {
+            Some((iter, _item)) => {
+                let (min, max) = iter.size_hint();
+                (min.saturating_add(1), max.and_then(|max| max.checked_add(1)))
+            },
+            None => (0, Some(0)),
+        }
+    }
+    // Lint (no additional curly braces needed)
+    let opt = Some((5, 2));
+    let get_tup = || -> (i32, i32) { (1, 2) };
+    match opt {
+        #[rustfmt::skip]
+        Some((first, _second)) => {
+            let (a, b) = get_tup();
+            println!("a {:?} and b {:?}", a, b);
+        },
+        None => println!("nothing"),
+    }
 }
diff --git a/tests/ui/match_single_binding.rs b/tests/ui/match_single_binding.rs
index 6a2ca7c5e93..6a6b3e8e8a9 100644
--- a/tests/ui/match_single_binding.rs
+++ b/tests/ui/match_single_binding.rs
@@ -132,4 +132,33 @@ fn main() {
         // =>
         _ => println!("Not an array index start"),
     }
+    // Lint (additional curly braces needed, see #6572)
+    struct AppendIter<I>
+    where
+        I: Iterator,
+    {
+        inner: Option<(I, <I as Iterator>::Item)>,
+    }
+
+    #[allow(dead_code)]
+    fn size_hint<I: Iterator>(iter: &AppendIter<I>) -> (usize, Option<usize>) {
+        match &iter.inner {
+            Some((iter, _item)) => match iter.size_hint() {
+                (min, max) => (min.saturating_add(1), max.and_then(|max| max.checked_add(1))),
+            },
+            None => (0, Some(0)),
+        }
+    }
+    // Lint (no additional curly braces needed)
+    let opt = Some((5, 2));
+    let get_tup = || -> (i32, i32) { (1, 2) };
+    match opt {
+        #[rustfmt::skip]
+        Some((first, _second)) => {
+            match get_tup() {
+                (a, b) => println!("a {:?} and b {:?}", a, b),
+            }
+        },
+        None => println!("nothing"),
+    }
 }
diff --git a/tests/ui/match_single_binding.stderr b/tests/ui/match_single_binding.stderr
index cbbf5d29c02..73cc867dd9f 100644
--- a/tests/ui/match_single_binding.stderr
+++ b/tests/ui/match_single_binding.stderr
@@ -178,5 +178,36 @@ LL | |         _ => println!("Single branch"),
 LL | |     }
    | |_____^ help: consider using the match body instead: `println!("Single branch");`
 
-error: aborting due to 12 previous errors
+error: this match could be written as a `let` statement
+  --> $DIR/match_single_binding.rs:146:36
+   |
+LL |               Some((iter, _item)) => match iter.size_hint() {
+   |  ____________________________________^
+LL | |                 (min, max) => (min.saturating_add(1), max.and_then(|max| max.checked_add(1))),
+LL | |             },
+   | |_____________^
+   |
+help: consider using `let` statement
+   |
+LL |             Some((iter, _item)) => {
+LL |                 let (min, max) = iter.size_hint();
+LL |                 (min.saturating_add(1), max.and_then(|max| max.checked_add(1)))
+LL |             },
+   |
+
+error: this match could be written as a `let` statement
+  --> $DIR/match_single_binding.rs:158:13
+   |
+LL | /             match get_tup() {
+LL | |                 (a, b) => println!("a {:?} and b {:?}", a, b),
+LL | |             }
+   | |_____________^
+   |
+help: consider using `let` statement
+   |
+LL |             let (a, b) = get_tup();
+LL |             println!("a {:?} and b {:?}", a, b);
+   |
+
+error: aborting due to 14 previous errors