Skip to content

Commit 059cf0e

Browse files
authored
Preserve unsafe blocks in option_map_unit suggestion (#15570)
changelog: [`option_map_unit`]: preserve `unsafe` blocks Fixes #15568
2 parents d97640c + a527e9f commit 059cf0e

File tree

4 files changed

+48
-9
lines changed

4 files changed

+48
-9
lines changed

clippy_lints/src/map_unit_fn.rs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -116,31 +116,35 @@ fn is_unit_expression(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
116116
/// The expression inside a closure may or may not have surrounding braces and
117117
/// semicolons, which causes problems when generating a suggestion. Given an
118118
/// expression that evaluates to '()' or '!', recursively remove useless braces
119-
/// and semi-colons until is suitable for including in the suggestion template
120-
fn reduce_unit_expression(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<Span> {
119+
/// and semi-colons until is suitable for including in the suggestion template.
120+
/// The `bool` is `true` when the resulting `span` needs to be enclosed in an
121+
/// `unsafe` block.
122+
fn reduce_unit_expression(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<(Span, bool)> {
121123
if !is_unit_expression(cx, expr) {
122124
return None;
123125
}
124126

125127
match expr.kind {
126128
hir::ExprKind::Call(_, _) | hir::ExprKind::MethodCall(..) => {
127129
// Calls can't be reduced any more
128-
Some(expr.span)
130+
Some((expr.span, false))
129131
},
130132
hir::ExprKind::Block(block, _) => {
133+
let is_unsafe = matches!(block.rules, hir::BlockCheckMode::UnsafeBlock(_));
131134
match (block.stmts, block.expr.as_ref()) {
132135
([], Some(inner_expr)) => {
133136
// If block only contains an expression,
134137
// reduce `{ X }` to `X`
135138
reduce_unit_expression(cx, inner_expr)
139+
.map(|(span, inner_is_unsafe)| (span, inner_is_unsafe || is_unsafe))
136140
},
137141
([inner_stmt], None) => {
138142
// If block only contains statements,
139143
// reduce `{ X; }` to `X` or `X;`
140144
match inner_stmt.kind {
141-
hir::StmtKind::Let(local) => Some(local.span),
142-
hir::StmtKind::Expr(e) => Some(e.span),
143-
hir::StmtKind::Semi(..) => Some(inner_stmt.span),
145+
hir::StmtKind::Let(local) => Some((local.span, is_unsafe)),
146+
hir::StmtKind::Expr(e) => Some((e.span, is_unsafe)),
147+
hir::StmtKind::Semi(..) => Some((inner_stmt.span, is_unsafe)),
144148
hir::StmtKind::Item(..) => None,
145149
}
146150
},
@@ -228,10 +232,11 @@ fn lint_map_unit_fn(
228232
let msg = suggestion_msg("closure", map_type);
229233

230234
span_lint_and_then(cx, lint, expr.span, msg, |diag| {
231-
if let Some(reduced_expr_span) = reduce_unit_expression(cx, closure_expr) {
235+
if let Some((reduced_expr_span, is_unsafe)) = reduce_unit_expression(cx, closure_expr) {
232236
let mut applicability = Applicability::MachineApplicable;
237+
let (prefix_is_unsafe, suffix_is_unsafe) = if is_unsafe { ("unsafe { ", " }") } else { ("", "") };
233238
let suggestion = format!(
234-
"if let {0}({1}) = {2} {{ {3} }}",
239+
"if let {0}({1}) = {2} {{ {prefix_is_unsafe}{3}{suffix_is_unsafe} }}",
235240
variant,
236241
snippet_with_applicability(cx, binding.pat.span, "_", &mut applicability),
237242
snippet_with_applicability(cx, var_arg.span, "_", &mut applicability),

tests/ui/option_map_unit_fn_fixable.fixed

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,4 +102,13 @@ fn option_map_unit_fn() {
102102
//~^ option_map_unit_fn
103103
}
104104

105+
fn issue15568() {
106+
unsafe fn f(_: u32) {}
107+
let x = Some(3);
108+
if let Some(x) = x { unsafe { f(x) } }
109+
//~^ option_map_unit_fn
110+
if let Some(x) = x { unsafe { f(x) } }
111+
//~^ option_map_unit_fn
112+
}
113+
105114
fn main() {}

tests/ui/option_map_unit_fn_fixable.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,4 +102,13 @@ fn option_map_unit_fn() {
102102
//~^ option_map_unit_fn
103103
}
104104

105+
fn issue15568() {
106+
unsafe fn f(_: u32) {}
107+
let x = Some(3);
108+
x.map(|x| unsafe { f(x) });
109+
//~^ option_map_unit_fn
110+
x.map(|x| unsafe { { f(x) } });
111+
//~^ option_map_unit_fn
112+
}
113+
105114
fn main() {}

tests/ui/option_map_unit_fn_fixable.stderr

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,5 +153,21 @@ LL | option().map(|value| println!("{:?}", value));
153153
| |
154154
| help: try: `if let Some(value) = option() { println!("{:?}", value) }`
155155

156-
error: aborting due to 19 previous errors
156+
error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()`
157+
--> tests/ui/option_map_unit_fn_fixable.rs:108:5
158+
|
159+
LL | x.map(|x| unsafe { f(x) });
160+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^-
161+
| |
162+
| help: try: `if let Some(x) = x { unsafe { f(x) } }`
163+
164+
error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()`
165+
--> tests/ui/option_map_unit_fn_fixable.rs:110:5
166+
|
167+
LL | x.map(|x| unsafe { { f(x) } });
168+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
169+
| |
170+
| help: try: `if let Some(x) = x { unsafe { f(x) } }`
171+
172+
error: aborting due to 21 previous errors
157173

0 commit comments

Comments
 (0)