Skip to content

Commit 55b5409

Browse files
authored
Fix wrong question_mark suggestion when match arm body is a destructuring assignment (#16863)
* Added `&& sugg.starts_with('{')` to the block-insertion branch. When the snippet doesn't start with `{` (i.e., destructuring assignment), it falls through to the else branch which correctly wraps the suggestion in an explicit block. * Added a new test case `issue_destructuring_assignment` in `tests/ui/question_mark.rs` to verify that the lint provides correct suggestion for match expressions with destructuring assignments that can be replaced by the `?` operator. fixes #16862 changelog: [`question_mark`]: Fix wrong suggestion when match arm body is a destructuring assignment
2 parents dce4ae9 + e3fa708 commit 55b5409

4 files changed

Lines changed: 43 additions & 2 deletions

File tree

clippy_lints/src/question_mark.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ fn check_if_try_match<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
495495
let mut sugg = reindent_multiline(&arm_body_snippet, true, Some(indent));
496496
let binding_snippet = snippet_with_applicability(cx, binding_span, "..", &mut applicability);
497497
let inner_indent = " ".repeat(indent + 4);
498-
if matches!(arm_body.kind, ExprKind::Block(..)) {
498+
if matches!(arm_body.kind, ExprKind::Block(..)) && sugg.starts_with('{') {
499499
sugg.insert_str(
500500
1,
501501
&format!("\n{inner_indent}let {binding_snippet} = {scrutinee_snippet}?;"),

tests/ui/question_mark.fixed

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,3 +564,14 @@ fn issue16751(mut v: Option<usize>) -> Option<usize> {
564564
if n > 10 { Some(42) } else { None }
565565
}
566566
}
567+
568+
fn issue_destructuring_assignment() -> Option<(i32, i32)> {
569+
let mut a = 0i32;
570+
let mut b = 0i32;
571+
let opt: Option<(i32, i32)> = Some((1, 2));
572+
{
573+
let x = opt?;
574+
(a, b) = x
575+
}
576+
Some((a, b))
577+
}

tests/ui/question_mark.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -710,3 +710,15 @@ fn issue16751(mut v: Option<usize>) -> Option<usize> {
710710
None => return None,
711711
}
712712
}
713+
714+
fn issue_destructuring_assignment() -> Option<(i32, i32)> {
715+
let mut a = 0i32;
716+
let mut b = 0i32;
717+
let opt: Option<(i32, i32)> = Some((1, 2));
718+
match opt {
719+
//~^ question_mark
720+
Some(x) => (a, b) = x,
721+
None => return None,
722+
}
723+
Some((a, b))
724+
}

tests/ui/question_mark.stderr

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -498,5 +498,23 @@ LL + if n > 10 { Some(42) } else { None }
498498
LL + }
499499
|
500500

501-
error: aborting due to 46 previous errors
501+
error: this `match` expression can be replaced with `?`
502+
--> tests/ui/question_mark.rs:718:5
503+
|
504+
LL | / match opt {
505+
LL | |
506+
LL | | Some(x) => (a, b) = x,
507+
LL | | None => return None,
508+
LL | | }
509+
| |_____^
510+
|
511+
help: try instead
512+
|
513+
LL ~ {
514+
LL + let x = opt?;
515+
LL + (a, b) = x
516+
LL + }
517+
|
518+
519+
error: aborting due to 47 previous errors
502520

0 commit comments

Comments
 (0)