Skip to content

Commit

Permalink
Auto merge of #10809 - nyurik:match-unsafe, r=Jarcho
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed May 23, 2023
2 parents ec2f2d5 + ed935de commit fe792d9
Show file tree
Hide file tree
Showing 7 changed files with 640 additions and 10 deletions.
13 changes: 9 additions & 4 deletions clippy_utils/src/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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} }}")
}
}
Expand Down
209 changes: 209 additions & 0 deletions tests/ui/single_match.fixed
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
25 changes: 23 additions & 2 deletions tests/ui/single_match.rs
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down Expand Up @@ -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);
}
},
_ => {},
}
}
44 changes: 43 additions & 1 deletion tests/ui/single_match.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Loading

0 comments on commit fe792d9

Please sign in to comment.