Auto merge of rust-lang#7851 - nbdd0121:master, r=flip1995

Fix manual_assert and match_wild_err_arm for `#![no_std]` and Rust 2021 Rust 2015 `std::panic!` has a wrapping block while `core::panic!` and Rust 2021 `std::panic!` does not. See rust-lang#88919 for details. Note that the test won't pass until clippy changes in rust-lang#88860 is synced. --- changelog: Fix [`manual_assert`] and [`match_wild_err_arm`] for `#![no_std]` and Rust 2021. Fixes rust-lang#7723
flip1995 · Nov 2, 2021 · 276e895 · 276e895
2 parents 38d8025 + 14e0390
commit 276e895
Show file tree

Hide file tree

Showing 13 changed files with 254 additions and 54 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/missing-test-files.rs b/tests/missing-test-files.rs
@@ -1,7 +1,10 @@
 #![cfg_attr(feature = "deny-warnings", deny(warnings))]
 #![warn(rust_2018_idioms, unused_lifetimes)]
 #![allow(clippy::assertions_on_constants)]
+#![feature(path_file_prefix)]
 
+use std::cmp::Ordering;
+use std::ffi::OsStr;
 use std::fs::{self, DirEntry};
 use std::path::Path;
 
@@ -21,29 +24,39 @@ fn test_missing_tests() {
     }
 }
 
-/*
-Test for missing files.
-
-Since rs files are alphabetically before stderr/stdout, we can sort by the full name
-and iter in that order. If we've seen the file stem for the first time and it's not
-a rust file, it means the rust file has to be missing.
-*/
+// Test for missing files.
 fn explore_directory(dir: &Path) -> Vec<String> {
     let mut missing_files: Vec<String> = Vec::new();
     let mut current_file = String::new();
     let mut files: Vec<DirEntry> = fs::read_dir(dir).unwrap().filter_map(Result::ok).collect();
-    files.sort_by_key(std::fs::DirEntry::path);
+    files.sort_by(|x, y| {
+        match x.path().file_prefix().cmp(&y.path().file_prefix()) {
+            Ordering::Equal => (),
+            ord => return ord,
+        }
+        // Sort rs files before the others if they share the same prefix. So when we see
+        // the file prefix for the first time and it's not a rust file, it means the rust
+        // file has to be missing.
+        match (
+            x.path().extension().and_then(OsStr::to_str),
+            y.path().extension().and_then(OsStr::to_str),
+        ) {
+            (Some("rs"), _) => Ordering::Less,
+            (_, Some("rs")) => Ordering::Greater,
+            _ => Ordering::Equal,
+        }
+    });
     for entry in &files {
         let path = entry.path();
         if path.is_dir() {
             missing_files.extend(explore_directory(&path));
         } else {
-            let file_stem = path.file_stem().unwrap().to_str().unwrap().to_string();
+            let file_prefix = path.file_prefix().unwrap().to_str().unwrap().to_string();
             if let Some(ext) = path.extension() {
                 match ext.to_str().unwrap() {
-                    "rs" => current_file = file_stem.clone(),
+                    "rs" => current_file = file_prefix.clone(),
                     "stderr" | "stdout" => {
-                        if file_stem != current_file {
+                        if file_prefix != current_file {
                             missing_files.push(path.to_str().unwrap().to_string());
                         }
                     },

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() {