From f39ed7abd823701ae62a272c54af389b7c644070 Mon Sep 17 00:00:00 2001 From: yanglsh Date: Sat, 23 Aug 2025 06:52:15 +0800 Subject: [PATCH 1/2] fix: `match_single_binding` suggests wrongly inside tuple --- .../src/matches/match_single_binding.rs | 54 +++--- tests/ui/match_single_binding.fixed | 79 +++++++++ tests/ui/match_single_binding.rs | 81 +++++++++ tests/ui/match_single_binding.stderr | 158 +++++++++++++++++- 4 files changed, 348 insertions(+), 24 deletions(-) diff --git a/clippy_lints/src/matches/match_single_binding.rs b/clippy_lints/src/matches/match_single_binding.rs index 82d5310663ee..449f5dabe23f 100644 --- a/clippy_lints/src/matches/match_single_binding.rs +++ b/clippy_lints/src/matches/match_single_binding.rs @@ -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,42 @@ 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(..) => 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..c2d9d168b4a1 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:358: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:367: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:377: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:391: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:398: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:403: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:408: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:413: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:420: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 From dd943fd5afc84cd653f88b6b6d39059152900df8 Mon Sep 17 00:00:00 2001 From: yanglsh Date: Sat, 11 Oct 2025 03:06:58 +0800 Subject: [PATCH 2/2] fix: `match_single_binding` misses curlies for const --- .../src/matches/match_single_binding.rs | 9 +++++++-- tests/ui/match_single_binding.stderr | 18 +++++++++--------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/matches/match_single_binding.rs b/clippy_lints/src/matches/match_single_binding.rs index 449f5dabe23f..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}; @@ -391,7 +391,12 @@ fn sugg_with_curlies<'a>( add_curlies(); } }, - Node::Expr(..) | Node::AnonConst(..) => 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(), _ => {}, } diff --git a/tests/ui/match_single_binding.stderr b/tests/ui/match_single_binding.stderr index c2d9d168b4a1..8a402dcca847 100644 --- a/tests/ui/match_single_binding.stderr +++ b/tests/ui/match_single_binding.stderr @@ -526,7 +526,7 @@ LL | | } | |_________^ help: consider using the match body instead: `b < c` error: this match could be replaced by its scrutinee and body - --> tests/ui/match_single_binding.rs:358:9 + --> tests/ui/match_single_binding.rs:357:9 | LL | / match { a } { LL | | @@ -543,7 +543,7 @@ LL ~ }, | error: this match could be replaced by its scrutinee and body - --> tests/ui/match_single_binding.rs:367:9 + --> tests/ui/match_single_binding.rs:366:9 | LL | / match { a } { LL | | @@ -560,7 +560,7 @@ LL ~ }, | error: this match could be replaced by its scrutinee and body - --> tests/ui/match_single_binding.rs:377:9 + --> tests/ui/match_single_binding.rs:376:9 | LL | / match { a } { LL | | @@ -577,7 +577,7 @@ LL ~ }, | error: this match could be replaced by its scrutinee and body - --> tests/ui/match_single_binding.rs:391:9 + --> tests/ui/match_single_binding.rs:390:9 | LL | / match { a } { LL | | @@ -594,7 +594,7 @@ LL ~ }, | error: this match could be replaced by its scrutinee and body - --> tests/ui/match_single_binding.rs:398:6 + --> tests/ui/match_single_binding.rs:397:6 | LL | -match { a } { | ______^ @@ -612,7 +612,7 @@ LL ~ }; | error: this match could be replaced by its scrutinee and body - --> tests/ui/match_single_binding.rs:403:9 + --> tests/ui/match_single_binding.rs:402:9 | LL | _ = match { a } { | _________^ @@ -628,7 +628,7 @@ LL ~ 1; | error: this match could be replaced by its scrutinee and body - --> tests/ui/match_single_binding.rs:408:16 + --> tests/ui/match_single_binding.rs:407:16 | LL | if let x = match { a } { | ________________^ @@ -646,7 +646,7 @@ LL ~ } {} | error: this match could be replaced by its scrutinee and body - --> tests/ui/match_single_binding.rs:413:8 + --> tests/ui/match_single_binding.rs:412:8 | LL | if match { a } { | ________^ @@ -664,7 +664,7 @@ LL ~ } { | error: this match could be replaced by its scrutinee and body - --> tests/ui/match_single_binding.rs:420:15 + --> tests/ui/match_single_binding.rs:419:15 | LL | [1, 2, 3][match { a } { | _______________^