From 23b16998c35674e74511bfe70a7324ca12119976 Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Thu, 6 Oct 2022 15:14:55 -0400 Subject: [PATCH 1/2] Add curlies to scrutinees with side effects --- .../src/matches/match_single_binding.rs | 31 +++++++++++++------ tests/ui/match_single_binding.fixed | 9 ++++++ tests/ui/match_single_binding.rs | 8 +++++ tests/ui/match_single_binding.stderr | 19 +++++++++++- 4 files changed, 57 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/matches/match_single_binding.rs b/clippy_lints/src/matches/match_single_binding.rs index 68682cedf..b64c45887 100644 --- a/clippy_lints/src/matches/match_single_binding.rs +++ b/clippy_lints/src/matches/match_single_binding.rs @@ -58,6 +58,7 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e &snippet_body, &mut applicability, Some(span), + false, ); span_lint_and_sugg( @@ -90,6 +91,7 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e &snippet_body, &mut applicability, None, + false, ); (expr.span, sugg) }, @@ -107,10 +109,14 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e }, PatKind::Wild => { if ex.can_have_side_effects() { - let indent = " ".repeat(indent_of(cx, expr.span).unwrap_or(0)); - let sugg = format!( - "{};\n{indent}{snippet_body}", - snippet_with_applicability(cx, ex.span, "..", &mut applicability) + let sugg = sugg_with_curlies( + cx, + (ex, expr), + (bind_names, matched_vars), + &snippet_body, + &mut applicability, + None, + true, ); span_lint_and_sugg( @@ -169,6 +175,7 @@ fn sugg_with_curlies<'a>( snippet_body: &str, applicability: &mut Applicability, assignment: Option, + needs_var_binding: bool, ) -> String { let mut indent = " ".repeat(indent_of(cx, ex.span).unwrap_or(0)); @@ -200,9 +207,15 @@ fn sugg_with_curlies<'a>( s }); - format!( - "{cbrace_start}let {} = {};\n{indent}{assignment_str}{snippet_body}{cbrace_end}", - snippet_with_applicability(cx, bind_names, "..", applicability), - snippet_with_applicability(cx, matched_vars, "..", applicability) - ) + let scrutinee = if needs_var_binding { + snippet_with_applicability(cx, matched_vars, "..", applicability).to_string() + } else { + format!( + "let {} = {}", + snippet_with_applicability(cx, bind_names, "..", applicability), + snippet_with_applicability(cx, matched_vars, "..", applicability) + ) + }; + + 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 951f552eb..a6e315e47 100644 --- a/tests/ui/match_single_binding.fixed +++ b/tests/ui/match_single_binding.fixed @@ -124,3 +124,12 @@ fn issue_8723() { let _ = val; } + +#[allow(dead_code)] +fn issue_9575() { + fn side_effects() {} + let _ = || { + side_effects(); + println!("Needs curlies"); + }; +} diff --git a/tests/ui/match_single_binding.rs b/tests/ui/match_single_binding.rs index 19c0fee8f..cecbd703e 100644 --- a/tests/ui/match_single_binding.rs +++ b/tests/ui/match_single_binding.rs @@ -140,3 +140,11 @@ fn issue_8723() { let _ = val; } + +#[allow(dead_code)] +fn issue_9575() { + fn side_effects() {} + let _ = || match side_effects() { + _ => println!("Needs curlies"), + }; +} diff --git a/tests/ui/match_single_binding.stderr b/tests/ui/match_single_binding.stderr index 5d4e7314b..2b9ec7ee7 100644 --- a/tests/ui/match_single_binding.stderr +++ b/tests/ui/match_single_binding.stderr @@ -196,5 +196,22 @@ LL + suf LL ~ }; | -error: aborting due to 13 previous errors +error: this match could be replaced by its scrutinee and body + --> $DIR/match_single_binding.rs:147:16 + | +LL | let _ = || match side_effects() { + | ________________^ +LL | | _ => println!("Needs curlies"), +LL | | }; + | |_____^ + | +help: consider using the scrutinee and body instead + | +LL ~ let _ = || { +LL + side_effects(); +LL + println!("Needs curlies"); +LL ~ }; + | + +error: aborting due to 14 previous errors From 39164acf6e62a3ff55d6c210acc1095d9ccc95bb Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Thu, 6 Oct 2022 15:36:28 -0400 Subject: [PATCH 2/2] Fix flipped variable that made it through --- clippy_lints/src/matches/match_single_binding.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/matches/match_single_binding.rs b/clippy_lints/src/matches/match_single_binding.rs index b64c45887..1bf8d4e96 100644 --- a/clippy_lints/src/matches/match_single_binding.rs +++ b/clippy_lints/src/matches/match_single_binding.rs @@ -58,7 +58,7 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e &snippet_body, &mut applicability, Some(span), - false, + true, ); span_lint_and_sugg( @@ -91,7 +91,7 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e &snippet_body, &mut applicability, None, - false, + true, ); (expr.span, sugg) }, @@ -116,7 +116,7 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e &snippet_body, &mut applicability, None, - true, + false, ); span_lint_and_sugg( @@ -208,13 +208,13 @@ fn sugg_with_curlies<'a>( }); let scrutinee = if needs_var_binding { - snippet_with_applicability(cx, matched_vars, "..", applicability).to_string() - } else { format!( "let {} = {}", snippet_with_applicability(cx, bind_names, "..", applicability), snippet_with_applicability(cx, matched_vars, "..", applicability) ) + } else { + snippet_with_applicability(cx, matched_vars, "..", applicability).to_string() }; format!("{cbrace_start}{scrutinee};\n{indent}{assignment_str}{snippet_body}{cbrace_end}")