Fix manual_assert for #![no_std] and Rust 2021

rust-lang · Nov 1, 2021 · 7e5f99a · 7e5f99a
1 parent 7cd9bf5
commit 7e5f99a
Show file tree

Hide file tree

Showing 12 changed files with 230 additions and 43 deletions.
diff --git a/clippy_lints/src/manual_assert.rs b/clippy_lints/src/manual_assert.rs
@@ -54,23 +54,24 @@ impl LateLintPass<'_> for ManualAssert {
             if !cx.tcx.sess.source_map().is_multiline(cond.span);
 
             then {
-                let span = if let Some(panic_expn) = PanicExpn::parse(semi) {
+                let call = if_chain! {
+                    if let ExprKind::Block(block, _) = semi.kind;
+                    if let Some(init) = block.expr;
+                    then {
+                        init
+                    } else {
+                        semi
+                    }
+                };
+                let span = if let Some(panic_expn) = PanicExpn::parse(call) {
                     match *panic_expn.format_args.value_args {
                         [] => panic_expn.format_args.format_string_span,
                         [.., last] => panic_expn.format_args.format_string_span.to(last.span),
                     }
+                } else if let ExprKind::Call(_, [format_args]) = call.kind {
+                    format_args.span
                 } else {
-                    if_chain! {
-                        if let ExprKind::Block(block, _) = semi.kind;
-                        if let Some(init) = block.expr;
-                        if let ExprKind::Call(_, [format_args]) = init.kind;
-
-                        then {
-                            format_args.span
-                        } else {
-                            return
-                        }
-                    }
+                    return
                 };
                 let mut applicability = Applicability::MachineApplicable;
                 let sugg = snippet_with_applicability(cx, span, "..", &mut applicability);

diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
@@ -967,8 +967,7 @@ fn check_wild_err_arm<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm
                     }
                     if_chain! {
                         if matching_wild;
-                        if let ExprKind::Block(block, _) = arm.body.kind;
-                        if is_panic_block(block);
+                        if is_panic_call(arm.body);
                         then {
                             // `Err(_)` or `Err(_e)` arm with `panic!` found
                             span_lint_and_note(cx,
@@ -1171,14 +1170,19 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>])
 }
 
 // If the block contains only a `panic!` macro (as expression or statement)
-fn is_panic_block(block: &Block<'_>) -> bool {
-    match (&block.expr, block.stmts.len(), block.stmts.first()) {
-        (&Some(exp), 0, _) => is_expn_of(exp.span, "panic").is_some() && is_expn_of(exp.span, "unreachable").is_none(),
-        (&None, 1, Some(stmt)) => {
-            is_expn_of(stmt.span, "panic").is_some() && is_expn_of(stmt.span, "unreachable").is_none()
-        },
-        _ => false,
-    }
+fn is_panic_call(expr: &Expr<'_>) -> bool {
+    // Unwrap any wrapping blocks
+    let span = if let ExprKind::Block(block, _) = expr.kind {
+        match (&block.expr, block.stmts.len(), block.stmts.first()) {
+            (&Some(exp), 0, _) => exp.span,
+            (&None, 1, Some(stmt)) => stmt.span,
+            _ => return false,
+        }
+    } else {
+        expr.span
+    };
+
+    is_expn_of(span, "panic").is_some() && is_expn_of(span, "unreachable").is_none()
 }
 
 fn check_match_ref_pats<'a, 'b, I>(cx: &LateContext<'_>, ex: &Expr<'_>, pats: I, expr: &Expr<'_>)

diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs
@@ -718,9 +718,7 @@ impl PanicExpn<'tcx> {
     /// Parses an expanded `panic!` invocation
     pub fn parse(expr: &'tcx Expr<'tcx>) -> Option<Self> {
         if_chain! {
-            if let ExprKind::Block(block, _) = expr.kind;
-            if let Some(init) = block.expr;
-            if let ExprKind::Call(_, [format_args]) = init.kind;
+            if let ExprKind::Call(_, [format_args]) = expr.kind;
             let expn_data = expr.span.ctxt().outer_expn_data();
             if let Some(format_args) = FormatArgsExpn::parse(format_args);
             then {

diff --git a/tests/ui/manual_assert.edition2018.fixed b/tests/ui/manual_assert.edition2018.fixed
@@ -0,0 +1,43 @@
+// revisions: edition2018 edition2021
+// [edition2018] edition:2018
+// [edition2021] edition:2021
+// run-rustfix
+#![warn(clippy::manual_assert)]
+
+fn main() {
+    let a = vec![1, 2, 3];
+    let c = Some(2);
+    if !a.is_empty()
+        && a.len() == 3
+        && c != None
+        && !a.is_empty()
+        && a.len() == 3
+        && !a.is_empty()
+        && a.len() == 3
+        && !a.is_empty()
+        && a.len() == 3
+    {
+        panic!("qaqaq{:?}", a);
+    }
+    assert!(a.is_empty(), "qaqaq{:?}", a);
+    assert!(a.is_empty(), "qwqwq");
+    if a.len() == 3 {
+        println!("qwq");
+        println!("qwq");
+        println!("qwq");
+    }
+    if let Some(b) = c {
+        panic!("orz {}", b);
+    }
+    if a.len() == 3 {
+        panic!("qaqaq");
+    } else {
+        println!("qwq");
+    }
+    let b = vec![1, 2, 3];
+    assert!(!b.is_empty(), "panic1");
+    assert!(!(b.is_empty() && a.is_empty()), "panic2");
+    assert!(!(a.is_empty() && !b.is_empty()), "panic3");
+    assert!(!(b.is_empty() || a.is_empty()), "panic4");
+    assert!(!(a.is_empty() || !b.is_empty()), "panic5");
+}
diff --git a/tests/ui/manual_assert.stderr → tests/ui/manual_assert.edition2018.stderr b/tests/ui/manual_assert.stderr → tests/ui/manual_assert.edition2018.stderr
@@ -1,5 +1,5 @@
 error: only a `panic!` in `if`-then statement
-  --> $DIR/manual_assert.rs:21:5
+  --> $DIR/manual_assert.rs:22:5
    |
 LL | /     if !a.is_empty() {
 LL | |         panic!("qaqaq{:?}", a);
@@ -9,47 +9,47 @@ LL | |     }
    = note: `-D clippy::manual-assert` implied by `-D warnings`
 
 error: only a `panic!` in `if`-then statement
-  --> $DIR/manual_assert.rs:24:5
+  --> $DIR/manual_assert.rs:25:5
    |
 LL | /     if !a.is_empty() {
 LL | |         panic!("qwqwq");
 LL | |     }
    | |_____^ help: try: `assert!(a.is_empty(), "qwqwq");`
 
 error: only a `panic!` in `if`-then statement
-  --> $DIR/manual_assert.rs:41:5
+  --> $DIR/manual_assert.rs:42:5
    |
 LL | /     if b.is_empty() {
 LL | |         panic!("panic1");
 LL | |     }
    | |_____^ help: try: `assert!(!b.is_empty(), "panic1");`
 
 error: only a `panic!` in `if`-then statement
-  --> $DIR/manual_assert.rs:44:5
+  --> $DIR/manual_assert.rs:45:5
    |
 LL | /     if b.is_empty() && a.is_empty() {
 LL | |         panic!("panic2");
 LL | |     }
    | |_____^ help: try: `assert!(!(b.is_empty() && a.is_empty()), "panic2");`
 
 error: only a `panic!` in `if`-then statement
-  --> $DIR/manual_assert.rs:47:5
+  --> $DIR/manual_assert.rs:48:5
    |
 LL | /     if a.is_empty() && !b.is_empty() {
 LL | |         panic!("panic3");
 LL | |     }
    | |_____^ help: try: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");`
 
 error: only a `panic!` in `if`-then statement
-  --> $DIR/manual_assert.rs:50:5
+  --> $DIR/manual_assert.rs:51:5
    |
 LL | /     if b.is_empty() || a.is_empty() {
 LL | |         panic!("panic4");
 LL | |     }
    | |_____^ help: try: `assert!(!(b.is_empty() || a.is_empty()), "panic4");`
 
 error: only a `panic!` in `if`-then statement
-  --> $DIR/manual_assert.rs:53:5
+  --> $DIR/manual_assert.rs:54:5
    |
 LL | /     if a.is_empty() || !b.is_empty() {
 LL | |         panic!("panic5");

diff --git a/tests/ui/manual_assert.edition2021.fixed b/tests/ui/manual_assert.edition2021.fixed
@@ -0,0 +1,43 @@
+// revisions: edition2018 edition2021
+// [edition2018] edition:2018
+// [edition2021] edition:2021
+// run-rustfix
+#![warn(clippy::manual_assert)]
+
+fn main() {
+    let a = vec![1, 2, 3];
+    let c = Some(2);
+    if !a.is_empty()
+        && a.len() == 3
+        && c != None
+        && !a.is_empty()
+        && a.len() == 3
+        && !a.is_empty()
+        && a.len() == 3
+        && !a.is_empty()
+        && a.len() == 3
+    {
+        panic!("qaqaq{:?}", a);
+    }
+    assert!(a.is_empty(), "qaqaq{:?}", a);
+    assert!(a.is_empty(), "qwqwq");
+    if a.len() == 3 {
+        println!("qwq");
+        println!("qwq");
+        println!("qwq");
+    }
+    if let Some(b) = c {
+        panic!("orz {}", b);
+    }
+    if a.len() == 3 {
+        panic!("qaqaq");
+    } else {
+        println!("qwq");
+    }
+    let b = vec![1, 2, 3];
+    assert!(!b.is_empty(), "panic1");
+    assert!(!(b.is_empty() && a.is_empty()), "panic2");
+    assert!(!(a.is_empty() && !b.is_empty()), "panic3");
+    assert!(!(b.is_empty() || a.is_empty()), "panic4");
+    assert!(!(a.is_empty() || !b.is_empty()), "panic5");
+}
diff --git a/tests/ui/manual_assert.edition2021.stderr b/tests/ui/manual_assert.edition2021.stderr
@@ -0,0 +1,60 @@
+error: only a `panic!` in `if`-then statement
+  --> $DIR/manual_assert.rs:22:5
+   |
+LL | /     if !a.is_empty() {
+LL | |         panic!("qaqaq{:?}", a);
+LL | |     }
+   | |_____^ help: try: `assert!(a.is_empty(), "qaqaq{:?}", a);`
+   |
+   = note: `-D clippy::manual-assert` implied by `-D warnings`
+
+error: only a `panic!` in `if`-then statement
+  --> $DIR/manual_assert.rs:25:5
+   |
+LL | /     if !a.is_empty() {
+LL | |         panic!("qwqwq");
+LL | |     }
+   | |_____^ help: try: `assert!(a.is_empty(), "qwqwq");`
+
+error: only a `panic!` in `if`-then statement
+  --> $DIR/manual_assert.rs:42:5
+   |
+LL | /     if b.is_empty() {
+LL | |         panic!("panic1");
+LL | |     }
+   | |_____^ help: try: `assert!(!b.is_empty(), "panic1");`
+
+error: only a `panic!` in `if`-then statement
+  --> $DIR/manual_assert.rs:45:5
+   |
+LL | /     if b.is_empty() && a.is_empty() {
+LL | |         panic!("panic2");
+LL | |     }
+   | |_____^ help: try: `assert!(!(b.is_empty() && a.is_empty()), "panic2");`
+
+error: only a `panic!` in `if`-then statement
+  --> $DIR/manual_assert.rs:48:5
+   |
+LL | /     if a.is_empty() && !b.is_empty() {
+LL | |         panic!("panic3");
+LL | |     }
+   | |_____^ help: try: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");`
+
+error: only a `panic!` in `if`-then statement
+  --> $DIR/manual_assert.rs:51:5
+   |
+LL | /     if b.is_empty() || a.is_empty() {
+LL | |         panic!("panic4");
+LL | |     }
+   | |_____^ help: try: `assert!(!(b.is_empty() || a.is_empty()), "panic4");`
+
+error: only a `panic!` in `if`-then statement
+  --> $DIR/manual_assert.rs:54:5
+   |
+LL | /     if a.is_empty() || !b.is_empty() {
+LL | |         panic!("panic5");
+LL | |     }
+   | |_____^ help: try: `assert!(!(a.is_empty() || !b.is_empty()), "panic5");`
+
+error: aborting due to 7 previous errors
+
diff --git a/tests/ui/manual_assert.fixed b/tests/ui/manual_assert.fixed
@@ -1,6 +1,7 @@
-// edition:2018
+// revisions: edition2018 edition2021
+// [edition2018] edition:2018
+// [edition2021] edition:2021
 // run-rustfix
-//FIXME: This does not correctly match in edition 2021, see #7843
 #![warn(clippy::manual_assert)]
 
 fn main() {

diff --git a/tests/ui/manual_assert.rs b/tests/ui/manual_assert.rs
@@ -1,6 +1,7 @@
-// edition:2018
+// revisions: edition2018 edition2021
+// [edition2018] edition:2018
+// [edition2021] edition:2021
 // run-rustfix
-//FIXME: This does not correctly match in edition 2021, see #7843
 #![warn(clippy::manual_assert)]
 
 fn main() {

diff --git a/tests/ui/match_wild_err_arm.stderr → .../ui/match_wild_err_arm.edition2018.stderr b/tests/ui/match_wild_err_arm.stderr → .../ui/match_wild_err_arm.edition2018.stderr
@@ -1,5 +1,5 @@
 error: `Err(_)` matches all errors
-  --> $DIR/match_wild_err_arm.rs:13:9
+  --> $DIR/match_wild_err_arm.rs:14:9
    |
 LL |         Err(_) => panic!("err"),
    |         ^^^^^^
@@ -8,23 +8,23 @@ LL |         Err(_) => panic!("err"),
    = note: match each error separately or use the error output, or use `.except(msg)` if the error case is unreachable
 
 error: `Err(_)` matches all errors
-  --> $DIR/match_wild_err_arm.rs:19:9
+  --> $DIR/match_wild_err_arm.rs:20:9
    |
 LL |         Err(_) => panic!(),
    |         ^^^^^^
    |
    = note: match each error separately or use the error output, or use `.except(msg)` if the error case is unreachable
 
 error: `Err(_)` matches all errors
-  --> $DIR/match_wild_err_arm.rs:25:9
+  --> $DIR/match_wild_err_arm.rs:26:9
    |
 LL |         Err(_) => {
    |         ^^^^^^
    |
    = note: match each error separately or use the error output, or use `.except(msg)` if the error case is unreachable
 
 error: `Err(_e)` matches all errors
-  --> $DIR/match_wild_err_arm.rs:33:9
+  --> $DIR/match_wild_err_arm.rs:34:9
    |
 LL |         Err(_e) => panic!(),
    |         ^^^^^^^

diff --git a/tests/ui/match_wild_err_arm.edition2021.stderr b/tests/ui/match_wild_err_arm.edition2021.stderr
@@ -0,0 +1,35 @@
+error: `Err(_)` matches all errors
+  --> $DIR/match_wild_err_arm.rs:14:9
+   |
+LL |         Err(_) => panic!("err"),
+   |         ^^^^^^
+   |
+   = note: `-D clippy::match-wild-err-arm` implied by `-D warnings`
+   = note: match each error separately or use the error output, or use `.except(msg)` if the error case is unreachable
+
+error: `Err(_)` matches all errors
+  --> $DIR/match_wild_err_arm.rs:20:9
+   |
+LL |         Err(_) => panic!(),
+   |         ^^^^^^
+   |
+   = note: match each error separately or use the error output, or use `.except(msg)` if the error case is unreachable
+
+error: `Err(_)` matches all errors
+  --> $DIR/match_wild_err_arm.rs:26:9
+   |
+LL |         Err(_) => {
+   |         ^^^^^^
+   |
+   = note: match each error separately or use the error output, or use `.except(msg)` if the error case is unreachable
+
+error: `Err(_e)` matches all errors
+  --> $DIR/match_wild_err_arm.rs:34:9
+   |
+LL |         Err(_e) => panic!(),
+   |         ^^^^^^^
+   |
+   = note: match each error separately or use the error output, or use `.except(msg)` if the error case is unreachable
+
+error: aborting due to 4 previous errors
+