diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index b754a45aa40..a6cc1097441 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -530,16 +530,22 @@ fn check_single_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], exp // the lint noisy in unnecessary situations return; } - let els = remove_blocks(&arms[1].body); - let els = if is_unit_expr(els) { + let els = arms[1].body; + let els = if is_unit_expr(remove_blocks(els)) { None - } else if let ExprKind::Block(_, _) = els.kind { - // matches with blocks that contain statements are prettier as `if let + else` - Some(els) + } else if let ExprKind::Block(Block { stmts, expr: block_expr, .. }, _) = els.kind { + if stmts.len() == 1 && block_expr.is_none() || stmts.is_empty() && block_expr.is_some() { + // single statement/expr "else" block, don't lint + return; + } else { + // block with 2+ statements or 1 expr and 1+ statement + Some(els) + } } else { - // allow match arms with just expressions - return; + // not a block, don't lint + return; }; + let ty = cx.tables().expr_ty(ex); if ty.kind != ty::Bool || is_allowed(cx, MATCH_BOOL, ex.hir_id) { check_single_match_single_pattern(cx, ex, arms, expr, els); diff --git a/tests/ui/single_match_else.rs b/tests/ui/single_match_else.rs index 34193be0b75..b624a41a29b 100644 --- a/tests/ui/single_match_else.rs +++ b/tests/ui/single_match_else.rs @@ -1,4 +1,6 @@ #![warn(clippy::single_match_else)] +#![allow(clippy::needless_return)] +#![allow(clippy::no_effect)] enum ExprNode { ExprAddrOf, @@ -30,6 +32,55 @@ macro_rules! unwrap_addr { }; } +#[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 + match Some(1) { + Some(a) => println!("${:?}", a), + None => { + println!("else block"); + return + }, + } + + // lint here + match Some(1) { + Some(a) => println!("${:?}", a), + None => { + println!("else block"); + return; + }, + } } diff --git a/tests/ui/single_match_else.stderr b/tests/ui/single_match_else.stderr index 59861d46eb3..3a07c2ec542 100644 --- a/tests/ui/single_match_else.stderr +++ b/tests/ui/single_match_else.stderr @@ -1,5 +1,5 @@ error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` - --> $DIR/single_match_else.rs:12:5 + --> $DIR/single_match_else.rs:14:5 | LL | / match ExprNode::Butterflies { LL | | ExprNode::ExprAddrOf => Some(&NODE), @@ -19,5 +19,45 @@ LL | None LL | } | -error: aborting due to previous error +error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` + --> $DIR/single_match_else.rs:70:5 + | +LL | / match Some(1) { +LL | | Some(a) => println!("${:?}", a), +LL | | None => { +LL | | println!("else block"); +LL | | return +LL | | }, +LL | | } + | |_____^ + | +help: try this + | +LL | if let Some(a) = Some(1) { println!("${:?}", a) } else { +LL | println!("else block"); +LL | return +LL | } + | + +error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` + --> $DIR/single_match_else.rs:79:5 + | +LL | / match Some(1) { +LL | | Some(a) => println!("${:?}", a), +LL | | None => { +LL | | println!("else block"); +LL | | return; +LL | | }, +LL | | } + | |_____^ + | +help: try this + | +LL | if let Some(a) = Some(1) { println!("${:?}", a) } else { +LL | println!("else block"); +LL | return; +LL | } + | + +error: aborting due to 3 previous errors