From fd8bb2167640600d59ac9199a4ddeb1a2edf7f4f Mon Sep 17 00:00:00 2001 From: irelaxcn Date: Mon, 2 Mar 2026 00:34:09 +0800 Subject: [PATCH] fix: `question_mark` suggestion caused error --- clippy_lints/src/question_mark.rs | 5 +++-- tests/ui/question_mark.fixed | 13 +++++++++++++ tests/ui/question_mark.rs | 19 +++++++++++++++++++ tests/ui/question_mark.stderr | 21 ++++++++++++++++++++- 4 files changed, 55 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs index 719f364357d3..e4bb3f13aea0 100644 --- a/clippy_lints/src/question_mark.rs +++ b/clippy_lints/src/question_mark.rs @@ -501,7 +501,8 @@ fn check_if_let_some_or_err_and_early_return<'tcx>(cx: &LateContext<'tcx>, expr: let mut applicability = Applicability::MachineApplicable; let receiver_str = snippet_with_applicability(cx, let_expr.span, "..", &mut applicability); - let requires_semi = matches!(cx.tcx.parent_hir_node(expr.hir_id), Node::Stmt(_)); + let parent = cx.tcx.parent_hir_node(expr.hir_id); + let requires_semi = matches!(parent, Node::Stmt(_)) || cx.typeck_results().expr_ty(expr).is_unit(); let method_call_str = match by_ref { ByRef::Yes(_, Mutability::Mut) => ".as_mut()", ByRef::Yes(_, Mutability::Not) => ".as_ref()", @@ -512,7 +513,7 @@ fn check_if_let_some_or_err_and_early_return<'tcx>(cx: &LateContext<'tcx>, expr: "{receiver_str}{method_call_str}?{}", if requires_semi { ";" } else { "" } ); - if is_else_clause(cx.tcx, expr) { + if is_else_clause(cx.tcx, expr) || (requires_semi && !matches!(parent, Node::Stmt(_) | Node::Block(_))) { sugg = format!("{{ {sugg} }}"); } diff --git a/tests/ui/question_mark.fixed b/tests/ui/question_mark.fixed index 102517d34c61..e209da5c8258 100644 --- a/tests/ui/question_mark.fixed +++ b/tests/ui/question_mark.fixed @@ -524,3 +524,16 @@ fn issue16429(b: i32) -> Option { Some(0) } + +fn issue16654() -> Result<(), i32> { + let result = func_returning_result(); + + #[allow(clippy::collapsible_if)] + if true { + result?; + } + + _ = [{ result?; }]; + + Ok(()) +} diff --git a/tests/ui/question_mark.rs b/tests/ui/question_mark.rs index cfea1277fe76..579b51461d13 100644 --- a/tests/ui/question_mark.rs +++ b/tests/ui/question_mark.rs @@ -649,3 +649,22 @@ fn issue16429(b: i32) -> Option { Some(0) } + +fn issue16654() -> Result<(), i32> { + let result = func_returning_result(); + + #[allow(clippy::collapsible_if)] + if true { + if let Err(err) = result { + //~^ question_mark + return Err(err); + } + } + + _ = [if let Err(err) = result { + //~^ question_mark + return Err(err); + }]; + + Ok(()) +} diff --git a/tests/ui/question_mark.stderr b/tests/ui/question_mark.stderr index c243f12de040..1d7f665a2662 100644 --- a/tests/ui/question_mark.stderr +++ b/tests/ui/question_mark.stderr @@ -362,5 +362,24 @@ LL | | return None; LL | | }; | |_____^ help: replace it with: `{ a? }` -error: aborting due to 38 previous errors +error: this block may be rewritten with the `?` operator + --> tests/ui/question_mark.rs:658:9 + | +LL | / if let Err(err) = result { +LL | | +LL | | return Err(err); +LL | | } + | |_________^ help: replace it with: `result?;` + +error: this block may be rewritten with the `?` operator + --> tests/ui/question_mark.rs:664:10 + | +LL | _ = [if let Err(err) = result { + | __________^ +LL | | +LL | | return Err(err); +LL | | }]; + | |_____^ help: replace it with: `{ result?; }` + +error: aborting due to 40 previous errors