Auto merge of #10809 - nyurik:match-unsafe, r=Jarcho

Fix missing block for unsafe code

If a block is declared as unsafe, it needs an extra layer of curly braces around it.

Fixes #10808

This code adds handling for `UnsafeSource::UserProvided` block, i.e. `unsafe { ... }`. Note that we do not handle the `UnsafeSource::CompilerGenerated` as it seems to not be possible to generate that with the user code (?), or at least doesn't seem to be needed to be handled explicitly.

There is an issue with this code: it does not add an extra indentation for the unsafe blocks. I think this is a relatively minor concern for such an edge case, and should probably be done by a separate PR (fixing compile bug is more important than getting styling perfect especially when `rustfmt` will fix it anyway)

```rust
// original code
unsafe {
  ...
}

// code that is now generated by this PR
{ unsafe {
  ...
} }

// what we would ideally like to get
{
  unsafe {
    ...
  }
}
```

changelog: [`single_match`](https://rust-lang.github.io/rust-clippy/master/#single_match): Fix suggestion for `unsafe` blocks

7 files changed, 640 insertions, 10 deletions

diff --git a/clippy_utils/src/source.rs b/clippy_utils/src/source.rs
index 0f60290644a..582337b47e8 100644
--- a/clippy_utils/src/source.rs
+++ b/clippy_utils/src/source.rs
@@ -4,7 +4,7 @@
 
 use rustc_data_structures::sync::Lrc;
 use rustc_errors::Applicability;
-use rustc_hir::{Expr, ExprKind};
+use rustc_hir::{BlockCheckMode, Expr, ExprKind, UnsafeSource};
 use rustc_lint::{LateContext, LintContext};
 use rustc_session::Session;
 use rustc_span::source_map::{original_sp, SourceMap};
@@ -71,11 +71,16 @@ pub fn expr_block<T: LintContext>(
     app: &mut Applicability,
 ) -> String {
     let (code, from_macro) = snippet_block_with_context(cx, expr.span, outer, default, indent_relative_to, app);
-    if from_macro {
-        format!("{{ {code} }}")
-    } else if let ExprKind::Block(_, _) = expr.kind {
+    if !from_macro &&
+        let ExprKind::Block(block, _) = expr.kind &&
+        block.rules != BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided)
+    {
         format!("{code}")
     } else {
+        // FIXME: add extra indent for the unsafe blocks:
+        //     original code:   unsafe { ... }
+        //     result code:     { unsafe { ... } }
+        //     desired code:    {\n  unsafe { ... }\n}
         format!("{{ {code} }}")
     }
 }
diff --git a/tests/ui/single_match.fixed b/tests/ui/single_match.fixed
new file mode 100644
index 00000000000..77a2cf3b991
--- /dev/null
+++ b/tests/ui/single_match.fixed
@@ -0,0 +1,209 @@
+//@run-rustfix
+#![warn(clippy::single_match)]
+#![allow(unused, clippy::uninlined_format_args, clippy::redundant_pattern_matching)]
+fn dummy() {}
+
+fn single_match() {
+    let x = Some(1u8);
+
+    if let Some(y) = x {
+        println!("{:?}", y);
+    };
+
+    let x = Some(1u8);
+    if let Some(y) = x { println!("{:?}", y) }
+
+    let z = (1u8, 1u8);
+    if let (2..=3, 7..=9) = z { dummy() };
+
+    // Not linted (pattern guards used)
+    match x {
+        Some(y) if y == 0 => println!("{:?}", y),
+        _ => (),
+    }
+
+    // Not linted (no block with statements in the single arm)
+    match z {
+        (2..=3, 7..=9) => println!("{:?}", z),
+        _ => println!("nope"),
+    }
+}
+
+enum Foo {
+    Bar,
+    Baz(u8),
+}
+use std::borrow::Cow;
+use Foo::*;
+
+fn single_match_know_enum() {
+    let x = Some(1u8);
+    let y: Result<_, i8> = Ok(1i8);
+
+    if let Some(y) = x { dummy() };
+
+    if let Ok(y) = y { dummy() };
+
+    let c = Cow::Borrowed("");
+
+    if let Cow::Borrowed(..) = c { dummy() };
+
+    let z = Foo::Bar;
+    // no warning
+    match z {
+        Bar => println!("42"),
+        Baz(_) => (),
+    }
+
+    match z {
+        Baz(_) => println!("42"),
+        Bar => (),
+    }
+}
+
+// issue #173
+fn if_suggestion() {
+    let x = "test";
+    if x == "test" { println!() }
+
+    #[derive(PartialEq, Eq)]
+    enum Foo {
+        A,
+        B,
+        C(u32),
+    }
+
+    let x = Foo::A;
+    if x == Foo::A { println!() }
+
+    const FOO_C: Foo = Foo::C(0);
+    if x == FOO_C { println!() }
+
+    if x == Foo::A { println!() }
+
+    let x = &x;
+    if x == &Foo::A { println!() }
+
+    enum Bar {
+        A,
+        B,
+    }
+    impl PartialEq for Bar {
+        fn eq(&self, rhs: &Self) -> bool {
+            matches!((self, rhs), (Self::A, Self::A) | (Self::B, Self::B))
+        }
+    }
+    impl Eq for Bar {}
+
+    let x = Bar::A;
+    if let Bar::A = x { println!() }
+
+    // issue #7038
+    struct X;
+    let x = Some(X);
+    if let None = x { println!() };
+}
+
+// See: issue #8282
+fn ranges() {
+    enum E {
+        V,
+    }
+    let x = (Some(E::V), Some(42));
+
+    // Don't lint, because the `E` enum can be extended with additional fields later. Thus, the
+    // proposed replacement to `if let Some(E::V)` may hide non-exhaustive warnings that appeared
+    // because of `match` construction.
+    match x {
+        (Some(E::V), _) => {},
+        (None, _) => {},
+    }
+
+    // lint
+    if let (Some(_), _) = x {}
+
+    // lint
+    if let (Some(E::V), _) = x { todo!() }
+
+    // lint
+    if let (.., Some(E::V), _) = (Some(42), Some(E::V), Some(42)) {}
+
+    // Don't lint, see above.
+    match (Some(E::V), Some(E::V), Some(E::V)) {
+        (.., Some(E::V), _) => {},
+        (.., None, _) => {},
+    }
+
+    // Don't lint, see above.
+    match (Some(E::V), Some(E::V), Some(E::V)) {
+        (Some(E::V), ..) => {},
+        (None, ..) => {},
+    }
+
+    // Don't lint, see above.
+    match (Some(E::V), Some(E::V), Some(E::V)) {
+        (_, Some(E::V), ..) => {},
+        (_, None, ..) => {},
+    }
+}
+
+fn skip_type_aliases() {
+    enum OptionEx {
+        Some(i32),
+        None,
+    }
+    enum ResultEx {
+        Err(i32),
+        Ok(i32),
+    }
+
+    use OptionEx::{None, Some};
+    use ResultEx::{Err, Ok};
+
+    // don't lint
+    match Err(42) {
+        Ok(_) => dummy(),
+        Err(_) => (),
+    };
+
+    // don't lint
+    match Some(1i32) {
+        Some(_) => dummy(),
+        None => (),
+    };
+}
+
+macro_rules! single_match {
+    ($num:literal) => {
+        match $num {
+            15 => println!("15"),
+            _ => (),
+        }
+    };
+}
+
+fn main() {
+    single_match!(5);
+
+    // Don't lint
+    let _ = match Some(0) {
+        #[cfg(feature = "foo")]
+        Some(10) => 11,
+        Some(x) => x,
+        _ => 0,
+    };
+}
+
+fn issue_10808(bar: Option<i32>) {
+    if let Some(v) = bar { unsafe {
+        let r = &v as *const i32;
+        println!("{}", *r);
+    } }
+
+    if let Some(v) = bar {
+        unsafe {
+            let r = &v as *const i32;
+            println!("{}", *r);
+        }
+    }
+}
diff --git a/tests/ui/single_match.rs b/tests/ui/single_match.rs
index d0c9b7b5663..8d0ab5b99ad 100644
--- a/tests/ui/single_match.rs
+++ b/tests/ui/single_match.rs
@@ -1,6 +1,6 @@
+//@run-rustfix
 #![warn(clippy::single_match)]
-#![allow(clippy::uninlined_format_args)]
-
+#![allow(unused, clippy::uninlined_format_args, clippy::redundant_pattern_matching)]
 fn dummy() {}
 
 fn single_match() {
@@ -244,3 +244,24 @@ fn main() {
         _ => 0,
     };
 }
+
+fn issue_10808(bar: Option<i32>) {
+    match bar {
+        Some(v) => unsafe {
+            let r = &v as *const i32;
+            println!("{}", *r);
+        },
+        _ => {},
+    }
+
+    match bar {
+        #[rustfmt::skip]
+        Some(v) => {
+            unsafe {
+                let r = &v as *const i32;
+                println!("{}", *r);
+            }
+        },
+        _ => {},
+    }
+}
diff --git a/tests/ui/single_match.stderr b/tests/ui/single_match.stderr
index 7cecc1b7395..dad66e2ab2e 100644
--- a/tests/ui/single_match.stderr
+++ b/tests/ui/single_match.stderr
@@ -155,5 +155,47 @@ LL | |         (..) => {},
 LL | |     }
    | |_____^ help: try this: `if let (.., Some(E::V), _) = (Some(42), Some(E::V), Some(42)) {}`
 
