diff --git a/clippy_lints/src/matches/match_single_binding.rs b/clippy_lints/src/matches/match_single_binding.rs index 82d5310663ee..e40e21c490f3 100644 --- a/clippy_lints/src/matches/match_single_binding.rs +++ b/clippy_lints/src/matches/match_single_binding.rs @@ -8,7 +8,7 @@ use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; use rustc_hir::def::Res; use rustc_hir::intravisit::{Visitor, walk_block, walk_expr, walk_path, walk_stmt}; -use rustc_hir::{Arm, Block, Expr, ExprKind, HirId, Node, PatKind, Path, Stmt, StmtKind}; +use rustc_hir::{Arm, Block, Expr, ExprKind, HirId, Item, ItemKind, Node, PatKind, Path, Stmt, StmtKind}; use rustc_lint::LateContext; use rustc_span::{Span, Symbol}; @@ -307,26 +307,6 @@ fn expr_in_nested_block(cx: &LateContext<'_>, match_expr: &Expr<'_>) -> bool { false } -fn expr_must_have_curlies(cx: &LateContext<'_>, match_expr: &Expr<'_>) -> bool { - let parent = cx.tcx.parent_hir_node(match_expr.hir_id); - if let Node::Expr(Expr { - kind: ExprKind::Closure(..) | ExprKind::Binary(..), - .. - }) - | Node::AnonConst(..) = parent - { - return true; - } - - if let Node::Arm(arm) = &cx.tcx.parent_hir_node(match_expr.hir_id) - && let ExprKind::Match(..) = arm.body.kind - { - return true; - } - - false -} - fn indent_of_nth_line(snippet: &str, nth: usize) -> Option { snippet .lines() @@ -379,14 +359,47 @@ fn sugg_with_curlies<'a>( let mut indent = " ".repeat(indent_of(cx, ex.span).unwrap_or(0)); let (mut cbrace_start, mut cbrace_end) = (String::new(), String::new()); - if !expr_in_nested_block(cx, match_expr) - && ((needs_var_binding && is_var_binding_used_later) || expr_must_have_curlies(cx, match_expr)) - { + let mut add_curlies = || { cbrace_end = format!("\n{indent}}}"); // Fix body indent due to the closure indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0)); cbrace_start = format!("{{\n{indent}"); snippet_body = reindent_snippet_if_in_block(&snippet_body, !assignment_str.is_empty()); + }; + + if !expr_in_nested_block(cx, match_expr) { + let mut parent = cx.tcx.parent_hir_node(match_expr.hir_id); + if let Node::Expr(Expr { + kind: ExprKind::Assign(..), + hir_id, + .. + }) = parent + { + parent = cx.tcx.parent_hir_node(*hir_id); + } + if let Node::Stmt(stmt) = parent { + parent = cx.tcx.parent_hir_node(stmt.hir_id); + } + + match parent { + Node::Block(..) + | Node::Expr(Expr { + kind: ExprKind::Block(..) | ExprKind::ConstBlock(..), + .. + }) => { + if needs_var_binding && is_var_binding_used_later { + add_curlies(); + } + }, + Node::Expr(..) + | Node::AnonConst(..) + | Node::Item(Item { + kind: ItemKind::Const(..), + .. + }) => add_curlies(), + Node::Arm(arm) if let ExprKind::Match(..) = arm.body.kind => add_curlies(), + _ => {}, + } } format!("{cbrace_start}{scrutinee};\n{indent}{assignment_str}{snippet_body}{cbrace_end}") diff --git a/tests/ui/match_single_binding.fixed b/tests/ui/match_single_binding.fixed index 7e899a476666..fa82a316d64d 100644 --- a/tests/ui/match_single_binding.fixed +++ b/tests/ui/match_single_binding.fixed @@ -265,3 +265,82 @@ fn issue15269(a: usize, b: usize, c: usize) -> bool { a < b && b < c } + +#[allow( + irrefutable_let_patterns, + clippy::blocks_in_conditions, + clippy::unused_unit, + clippy::let_unit_value, + clippy::unit_arg, + clippy::unnecessary_operation +)] +fn issue15537(a: i32) -> ((), (), ()) { + let y = ( + { todo!() }, + { + { a }; + () + }, + (), + ); + + let y = [ + { todo!() }, + { + { a }; + () + }, + (), + ]; + + fn call(x: (), y: (), z: ()) {} + let y = call( + { todo!() }, + { + { a }; + () + }, + (), + ); + + struct Foo; + impl Foo { + fn method(&self, x: (), y: (), z: ()) {} + } + let x = Foo; + x.method( + { todo!() }, + { + { a }; + () + }, + (), + ); + + -{ + { a }; + 1 + }; + + _ = { a }; + 1; + + if let x = { + { a }; + 1 + } {} + + if { + { a }; + true + } { + todo!() + } + + [1, 2, 3][{ + { a }; + 1usize + }]; + + todo!() +} diff --git a/tests/ui/match_single_binding.rs b/tests/ui/match_single_binding.rs index 37a96f2287c8..6c1fae89e230 100644 --- a/tests/ui/match_single_binding.rs +++ b/tests/ui/match_single_binding.rs @@ -342,3 +342,84 @@ fn issue15269(a: usize, b: usize, c: usize) -> bool { (a, b) => b < c, } } + +#[allow( + irrefutable_let_patterns, + clippy::blocks_in_conditions, + clippy::unused_unit, + clippy::let_unit_value, + clippy::unit_arg, + clippy::unnecessary_operation +)] +fn issue15537(a: i32) -> ((), (), ()) { + let y = ( + { todo!() }, + match { a } { + //~^ match_single_binding + _ => (), + }, + (), + ); + + let y = [ + { todo!() }, + match { a } { + //~^ match_single_binding + _ => (), + }, + (), + ]; + + fn call(x: (), y: (), z: ()) {} + let y = call( + { todo!() }, + match { a } { + //~^ match_single_binding + _ => (), + }, + (), + ); + + struct Foo; + impl Foo { + fn method(&self, x: (), y: (), z: ()) {} + } + let x = Foo; + x.method( + { todo!() }, + match { a } { + //~^ match_single_binding + _ => (), + }, + (), + ); + + -match { a } { + //~^ match_single_binding + _ => 1, + }; + + _ = match { a } { + //~^ match_single_binding + _ => 1, + }; + + if let x = match { a } { + //~^ match_single_binding + _ => 1, + } {} + + if match { a } { + //~^ match_single_binding + _ => true, + } { + todo!() + } + + [1, 2, 3][match { a } { + //~^ match_single_binding + _ => 1usize, + }]; + + todo!() +} diff --git a/tests/ui/match_single_binding.stderr b/tests/ui/match_single_binding.stderr index 82fc43aaa5ea..8a402dcca847 100644 --- a/tests/ui/match_single_binding.stderr +++ b/tests/ui/match_single_binding.stderr @@ -525,5 +525,161 @@ LL | | (a, b) => b < c, LL | | } | |_________^ help: consider using the match body instead: `b < c` -error: aborting due to 37 previous errors +error: this match could be replaced by its scrutinee and body + --> tests/ui/match_single_binding.rs:357:9 + | +LL | / match { a } { +LL | | +LL | | _ => (), +LL | | }, + | |_________^ + | +help: consider using the scrutinee and body instead + | +LL ~ { +LL + { a }; +LL + () +LL ~ }, + | + +error: this match could be replaced by its scrutinee and body + --> tests/ui/match_single_binding.rs:366:9 + | +LL | / match { a } { +LL | | +LL | | _ => (), +LL | | }, + | |_________^ + | +help: consider using the scrutinee and body instead + | +LL ~ { +LL + { a }; +LL + () +LL ~ }, + | + +error: this match could be replaced by its scrutinee and body + --> tests/ui/match_single_binding.rs:376:9 + | +LL | / match { a } { +LL | | +LL | | _ => (), +LL | | }, + | |_________^ + | +help: consider using the scrutinee and body instead + | +LL ~ { +LL + { a }; +LL + () +LL ~ }, + | + +error: this match could be replaced by its scrutinee and body + --> tests/ui/match_single_binding.rs:390:9 + | +LL | / match { a } { +LL | | +LL | | _ => (), +LL | | }, + | |_________^ + | +help: consider using the scrutinee and body instead + | +LL ~ { +LL + { a }; +LL + () +LL ~ }, + | + +error: this match could be replaced by its scrutinee and body + --> tests/ui/match_single_binding.rs:397:6 + | +LL | -match { a } { + | ______^ +LL | | +LL | | _ => 1, +LL | | }; + | |_____^ + | +help: consider using the scrutinee and body instead + | +LL ~ -{ +LL + { a }; +LL + 1 +LL ~ }; + | + +error: this match could be replaced by its scrutinee and body + --> tests/ui/match_single_binding.rs:402:9 + | +LL | _ = match { a } { + | _________^ +LL | | +LL | | _ => 1, +LL | | }; + | |_____^ + | +help: consider using the scrutinee and body instead + | +LL ~ _ = { a }; +LL ~ 1; + | + +error: this match could be replaced by its scrutinee and body + --> tests/ui/match_single_binding.rs:407:16 + | +LL | if let x = match { a } { + | ________________^ +LL | | +LL | | _ => 1, +LL | | } {} + | |_____^ + | +help: consider using the scrutinee and body instead + | +LL ~ if let x = { +LL + { a }; +LL + 1 +LL ~ } {} + | + +error: this match could be replaced by its scrutinee and body + --> tests/ui/match_single_binding.rs:412:8 + | +LL | if match { a } { + | ________^ +LL | | +LL | | _ => true, +LL | | } { + | |_____^ + | +help: consider using the scrutinee and body instead + | +LL ~ if { +LL + { a }; +LL + true +LL ~ } { + | + +error: this match could be replaced by its scrutinee and body + --> tests/ui/match_single_binding.rs:419:15 + | +LL | [1, 2, 3][match { a } { + | _______________^ +LL | | +LL | | _ => 1usize, +LL | | }]; + | |_____^ + | +help: consider using the scrutinee and body instead + | +LL ~ [1, 2, 3][{ +LL + { a }; +LL + 1usize +LL ~ }]; + | + +error: aborting due to 46 previous errors