diff options
| -rw-r--r-- | clippy_lints/src/map_unit_fn.rs | 21 | ||||
| -rw-r--r-- | tests/ui/option_map_unit_fn_fixable.fixed | 9 | ||||
| -rw-r--r-- | tests/ui/option_map_unit_fn_fixable.rs | 9 | ||||
| -rw-r--r-- | tests/ui/option_map_unit_fn_fixable.stderr | 18 |
4 files changed, 48 insertions, 9 deletions
diff --git a/clippy_lints/src/map_unit_fn.rs b/clippy_lints/src/map_unit_fn.rs index af6a1b07a49..39e5289c62a 100644 --- a/clippy_lints/src/map_unit_fn.rs +++ b/clippy_lints/src/map_unit_fn.rs @@ -116,8 +116,10 @@ fn is_unit_expression(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { /// The expression inside a closure may or may not have surrounding braces and /// semicolons, which causes problems when generating a suggestion. Given an /// expression that evaluates to '()' or '!', recursively remove useless braces -/// and semi-colons until is suitable for including in the suggestion template -fn reduce_unit_expression(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<Span> { +/// and semi-colons until is suitable for including in the suggestion template. +/// The `bool` is `true` when the resulting `span` needs to be enclosed in an +/// `unsafe` block. +fn reduce_unit_expression(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<(Span, bool)> { if !is_unit_expression(cx, expr) { return None; } @@ -125,22 +127,24 @@ fn reduce_unit_expression(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option< match expr.kind { hir::ExprKind::Call(_, _) | hir::ExprKind::MethodCall(..) => { // Calls can't be reduced any more - Some(expr.span) + Some((expr.span, false)) }, hir::ExprKind::Block(block, _) => { + let is_unsafe = matches!(block.rules, hir::BlockCheckMode::UnsafeBlock(_)); match (block.stmts, block.expr.as_ref()) { ([], Some(inner_expr)) => { // If block only contains an expression, // reduce `{ X }` to `X` reduce_unit_expression(cx, inner_expr) + .map(|(span, inner_is_unsafe)| (span, inner_is_unsafe || is_unsafe)) }, ([inner_stmt], None) => { // If block only contains statements, // reduce `{ X; }` to `X` or `X;` match inner_stmt.kind { - hir::StmtKind::Let(local) => Some(local.span), - hir::StmtKind::Expr(e) => Some(e.span), - hir::StmtKind::Semi(..) => Some(inner_stmt.span), + hir::StmtKind::Let(local) => Some((local.span, is_unsafe)), + hir::StmtKind::Expr(e) => Some((e.span, is_unsafe)), + hir::StmtKind::Semi(..) => Some((inner_stmt.span, is_unsafe)), hir::StmtKind::Item(..) => None, } }, @@ -228,10 +232,11 @@ fn lint_map_unit_fn( let msg = suggestion_msg("closure", map_type); span_lint_and_then(cx, lint, expr.span, msg, |diag| { - if let Some(reduced_expr_span) = reduce_unit_expression(cx, closure_expr) { + if let Some((reduced_expr_span, is_unsafe)) = reduce_unit_expression(cx, closure_expr) { let mut applicability = Applicability::MachineApplicable; + let (prefix_is_unsafe, suffix_is_unsafe) = if is_unsafe { ("unsafe { ", " }") } else { ("", "") }; let suggestion = format!( - "if let {0}({1}) = {2} {{ {3} }}", + "if let {0}({1}) = {2} {{ {prefix_is_unsafe}{3}{suffix_is_unsafe} }}", variant, snippet_with_applicability(cx, binding.pat.span, "_", &mut applicability), snippet_with_applicability(cx, var_arg.span, "_", &mut applicability), diff --git a/tests/ui/option_map_unit_fn_fixable.fixed b/tests/ui/option_map_unit_fn_fixable.fixed index 55c1b8f110c..340be7c7e93 100644 --- a/tests/ui/option_map_unit_fn_fixable.fixed +++ b/tests/ui/option_map_unit_fn_fixable.fixed @@ -102,4 +102,13 @@ fn option_map_unit_fn() { //~^ option_map_unit_fn } +fn issue15568() { + unsafe fn f(_: u32) {} + let x = Some(3); + if let Some(x) = x { unsafe { f(x) } } + //~^ option_map_unit_fn + if let Some(x) = x { unsafe { f(x) } } + //~^ option_map_unit_fn +} + fn main() {} diff --git a/tests/ui/option_map_unit_fn_fixable.rs b/tests/ui/option_map_unit_fn_fixable.rs index 5ed47e4c60b..d902c87379b 100644 --- a/tests/ui/option_map_unit_fn_fixable.rs +++ b/tests/ui/option_map_unit_fn_fixable.rs @@ -102,4 +102,13 @@ fn option_map_unit_fn() { //~^ option_map_unit_fn } +fn issue15568() { + unsafe fn f(_: u32) {} + let x = Some(3); + x.map(|x| unsafe { f(x) }); + //~^ option_map_unit_fn + x.map(|x| unsafe { { f(x) } }); + //~^ option_map_unit_fn +} + fn main() {} diff --git a/tests/ui/option_map_unit_fn_fixable.stderr b/tests/ui/option_map_unit_fn_fixable.stderr index 3f7abae34ee..2405aa9a7cc 100644 --- a/tests/ui/option_map_unit_fn_fixable.stderr +++ b/tests/ui/option_map_unit_fn_fixable.stderr @@ -153,5 +153,21 @@ LL | option().map(|value| println!("{:?}", value)); | | | help: try: `if let Some(value) = option() { println!("{:?}", value) }` -error: aborting due to 19 previous errors +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` + --> tests/ui/option_map_unit_fn_fixable.rs:108:5 + | +LL | x.map(|x| unsafe { f(x) }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^- + | | + | help: try: `if let Some(x) = x { unsafe { f(x) } }` + +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` + --> tests/ui/option_map_unit_fn_fixable.rs:110:5 + | +LL | x.map(|x| unsafe { { f(x) } }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- + | | + | help: try: `if let Some(x) = x { unsafe { f(x) } }` + +error: aborting due to 21 previous errors |