Skip to content

Commit

Permalink
Auto merge of rust-lang#6941 - ThibsG:suggMatchSingleBinding, r=llogiq
Browse files Browse the repository at this point in the history
Fix bad suggestion for `match_single_binding` lint

Fix a bad suggestion that needs curly braces when the target `match` is the body of an arm.

Fixes rust-lang#6572

changelog: none
  • Loading branch information
bors committed Mar 20, 2021
2 parents 478f258 + 7d45d8a commit 0bdaa77
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 1 deletion.
14 changes: 13 additions & 1 deletion clippy_lints/src/matches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1353,6 +1353,7 @@ fn find_bool_lit(ex: &ExprKind<'_>, desugared: bool) -> Option<bool> {
}
}

#[allow(clippy::too_many_lines)]
fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], expr: &Expr<'_>) {
if in_macro(expr.span) || arms.len() != 1 || is_refutable(cx, arms[0].pat) {
return;
Expand Down Expand Up @@ -1427,7 +1428,18 @@ fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[A
indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
cbrace_start = format!("{{\n{}", indent);
}
};
}
// If the parent is already an arm, and the body is another match statement,
// we need curly braces around suggestion
let parent_node_id = cx.tcx.hir().get_parent_node(expr.hir_id);
if let Node::Arm(arm) = &cx.tcx.hir().get(parent_node_id) {
if let ExprKind::Match(..) = arm.body.kind {
cbrace_end = format!("\n{}}}", indent);
// Fix body indent due to the match
indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
cbrace_start = format!("{{\n{}", indent);
}
}
(
expr.span,
format!(
Expand Down
37 changes: 37 additions & 0 deletions tests/ui/match_single_binding2.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// run-rustfix

#![warn(clippy::match_single_binding)]
#![allow(unused_variables)]

fn main() {
// Lint (additional curly braces needed, see #6572)
struct AppendIter<I>
where
I: Iterator,
{
inner: Option<(I, <I as Iterator>::Item)>,
}

#[allow(dead_code)]
fn size_hint<I: Iterator>(iter: &AppendIter<I>) -> (usize, Option<usize>) {
match &iter.inner {
Some((iter, _item)) => {
let (min, max) = iter.size_hint();
(min.saturating_add(1), max.and_then(|max| max.checked_add(1)))
},
None => (0, Some(0)),
}
}

// Lint (no additional curly braces needed)
let opt = Some((5, 2));
let get_tup = || -> (i32, i32) { (1, 2) };
match opt {
#[rustfmt::skip]
Some((first, _second)) => {
let (a, b) = get_tup();
println!("a {:?} and b {:?}", a, b);
},
None => println!("nothing"),
}
}
37 changes: 37 additions & 0 deletions tests/ui/match_single_binding2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// run-rustfix

#![warn(clippy::match_single_binding)]
#![allow(unused_variables)]

fn main() {
// Lint (additional curly braces needed, see #6572)
struct AppendIter<I>
where
I: Iterator,
{
inner: Option<(I, <I as Iterator>::Item)>,
}

#[allow(dead_code)]
fn size_hint<I: Iterator>(iter: &AppendIter<I>) -> (usize, Option<usize>) {
match &iter.inner {
Some((iter, _item)) => match iter.size_hint() {
(min, max) => (min.saturating_add(1), max.and_then(|max| max.checked_add(1))),
},
None => (0, Some(0)),
}
}

// Lint (no additional curly braces needed)
let opt = Some((5, 2));
let get_tup = || -> (i32, i32) { (1, 2) };
match opt {
#[rustfmt::skip]
Some((first, _second)) => {
match get_tup() {
(a, b) => println!("a {:?} and b {:?}", a, b),
}
},
None => println!("nothing"),
}
}
34 changes: 34 additions & 0 deletions tests/ui/match_single_binding2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
error: this match could be written as a `let` statement
--> $DIR/match_single_binding2.rs:18:36
|
LL | Some((iter, _item)) => match iter.size_hint() {
| ____________________________________^
LL | | (min, max) => (min.saturating_add(1), max.and_then(|max| max.checked_add(1))),
LL | | },
| |_____________^
|
= note: `-D clippy::match-single-binding` implied by `-D warnings`
help: consider using `let` statement
|
LL | Some((iter, _item)) => {
LL | let (min, max) = iter.size_hint();
LL | (min.saturating_add(1), max.and_then(|max| max.checked_add(1)))
LL | },
|

error: this match could be written as a `let` statement
--> $DIR/match_single_binding2.rs:31:13
|
LL | / match get_tup() {
LL | | (a, b) => println!("a {:?} and b {:?}", a, b),
LL | | }
| |_____________^
|
help: consider using `let` statement
|
LL | let (a, b) = get_tup();
LL | println!("a {:?} and b {:?}", a, b);
|

error: aborting due to 2 previous errors

0 comments on commit 0bdaa77

Please sign in to comment.