-error: aborting due to 16 previous errors
+error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
+  --> $DIR/single_match.rs:249:5
+   |
+LL | /     match bar {
+LL | |         Some(v) => unsafe {
+LL | |             let r = &v as *const i32;
+LL | |             println!("{}", *r);
+LL | |         },
+LL | |         _ => {},
+LL | |     }
+   | |_____^
+   |
+help: try this
+   |
+LL ~     if let Some(v) = bar { unsafe {
+LL +         let r = &v as *const i32;
+LL +         println!("{}", *r);
+LL +     } }
+   |
+
+error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
+  --> $DIR/single_match.rs:257:5
+   |
+LL | /     match bar {
+LL | |         #[rustfmt::skip]
+LL | |         Some(v) => {
+LL | |             unsafe {
+...  |
+LL | |         _ => {},
+LL | |     }
+   | |_____^
+   |
+help: try this
+   |
+LL ~     if let Some(v) = bar {
+LL +         unsafe {
+LL +             let r = &v as *const i32;
+LL +             println!("{}", *r);
+LL +         }
+LL +     }
+   |
+
+error: aborting due to 18 previous errors
 
diff --git a/tests/ui/single_match_else.fixed b/tests/ui/single_match_else.fixed
new file mode 100644
index 00000000000..f88498655a4
--- /dev/null
+++ b/tests/ui/single_match_else.fixed
@@ -0,0 +1,173 @@
+//@run-rustfix
+//@aux-build: proc_macros.rs
+#![warn(clippy::single_match_else)]
+#![allow(unused, clippy::needless_return, clippy::no_effect, clippy::uninlined_format_args)]
+extern crate proc_macros;
+use proc_macros::with_span;
+
+enum ExprNode {
+    ExprAddrOf,
+    Butterflies,
+    Unicorns,
+}
+
+static NODE: ExprNode = ExprNode::Unicorns;
+
+fn unwrap_addr() -> Option<&'static ExprNode> {
+    let _ = if let ExprNode::ExprAddrOf = ExprNode::Butterflies { Some(&NODE) } else {
+        let x = 5;
+        None
+    };
+
+    // Don't lint
+    with_span!(span match ExprNode::Butterflies {
+        ExprNode::ExprAddrOf => Some(&NODE),
+        _ => {
+            let x = 5;
+            None
+        },
+    })
+}
+
+macro_rules! unwrap_addr {
+    ($expression:expr) => {
+        match $expression {
+            ExprNode::ExprAddrOf => Some(&NODE),
+            _ => {
+                let x = 5;
+                None
+            },
+        }
+    };
+}
+
+#[rustfmt::skip]
+fn main() {
+    unwrap_addr!(ExprNode::Unicorns);
+
+    //
+    // don't lint single exprs/statements
+    //
+
+    // don't lint here
+    match Some(1) {
+        Some(a) => println!("${:?}", a),
+        None => return,
+    }
+
+    // don't lint here
+    match Some(1) {
+        Some(a) => println!("${:?}", a),
+        None => {
+            return
+        },
+    }
+
+    // don't lint here
+    match Some(1) {
+        Some(a) => println!("${:?}", a),
+        None => {
+            return;
+        },
+    }
+
+    //
+    // lint multiple exprs/statements "else" blocks
+    //
+
+    // lint here
+    if let Some(a) = Some(1) { println!("${:?}", a) } else {
+        println!("else block");
+        return
+    }
+
+    // lint here
+    if let Some(a) = Some(1) { println!("${:?}", a) } else {
+        println!("else block");
+        return;
+    }
+
+    // lint here
+    use std::convert::Infallible;
+    if let Ok(a) = Result::<i32, Infallible>::Ok(1) { println!("${:?}", a) } else {
+        println!("else block");
+        return;
+    }
+
+    use std::borrow::Cow;
+    if let Cow::Owned(a) = Cow::from("moo") { println!("${:?}", a) } else {
+        println!("else block");
+        return;
+    }
+}
+
+fn issue_10808(bar: Option<i32>) {
+    if let Some(v) = bar { unsafe {
+        let r = &v as *const i32;
+        println!("{}", *r);
+    } } else {
+        println!("None1");
+        println!("None2");
+    }
+
+    if let Some(v) = bar {
+        println!("Some");
+        println!("{v}");
+    } else { unsafe {
+        let v = 0;
+        let r = &v as *const i32;
+        println!("{}", *r);
+    } }
+
+    if let Some(v) = bar { unsafe {
+        let r = &v as *const i32;
+        println!("{}", *r);
+    } } else { unsafe {
+        let v = 0;
+        let r = &v as *const i32;
+        println!("{}", *r);
+    } }
+
+    if let Some(v) = bar {
+        unsafe {
+            let r = &v as *const i32;
+            println!("{}", *r);
+        }
+    } else {
+        println!("None");
+        println!("None");
+    }
+
+    match bar {
+        Some(v) => {
+            println!("Some");
+            println!("{v}");
+        },
+        #[rustfmt::skip]
+        None => {
+            unsafe {
+                let v = 0;
+                let r = &v as *const i32;
+                println!("{}", *r);
+            }
+        },
+    }
+
+    match bar {
+        #[rustfmt::skip]
+        Some(v) => {
+            unsafe {
+                let r = &v as *const i32;
+                println!("{}", *r);
+            }
+        },
+        #[rustfmt::skip]
+        None => {
+            unsafe {
+                let v = 0;
+                let r = &v as *const i32;
+                println!("{}", *r);
+            }
+        },
+    }
+}
diff --git a/tests/ui/single_match_else.rs b/tests/ui/single_match_else.rs
index c8ac768b60f..b34b9553919 100644
--- a/tests/ui/single_match_else.rs
+++ b/tests/ui/single_match_else.rs
@@ -1,7 +1,7 @@
+//@run-rustfix
 //@aux-build: proc_macros.rs
 #![warn(clippy::single_match_else)]
-#![allow(clippy::needless_return, clippy::no_effect, clippy::uninlined_format_args)]
-
+#![allow(unused, clippy::needless_return, clippy::no_effect, clippy::uninlined_format_args)]
 extern crate proc_macros;
 use proc_macros::with_span;
 
@@ -115,3 +115,87 @@ fn main() {
         }
     }
 }
