Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions clippy_lints/src/map_unit_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,31 +116,35 @@ 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;
}

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,
}
},
Expand Down Expand Up @@ -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),
Expand Down
9 changes: 9 additions & 0 deletions tests/ui/option_map_unit_fn_fixable.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
9 changes: 9 additions & 0 deletions tests/ui/option_map_unit_fn_fixable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
18 changes: 17 additions & 1 deletion tests/ui/option_map_unit_fn_fixable.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Loading