From f0010d1a25862cff215f525e6a5794a88691d08f Mon Sep 17 00:00:00 2001 From: Gri-ffin Date: Wed, 13 May 2026 21:05:58 +0100 Subject: [PATCH] extend needless_borrowed_reference to lint mutable ref patterns add inference test --- clippy_lints/src/needless_borrowed_ref.rs | 73 ++++++++++++++++++++++- tests/ui/needless_borrowed_ref.fixed | 44 ++++++++++++++ tests/ui/needless_borrowed_ref.rs | 44 ++++++++++++++ tests/ui/needless_borrowed_ref.stderr | 26 +++++++- 4 files changed, 185 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/needless_borrowed_ref.rs b/clippy_lints/src/needless_borrowed_ref.rs index dad198a90342..f267450470d2 100644 --- a/clippy_lints/src/needless_borrowed_ref.rs +++ b/clippy_lints/src/needless_borrowed_ref.rs @@ -1,6 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::usage::local_used_after_expr; use rustc_errors::Applicability; -use rustc_hir::{BindingMode, Mutability, Node, Pat, PatKind, Pinnedness}; +use rustc_hir::{self as hir, BindingMode, ExprKind, Mutability, Node, Pat, PatKind, Pinnedness, QPath}; use rustc_lint::{LateContext, LateLintPass, declare_lint_pass}; declare_clippy_lint! { @@ -104,7 +105,77 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrowedRef { _ => {}, } } + + // Check for `&mut ref mut x` patterns where the `&mut` can be removed + if let PatKind::Ref(pat, Pinnedness::Not, Mutability::Mut) = ref_pat.kind + && !ref_pat.span.from_expansion() + && let PatKind::Binding(BindingMode::REF_MUT, _, ident, None) = pat.kind + && cx + .tcx + .hir_parent_iter(ref_pat.hir_id) + .map_while(|(_, parent)| if let Node::Pat(pat) = parent { Some(pat) } else { None }) + // Do not lint if behind another ref pattern (can't move out of a reference) + // or part of an OR `|` pattern + .all(|pat| !matches!(pat.kind, PatKind::Or(_) | PatKind::Ref(..))) + && can_move_ref(cx, ref_pat) + { + span_lint_and_then( + cx, + NEEDLESS_BORROWED_REFERENCE, + ref_pat.span, + "this pattern takes a mutable reference on something that is being dereferenced", + |diag| { + let span = ref_pat.span.until(ident.span); + diag.span_suggestion_verbose( + span, + "try removing the `&mut ref mut` part", + String::new(), + Applicability::MachineApplicable, + ); + }, + ); + } + } +} + +/// Checks whether the `&mut` reference matched by `ref_pat` can be moved (rather than reborrowed). +/// This is only valid when the scrutinee is a simple local variable that is not used after the +/// enclosing match/if let expression. +fn can_move_ref(cx: &LateContext<'_>, ref_pat: &Pat<'_>) -> bool { + // Walk up past patterns and arms to find the enclosing match/if let expression + let mut scrutinee_and_expr = None; + for (_, parent) in cx.tcx.hir_parent_iter(ref_pat.hir_id) { + if matches!(parent, Node::Pat(_) | Node::PatField(_) | Node::Arm(_)) { + // skip intermediate pattern/arm nodes + } else if let Node::Expr(expr) = parent { + match expr.kind { + ExprKind::Match(scrutinee, _, _) => { + scrutinee_and_expr = Some((scrutinee, expr)); + break; + }, + // the pattern's direct parent is the `Let` expression; + // keep going one level up to the `If` for the right "used after" scope. + ExprKind::Let(let_expr) => { + scrutinee_and_expr = Some((let_expr.init, expr)); + }, + ExprKind::If(..) if scrutinee_and_expr.is_some() => { + scrutinee_and_expr = scrutinee_and_expr.map(|(s, _)| (s, expr)); + break; + }, + _ => break, + } + } else { + break; + } + } + + if let Some((scrutinee, scope_expr)) = scrutinee_and_expr + && let ExprKind::Path(QPath::Resolved(None, path)) = scrutinee.kind + && let hir::def::Res::Local(local_id) = path.res + { + return !local_used_after_expr(cx, local_id, scope_expr); } + false } fn check_subpatterns<'tcx>( diff --git a/tests/ui/needless_borrowed_ref.fixed b/tests/ui/needless_borrowed_ref.fixed index 0db5ae5dbe54..296ad69a04bc 100644 --- a/tests/ui/needless_borrowed_ref.fixed +++ b/tests/ui/needless_borrowed_ref.fixed @@ -123,6 +123,50 @@ fn should_not_lint( } } +fn should_lint_mut_ref(x: Option<&mut i32>) { + if let Some(x) = x { + //~^ needless_borrowed_reference + *x = 0; + } +} + +fn should_not_lint_mut_ref_nested(x: Option<&mut &mut i32>) { + if let Some(&mut &mut ref mut x) = x { + *x = 0; + } +} + +fn should_not_lint_mut_ref_used_after(x: Option<&mut i32>) -> Option<&mut i32> { + if let Some(&mut ref mut x) = x { + *x = 0; + } + x +} + +struct WithDrop<'a>(Option<&'a mut i32>); +impl Drop for WithDrop<'_> { + fn drop(&mut self) {} +} + +fn should_not_lint_mut_ref_drop(x: WithDrop<'_>) { + if let Some(&mut ref mut x) = x.0 { + *x = 0; + } +} + +fn identity(value: T) -> T { + value +} + +fn inferred_scrutinee(reference: &mut i32) { + let val = identity(Some(reference)); + + if let Some(x) = val { + //~^ needless_borrowed_reference + *x = 0; + } +} + enum Animal { Cat(u64), Dog(u64), diff --git a/tests/ui/needless_borrowed_ref.rs b/tests/ui/needless_borrowed_ref.rs index 95dcd1bbc731..c90541a31414 100644 --- a/tests/ui/needless_borrowed_ref.rs +++ b/tests/ui/needless_borrowed_ref.rs @@ -123,6 +123,50 @@ fn should_not_lint( } } +fn should_lint_mut_ref(x: Option<&mut i32>) { + if let Some(&mut ref mut x) = x { + //~^ needless_borrowed_reference + *x = 0; + } +} + +fn should_not_lint_mut_ref_nested(x: Option<&mut &mut i32>) { + if let Some(&mut &mut ref mut x) = x { + *x = 0; + } +} + +fn should_not_lint_mut_ref_used_after(x: Option<&mut i32>) -> Option<&mut i32> { + if let Some(&mut ref mut x) = x { + *x = 0; + } + x +} + +struct WithDrop<'a>(Option<&'a mut i32>); +impl Drop for WithDrop<'_> { + fn drop(&mut self) {} +} + +fn should_not_lint_mut_ref_drop(x: WithDrop<'_>) { + if let Some(&mut ref mut x) = x.0 { + *x = 0; + } +} + +fn identity(value: T) -> T { + value +} + +fn inferred_scrutinee(reference: &mut i32) { + let val = identity(Some(reference)); + + if let Some(&mut ref mut x) = val { + //~^ needless_borrowed_reference + *x = 0; + } +} + enum Animal { Cat(u64), Dog(u64), diff --git a/tests/ui/needless_borrowed_ref.stderr b/tests/ui/needless_borrowed_ref.stderr index 980022715628..6961bc4eb6be 100644 --- a/tests/ui/needless_borrowed_ref.stderr +++ b/tests/ui/needless_borrowed_ref.stderr @@ -213,5 +213,29 @@ LL - if let &Struct { ref a, b: _, .. } = &s {} LL + if let Struct { a, b: _, .. } = &s {} | -error: aborting due to 17 previous errors +error: this pattern takes a mutable reference on something that is being dereferenced + --> tests/ui/needless_borrowed_ref.rs:127:17 + | +LL | if let Some(&mut ref mut x) = x { + | ^^^^^^^^^^^^^^ + | +help: try removing the `&mut ref mut` part + | +LL - if let Some(&mut ref mut x) = x { +LL + if let Some(x) = x { + | + +error: this pattern takes a mutable reference on something that is being dereferenced + --> tests/ui/needless_borrowed_ref.rs:164:17 + | +LL | if let Some(&mut ref mut x) = val { + | ^^^^^^^^^^^^^^ + | +help: try removing the `&mut ref mut` part + | +LL - if let Some(&mut ref mut x) = val { +LL + if let Some(x) = val { + | + +error: aborting due to 19 previous errors