diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index cd1ad57ee828..a31a30036b3c 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -8,6 +8,7 @@ use clippy_utils::ty::{ use clippy_utils::{ DefinedTy, ExprUseNode, get_expr_use_site, get_parent_expr, is_block_like, is_from_proc_macro, is_lint_allowed, sym, }; +use rustc_ast::BinOpKind; use rustc_ast::util::parser::ExprPrecedence; use rustc_data_structures::fx::FxIndexMap; use rustc_errors::Applicability; @@ -1025,6 +1026,61 @@ fn ty_contains_field(ty: Ty<'_>, name: Symbol) -> bool { } } +// Returns true if the expression is in a position where replacing it with &expr would create a ref +// to a pattern binding that doesn't live long enough +fn is_in_pattern_branch_tail(tcx: TyCtxt<'_>, mut id: HirId) -> bool { + loop { + match tcx.parent_hir_node(id) { + Node::Arm(arm) => return arm.body.hir_id == id, + Node::Block(block) if block.expr.is_some_and(|e| e.hir_id == id) => { + id = block.hir_id; + }, + Node::Expr(expr) => match expr.kind { + ExprKind::Block(..) | ExprKind::DropTemps(_) => { + id = expr.hir_id; + }, + ExprKind::If(cond, then_expr, else_expr) => { + if then_expr.hir_id == id { + if contains_let_expr(cond) { + return true; + } + // Bubble up for normal if branch + id = expr.hir_id; + } else if else_expr.is_some_and(|e| e.hir_id == id) { + // Bubble up for normal else branch + id = expr.hir_id; + } else { + return false; + } + }, + + // break x or break { x } + ExprKind::Break(_, Some(break_expr)) if break_expr.hir_id == id => { + return true; + }, + + _ => return false, + }, + + _ => return false, + } + } +} + +fn contains_let_expr(cond: &Expr<'_>) -> bool { + match cond.kind { + ExprKind::Let(_) => true, + // let _ = if cond && Some(ref x) = opt { + // x + // } + ExprKind::Binary(op, lhs, rhs) if matches!(op.node, BinOpKind::And) => { + contains_let_expr(lhs) || contains_let_expr(rhs) + }, + ExprKind::DropTemps(expr) => contains_let_expr(expr), + _ => false, + } +} + impl<'tcx> Dereferencing<'tcx> { fn in_deref_impl(&self) -> bool { self.outermost_deref_impl.is_some() @@ -1065,21 +1121,33 @@ impl<'tcx> Dereferencing<'tcx> { pat.replacements.push((span, snip.into())); }, Some(parent) if !parent.span.from_expansion() => { - // Double reference might be needed at this point. - if cx.precedence(parent) == ExprPrecedence::Unambiguous { - // Parentheses would be needed here, don't lint. + // If the expression is in the tail position of a match arm, suggesting + // `&x` would create a reference to a local binding that doesn't live + // long enough. + if is_in_pattern_branch_tail(cx.tcx, e.hir_id) + // If parentheses would be needed here, don't lint. + || cx.precedence(parent) == ExprPrecedence::Unambiguous + { *outer_pat = None; } else { + // Double reference might be needed at this point. pat.always_deref = false; let snip = snippet_with_context(cx, e.span, parent.span.ctxt(), "..", &mut pat.app).0; pat.replacements.push((e.span, format!("&{snip}"))); } }, _ if !e.span.from_expansion() => { - // Double reference might be needed at this point. - pat.always_deref = false; - let snip = snippet_with_applicability(cx, e.span, "..", &mut pat.app); - pat.replacements.push((e.span, format!("&{snip}"))); + // If the expression is in the tail position of a match arm, suggesting + // `&x` would create a reference to a local binding that doesn't live + // long enough. + if is_in_pattern_branch_tail(cx.tcx, e.hir_id) { + *outer_pat = None; + } else { + // Double reference might be needed at this point. + pat.always_deref = false; + let snip = snippet_with_applicability(cx, e.span, "..", &mut pat.app); + pat.replacements.push((e.span, format!("&{snip}"))); + } }, // Edge case for macros. The span of the identifier will usually match the context of the // binding, but not if the identifier was created in a macro. e.g. `concat_idents` and proc diff --git a/tests/ui/ref_binding_to_reference.fixed b/tests/ui/ref_binding_to_reference.fixed index 0a16bed4a43b..47efe8d9cb24 100644 --- a/tests/ui/ref_binding_to_reference.fixed +++ b/tests/ui/ref_binding_to_reference.fixed @@ -1,5 +1,5 @@ #![warn(clippy::ref_binding_to_reference)] -#![expect(clippy::explicit_auto_deref)] +#![expect(clippy::collapsible_match, clippy::explicit_auto_deref, clippy::never_loop)] #![allow(clippy::needless_borrowed_reference)] fn f1(_: &str) {} @@ -23,21 +23,11 @@ fn main() { None => return, }; - // Err, reference to a &String - #[expect( - clippy::ref_binding_to_reference, - reason = "The suggestion doesn't compile, see https://github.com/rust-lang/rust-clippy/issues/17370" - )] let _: &&String = match Some(&x) { Some(ref x) => x, None => return, }; - // Err, reference to a &String - #[expect( - clippy::ref_binding_to_reference, - reason = "The suggestion doesn't compile, see https://github.com/rust-lang/rust-clippy/issues/17370" - )] let _: &&String = match Some(&x) { Some(ref x) => { f1(x); @@ -89,11 +79,144 @@ impl T1 for S { } } -fn check_expect_suppression() { - let x = String::new(); - #[expect(clippy::ref_binding_to_reference)] - let _: &&String = match Some(&x) { - Some(ref x) => x, - None => return, - }; +mod issue17370 { + fn f1(_: &str) {} + + fn match_ref_some() { + let x = String::new(); + let _: &&String = match Some(&x) { + Some(ref x) => x, + None => return, + }; + } + + fn match_ref(x: String) { + let _: &&String = match &x { + ref x => x, + _ => return, + }; + } + + fn if_let(x: String) { + let opt = Some(&x); + + let _: &&String = if let Some(ref x) = opt { + x + } else { + return; + }; + } + + fn if_let_block_tail(x: String) { + let opt = Some(&x); + + let _: &&String = if let Some(ref x) = opt { + f1(x); + f1(*x); + x + } else { + return; + }; + } + + fn normal_if_tail(x: String) { + let _: &&String = match Some(&x) { + Some(ref x) => { + if true { + x + } else { + return; + } + }, + None => return, + }; + } + + fn normal_if_block_tail(x: String) { + let _: &&String = match Some(&x) { + Some(ref x) => { + if true { + f1(x); + f1(*x); + x + } else { + return; + } + }, + None => return, + }; + } + + fn normal_else_tail(x: String) { + let _: &&String = match Some(&x) { + Some(ref x) => { + if true { + return; + } else { + x + } + }, + None => return, + }; + } + + fn normal_else_block_tail(x: String) { + let _: &&String = match Some(&x) { + Some(ref x) => { + if true { + return; + } else { + f1(x); + f1(*x); + x + } + }, + None => return, + }; + } + + fn break_if_let(x: String) { + let opt = Some(&x); + + let _: &&String = loop { + if let Some(ref x) = opt { + break x; + } else { + return; + } + }; + } + + fn break_match(x: String) { + let opt = Some(&x); + + let _: &&String = loop { + match opt { + Some(ref x) => break x, + None => return, + } + }; + } + + fn if_let_chain(x: String) { + let opt = Some(&x); + + let _: &&String = if true && let Some(ref x) = opt { + x + } else { + return; + }; + } + + fn if_let_chain_block_tail(x: String) { + let opt = Some(&x); + + let _: &&String = if true && let Some(ref x) = opt { + f1(x); + f1(*x); + x + } else { + return; + }; + } } diff --git a/tests/ui/ref_binding_to_reference.rs b/tests/ui/ref_binding_to_reference.rs index 04d8c9e95cf3..4b7222a692a2 100644 --- a/tests/ui/ref_binding_to_reference.rs +++ b/tests/ui/ref_binding_to_reference.rs @@ -1,5 +1,5 @@ #![warn(clippy::ref_binding_to_reference)] -#![expect(clippy::explicit_auto_deref)] +#![expect(clippy::collapsible_match, clippy::explicit_auto_deref, clippy::never_loop)] #![allow(clippy::needless_borrowed_reference)] fn f1(_: &str) {} @@ -23,21 +23,11 @@ fn main() { None => return, }; - // Err, reference to a &String - #[expect( - clippy::ref_binding_to_reference, - reason = "The suggestion doesn't compile, see https://github.com/rust-lang/rust-clippy/issues/17370" - )] let _: &&String = match Some(&x) { Some(ref x) => x, None => return, }; - // Err, reference to a &String - #[expect( - clippy::ref_binding_to_reference, - reason = "The suggestion doesn't compile, see https://github.com/rust-lang/rust-clippy/issues/17370" - )] let _: &&String = match Some(&x) { Some(ref x) => { f1(x); @@ -89,11 +79,144 @@ impl T1 for S { } } -fn check_expect_suppression() { - let x = String::new(); - #[expect(clippy::ref_binding_to_reference)] - let _: &&String = match Some(&x) { - Some(ref x) => x, - None => return, - }; +mod issue17370 { + fn f1(_: &str) {} + + fn match_ref_some() { + let x = String::new(); + let _: &&String = match Some(&x) { + Some(ref x) => x, + None => return, + }; + } + + fn match_ref(x: String) { + let _: &&String = match &x { + ref x => x, + _ => return, + }; + } + + fn if_let(x: String) { + let opt = Some(&x); + + let _: &&String = if let Some(ref x) = opt { + x + } else { + return; + }; + } + + fn if_let_block_tail(x: String) { + let opt = Some(&x); + + let _: &&String = if let Some(ref x) = opt { + f1(x); + f1(*x); + x + } else { + return; + }; + } + + fn normal_if_tail(x: String) { + let _: &&String = match Some(&x) { + Some(ref x) => { + if true { + x + } else { + return; + } + }, + None => return, + }; + } + + fn normal_if_block_tail(x: String) { + let _: &&String = match Some(&x) { + Some(ref x) => { + if true { + f1(x); + f1(*x); + x + } else { + return; + } + }, + None => return, + }; + } + + fn normal_else_tail(x: String) { + let _: &&String = match Some(&x) { + Some(ref x) => { + if true { + return; + } else { + x + } + }, + None => return, + }; + } + + fn normal_else_block_tail(x: String) { + let _: &&String = match Some(&x) { + Some(ref x) => { + if true { + return; + } else { + f1(x); + f1(*x); + x + } + }, + None => return, + }; + } + + fn break_if_let(x: String) { + let opt = Some(&x); + + let _: &&String = loop { + if let Some(ref x) = opt { + break x; + } else { + return; + } + }; + } + + fn break_match(x: String) { + let opt = Some(&x); + + let _: &&String = loop { + match opt { + Some(ref x) => break x, + None => return, + } + }; + } + + fn if_let_chain(x: String) { + let opt = Some(&x); + + let _: &&String = if true && let Some(ref x) = opt { + x + } else { + return; + }; + } + + fn if_let_chain_block_tail(x: String) { + let opt = Some(&x); + + let _: &&String = if true && let Some(ref x) = opt { + f1(x); + f1(*x); + x + } else { + return; + }; + } } diff --git a/tests/ui/ref_binding_to_reference.stderr b/tests/ui/ref_binding_to_reference.stderr index b16cccf6f239..881010a7970e 100644 --- a/tests/ui/ref_binding_to_reference.stderr +++ b/tests/ui/ref_binding_to_reference.stderr @@ -1,5 +1,5 @@ error: this pattern creates a reference to a reference - --> tests/ui/ref_binding_to_reference.rs:52:14 + --> tests/ui/ref_binding_to_reference.rs:42:14 | LL | Some(ref x) => m2!(x), | ^^^^^ @@ -13,7 +13,7 @@ LL + Some(x) => m2!(&x), | error: this pattern creates a reference to a reference - --> tests/ui/ref_binding_to_reference.rs:58:15 + --> tests/ui/ref_binding_to_reference.rs:48:15 | LL | let _ = |&ref x: &&String| { | ^^^^^ @@ -27,7 +27,7 @@ LL ~ let _: &&String = &x; | error: this pattern creates a reference to a reference - --> tests/ui/ref_binding_to_reference.rs:66:12 + --> tests/ui/ref_binding_to_reference.rs:56:12 | LL | fn f2<'a>(&ref x: &&'a String) -> &'a String { | ^^^^^ @@ -42,7 +42,7 @@ LL ~ x | error: this pattern creates a reference to a reference - --> tests/ui/ref_binding_to_reference.rs:75:11 + --> tests/ui/ref_binding_to_reference.rs:65:11 | LL | fn f(&ref x: &&String) { | ^^^^^ @@ -56,7 +56,7 @@ LL ~ let _: &&String = &x; | error: this pattern creates a reference to a reference - --> tests/ui/ref_binding_to_reference.rs:85:11 + --> tests/ui/ref_binding_to_reference.rs:75:11 | LL | fn f(&ref x: &&String) { | ^^^^^