From 209e4d7d853950574dab187d360d71a479ce2402 Mon Sep 17 00:00:00 2001 From: Linshu Yang Date: Sat, 3 Jan 2026 02:40:38 +0000 Subject: [PATCH] fix: `question_mark` wrongly unmangled macros --- clippy_lints/src/question_mark.rs | 7 ++++--- tests/ui/question_mark.fixed | 17 ++++++++++++++++- tests/ui/question_mark.rs | 22 +++++++++++++++++++++- tests/ui/question_mark.stderr | 19 ++++++++++++++++++- 4 files changed, 59 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs index 59d31f782bc3..e5fb3c0fa431 100644 --- a/clippy_lints/src/question_mark.rs +++ b/clippy_lints/src/question_mark.rs @@ -5,7 +5,7 @@ use clippy_config::types::MatchLintBehaviour; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::res::{MaybeDef, MaybeQPath, MaybeResPath}; -use clippy_utils::source::snippet_with_applicability; +use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; use clippy_utils::sugg::Sugg; use clippy_utils::ty::{implements_trait, is_copy}; use clippy_utils::usage::local_used_after_expr; @@ -147,7 +147,8 @@ fn check_let_some_else_return_none(cx: &LateContext<'_>, stmt: &Stmt<'_>) { && !span_contains_cfg(cx, els.span) { let mut applicability = Applicability::MaybeIncorrect; - let init_expr_str = Sugg::hir_with_applicability(cx, init_expr, "..", &mut applicability).maybe_paren(); + let init_expr_str = + Sugg::hir_with_context(cx, init_expr, stmt.span.ctxt(), "..", &mut applicability).maybe_paren(); // Take care when binding is `ref` let sugg = if let PatKind::Binding( BindingMode(ByRef::Yes(_, ref_mutability), binding_mutability), @@ -295,7 +296,7 @@ fn check_is_none_or_err_and_early_return<'tcx>(cx: &LateContext<'tcx>, expr: &Ex && (is_early_return(sym::Option, cx, &if_block) || is_early_return(sym::Result, cx, &if_block)) { let mut applicability = Applicability::MachineApplicable; - let receiver_str = snippet_with_applicability(cx, caller.span, "..", &mut applicability); + let receiver_str = snippet_with_context(cx, caller.span, expr.span.ctxt(), "..", &mut applicability).0; let by_ref = !cx.type_is_copy_modulo_regions(caller_ty) && !matches!(caller.kind, ExprKind::Call(..) | ExprKind::MethodCall(..)); let sugg = if let Some(else_inner) = r#else { diff --git a/tests/ui/question_mark.fixed b/tests/ui/question_mark.fixed index 2c5ee0245038..b8072932c4ea 100644 --- a/tests/ui/question_mark.fixed +++ b/tests/ui/question_mark.fixed @@ -1,5 +1,5 @@ #![feature(try_blocks)] -#![allow(clippy::unnecessary_wraps)] +#![allow(clippy::unnecessary_wraps, clippy::no_effect)] use std::sync::MutexGuard; @@ -500,3 +500,18 @@ mod issue14894 { Ok(()) } } + +fn wrongly_unmangled_macros() -> Option { + macro_rules! test_expr { + ($val:expr) => { + Some($val) + }; + } + + let x = test_expr!(42)?; + //~^^^ question_mark + Some(x); + + test_expr!(42)?; + test_expr!(42) +} diff --git a/tests/ui/question_mark.rs b/tests/ui/question_mark.rs index b9ff9d1565b2..b320dcd4b0bc 100644 --- a/tests/ui/question_mark.rs +++ b/tests/ui/question_mark.rs @@ -1,5 +1,5 @@ #![feature(try_blocks)] -#![allow(clippy::unnecessary_wraps)] +#![allow(clippy::unnecessary_wraps, clippy::no_effect)] use std::sync::MutexGuard; @@ -615,3 +615,23 @@ mod issue14894 { Ok(()) } } + +fn wrongly_unmangled_macros() -> Option { + macro_rules! test_expr { + ($val:expr) => { + Some($val) + }; + } + + let Some(x) = test_expr!(42) else { + return None; + }; + //~^^^ question_mark + Some(x); + + if test_expr!(42).is_none() { + //~^ question_mark + return None; + } + test_expr!(42) +} diff --git a/tests/ui/question_mark.stderr b/tests/ui/question_mark.stderr index 9b2896328e66..d645c8830adc 100644 --- a/tests/ui/question_mark.stderr +++ b/tests/ui/question_mark.stderr @@ -333,5 +333,22 @@ LL | | return Err(reason); LL | | } | |_________^ help: replace it with: `result?;` -error: aborting due to 35 previous errors +error: this `let...else` may be rewritten with the `?` operator + --> tests/ui/question_mark.rs:626:5 + | +LL | / let Some(x) = test_expr!(42) else { +LL | | return None; +LL | | }; + | |______^ help: replace it with: `let x = test_expr!(42)?;` + +error: this block may be rewritten with the `?` operator + --> tests/ui/question_mark.rs:632:5 + | +LL | / if test_expr!(42).is_none() { +LL | | +LL | | return None; +LL | | } + | |_____^ help: replace it with: `test_expr!(42)?;` + +error: aborting due to 37 previous errors