+
+fn issue_10808(bar: Option<i32>) {
+    match bar {
+        Some(v) => unsafe {
+            let r = &v as *const i32;
+            println!("{}", *r);
+        },
+        None => {
+            println!("None1");
+            println!("None2");
+        },
+    }
+
+    match bar {
+        Some(v) => {
+            println!("Some");
+            println!("{v}");
+        },
+        None => unsafe {
+            let v = 0;
+            let r = &v as *const i32;
+            println!("{}", *r);
+        },
+    }
+
+    match bar {
+        Some(v) => unsafe {
+            let r = &v as *const i32;
+            println!("{}", *r);
+        },
+        None => unsafe {
+            let v = 0;
+            let r = &v as *const i32;
+            println!("{}", *r);
+        },
+    }
+
+    match bar {
+        #[rustfmt::skip]
+        Some(v) => {
+            unsafe {
+                let r = &v as *const i32;
+                println!("{}", *r);
+            }
+        },
+        None => {
+            println!("None");
+            println!("None");
+        },
+    }
+
+    match bar {
+        Some(v) => {
+            println!("Some");
+            println!("{v}");
+        },
+        #[rustfmt::skip]
+        None => {
+            unsafe {
+                let v = 0;
+                let r = &v as *const i32;
+                println!("{}", *r);
+            }
+        },
+    }
+
+    match bar {
+        #[rustfmt::skip]
+        Some(v) => {
+            unsafe {
+                let r = &v as *const i32;
+                println!("{}", *r);
+            }
+        },
+        #[rustfmt::skip]
+        None => {
+            unsafe {
+                let v = 0;
+                let r = &v as *const i32;
+                println!("{}", *r);
+            }
+        },
+    }
+}
diff --git a/tests/ui/single_match_else.stderr b/tests/ui/single_match_else.stderr
index 62876a55dc6..228236f3bb8 100644
--- a/tests/ui/single_match_else.stderr
+++ b/tests/ui/single_match_else.stderr
@@ -100,5 +100,101 @@ LL +         return;
 LL +     }
    |
 
