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
This commit is contained in:
bors 2023-05-23 04:45:14 +00:00
commit fe792d9f7d
7 changed files with 640 additions and 10 deletions

View File

@ -4,7 +4,7 @@
use rustc_data_structures::sync::Lrc; use rustc_data_structures::sync::Lrc;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind}; use rustc_hir::{BlockCheckMode, Expr, ExprKind, UnsafeSource};
use rustc_lint::{LateContext, LintContext}; use rustc_lint::{LateContext, LintContext};
use rustc_session::Session; use rustc_session::Session;
use rustc_span::source_map::{original_sp, SourceMap}; use rustc_span::source_map::{original_sp, SourceMap};
@ -71,11 +71,16 @@ pub fn expr_block<T: LintContext>(
app: &mut Applicability, app: &mut Applicability,
) -> String { ) -> String {
let (code, from_macro) = snippet_block_with_context(cx, expr.span, outer, default, indent_relative_to, app); let (code, from_macro) = snippet_block_with_context(cx, expr.span, outer, default, indent_relative_to, app);
if from_macro { if !from_macro &&
format!("{{ {code} }}") let ExprKind::Block(block, _) = expr.kind &&
} else if let ExprKind::Block(_, _) = expr.kind { block.rules != BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided)
{
format!("{code}") format!("{code}")
} else { } else {
// FIXME: add extra indent for the unsafe blocks:
// original code: unsafe { ... }
// result code: { unsafe { ... } }
// desired code: {\n unsafe { ... }\n}
format!("{{ {code} }}") format!("{{ {code} }}")
} }
} }

209
tests/ui/single_match.fixed Normal file
View File

@ -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);
}
}
}

View File

@ -1,6 +1,6 @@
//@run-rustfix
#![warn(clippy::single_match)] #![warn(clippy::single_match)]
#![allow(clippy::uninlined_format_args)] #![allow(unused, clippy::uninlined_format_args, clippy::redundant_pattern_matching)]
fn dummy() {} fn dummy() {}
fn single_match() { fn single_match() {
@ -244,3 +244,24 @@ fn main() {
_ => 0, _ => 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);
}
},
_ => {},
}
}

View File

@ -155,5 +155,47 @@ LL | | (..) => {},
LL | | } LL | | }
| |_____^ help: try this: `if let (.., Some(E::V), _) = (Some(42), Some(E::V), Some(42)) {}` | |_____^ 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

View File

@ -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);
}
},
}
}

View File

@ -1,7 +1,7 @@
//@run-rustfix
//@aux-build: proc_macros.rs //@aux-build: proc_macros.rs
#![warn(clippy::single_match_else)] #![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; extern crate proc_macros;
use proc_macros::with_span; 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);
}
},
}
}

View File

@ -100,5 +100,101 @@ LL + return;
LL + } 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