-error: aborting due to 5 previous errors
+error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
+  --> $DIR/single_match_else.rs:120:5
+   |
+LL | /     match bar {
+LL | |         Some(v) => unsafe {
+LL | |             let r = &v as *const i32;
+LL | |             println!("{}", *r);
+...  |
+LL | |         },
+LL | |     }
+   | |_____^
+   |
+help: try this
+   |
+LL ~     if let Some(v) = bar { unsafe {
+LL +         let r = &v as *const i32;
+LL +         println!("{}", *r);
+LL +     } } else {
+LL +         println!("None1");
+LL +         println!("None2");
+LL +     }
+   |
+
+error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
+  --> $DIR/single_match_else.rs:131:5
+   |
+LL | /     match bar {
+LL | |         Some(v) => {
+LL | |             println!("Some");
+LL | |             println!("{v}");
+...  |
+LL | |         },
+LL | |     }
+   | |_____^
+   |
+help: try this
+   |
+LL ~     if let Some(v) = bar {
+LL +         println!("Some");
+LL +         println!("{v}");
+LL +     } else { unsafe {
+LL +         let v = 0;
+LL +         let r = &v as *const i32;
+LL +         println!("{}", *r);
+LL +     } }
+   |
+
+error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
+  --> $DIR/single_match_else.rs:143:5
+   |
+LL | /     match bar {
+LL | |         Some(v) => unsafe {
+LL | |             let r = &v as *const i32;
+LL | |             println!("{}", *r);
+...  |
+LL | |         },
+LL | |     }
+   | |_____^
+   |
+help: try this
+   |
+LL ~     if let Some(v) = bar { unsafe {
+LL +         let r = &v as *const i32;
+LL +         println!("{}", *r);
+LL +     } } else { unsafe {
+LL +         let v = 0;
+LL +         let r = &v as *const i32;
+LL +         println!("{}", *r);
+LL +     } }
+   |
+
+error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
+  --> $DIR/single_match_else.rs:155:5
+   |
+LL | /     match bar {
+LL | |         #[rustfmt::skip]
+LL | |         Some(v) => {
+LL | |             unsafe {
+...  |
+LL | |         },
+LL | |     }
+   | |_____^
+   |
+help: try this
+   |
+LL ~     if let Some(v) = bar {
+LL +         unsafe {
+LL +             let r = &v as *const i32;
+LL +             println!("{}", *r);
+LL +         }
+LL +     } else {
+LL +         println!("None");
+LL +         println!("None");
+LL +     }
+   |
+
+error: aborting due to 9 previous errors