diff --git a/clippy_lints/src/borrow_deref_ref.rs b/clippy_lints/src/borrow_deref_ref.rs index 7cde007a9b66..70c9c45a60c8 100644 --- a/clippy_lints/src/borrow_deref_ref.rs +++ b/clippy_lints/src/borrow_deref_ref.rs @@ -2,9 +2,9 @@ use crate::reference::DEREF_ADDROF; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::SpanRangeExt; use clippy_utils::ty::implements_trait; -use clippy_utils::{get_parent_expr, is_from_proc_macro, is_lint_allowed, is_mutable}; +use clippy_utils::{get_parent_expr, is_expr_temporary_value, is_from_proc_macro, is_lint_allowed, is_mutable}; use rustc_errors::Applicability; -use rustc_hir::{BorrowKind, ExprKind, UnOp}; +use rustc_hir::{BorrowKind, Expr, ExprKind, Node, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::mir::Mutability; use rustc_middle::ty; @@ -48,7 +48,7 @@ declare_clippy_lint! { declare_lint_pass!(BorrowDerefRef => [BORROW_DEREF_REF]); impl<'tcx> LateLintPass<'tcx> for BorrowDerefRef { - fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &rustc_hir::Expr<'tcx>) { + fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) { if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, addrof_target) = e.kind && let ExprKind::Unary(UnOp::Deref, deref_target) = addrof_target.kind && !matches!(deref_target.kind, ExprKind::Unary(UnOp::Deref, ..)) @@ -76,6 +76,9 @@ impl<'tcx> LateLintPass<'tcx> for BorrowDerefRef { && let e_ty = cx.typeck_results().expr_ty_adjusted(e) // check if the reference is coercing to a mutable reference && (!matches!(e_ty.kind(), ty::Ref(_, _, Mutability::Mut)) || is_mutable(cx, deref_target)) + // If the new borrow might be itself borrowed mutably and the original reference is not a temporary + // value, do not propose to use it directly. + && (is_expr_temporary_value(cx, deref_target) || !potentially_bound_to_mutable_ref(cx, e)) && let Some(deref_text) = deref_target.span.get_source_text(cx) { span_lint_and_then( @@ -110,3 +113,9 @@ impl<'tcx> LateLintPass<'tcx> for BorrowDerefRef { } } } + +/// Checks if `expr` is used as part of a `let` statement containing a `ref mut` binding. +fn potentially_bound_to_mutable_ref<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool { + matches!(cx.tcx.parent_hir_node(expr.hir_id), Node::LetStmt(let_stmt) + if let_stmt.pat.contains_explicit_ref_binding() == Some(Mutability::Mut)) +} diff --git a/tests/ui/borrow_deref_ref.fixed b/tests/ui/borrow_deref_ref.fixed index 765dd75fceb9..6d06fcc3037a 100644 --- a/tests/ui/borrow_deref_ref.fixed +++ b/tests/ui/borrow_deref_ref.fixed @@ -124,3 +124,50 @@ mod issue_11346 { //~^ borrow_deref_ref } } + +fn issue_14934() { + let x: &'static str = "x"; + let y = "y".to_string(); + { + #[expect(clippy::toplevel_ref_arg)] + let ref mut x = &*x; // Do not lint + *x = &*y; + } + { + let mut x = x; + //~^ borrow_deref_ref + x = &*y; + } + { + #[expect(clippy::toplevel_ref_arg, clippy::needless_borrow)] + let ref x = x; + //~^ borrow_deref_ref + } + { + #[expect(clippy::toplevel_ref_arg)] + let ref mut x = std::convert::identity(x); + //~^ borrow_deref_ref + *x = &*y; + } + { + #[derive(Clone)] + struct S(&'static str); + let s = S("foo"); + #[expect(clippy::toplevel_ref_arg)] + let ref mut x = &*s.0; // Do not lint + *x = "bar"; + #[expect(clippy::toplevel_ref_arg)] + let ref mut x = s.clone().0; + //~^ borrow_deref_ref + *x = "bar"; + #[expect(clippy::toplevel_ref_arg)] + let ref mut x = &*std::convert::identity(&s).0; + *x = "bar"; + } + { + let y = &1; + #[expect(clippy::toplevel_ref_arg)] + let ref mut x = { y }; + //~^ borrow_deref_ref + } +} diff --git a/tests/ui/borrow_deref_ref.rs b/tests/ui/borrow_deref_ref.rs index 8ee66bfa881a..b43f4c93bf2b 100644 --- a/tests/ui/borrow_deref_ref.rs +++ b/tests/ui/borrow_deref_ref.rs @@ -124,3 +124,50 @@ mod issue_11346 { //~^ borrow_deref_ref } } + +fn issue_14934() { + let x: &'static str = "x"; + let y = "y".to_string(); + { + #[expect(clippy::toplevel_ref_arg)] + let ref mut x = &*x; // Do not lint + *x = &*y; + } + { + let mut x = &*x; + //~^ borrow_deref_ref + x = &*y; + } + { + #[expect(clippy::toplevel_ref_arg, clippy::needless_borrow)] + let ref x = &*x; + //~^ borrow_deref_ref + } + { + #[expect(clippy::toplevel_ref_arg)] + let ref mut x = &*std::convert::identity(x); + //~^ borrow_deref_ref + *x = &*y; + } + { + #[derive(Clone)] + struct S(&'static str); + let s = S("foo"); + #[expect(clippy::toplevel_ref_arg)] + let ref mut x = &*s.0; // Do not lint + *x = "bar"; + #[expect(clippy::toplevel_ref_arg)] + let ref mut x = &*s.clone().0; + //~^ borrow_deref_ref + *x = "bar"; + #[expect(clippy::toplevel_ref_arg)] + let ref mut x = &*std::convert::identity(&s).0; + *x = "bar"; + } + { + let y = &1; + #[expect(clippy::toplevel_ref_arg)] + let ref mut x = { &*y }; + //~^ borrow_deref_ref + } +} diff --git a/tests/ui/borrow_deref_ref.stderr b/tests/ui/borrow_deref_ref.stderr index 3d55da25b9b2..3a1f968b4be1 100644 --- a/tests/ui/borrow_deref_ref.stderr +++ b/tests/ui/borrow_deref_ref.stderr @@ -25,5 +25,35 @@ error: deref on an immutable reference LL | (&*s).foo(); | ^^^^^ help: if you would like to reborrow, try removing `&*`: `s` -error: aborting due to 4 previous errors +error: deref on an immutable reference + --> tests/ui/borrow_deref_ref.rs:137:21 + | +LL | let mut x = &*x; + | ^^^ help: if you would like to reborrow, try removing `&*`: `x` + +error: deref on an immutable reference + --> tests/ui/borrow_deref_ref.rs:143:21 + | +LL | let ref x = &*x; + | ^^^ help: if you would like to reborrow, try removing `&*`: `x` + +error: deref on an immutable reference + --> tests/ui/borrow_deref_ref.rs:148:25 + | +LL | let ref mut x = &*std::convert::identity(x); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: if you would like to reborrow, try removing `&*`: `std::convert::identity(x)` + +error: deref on an immutable reference + --> tests/ui/borrow_deref_ref.rs:160:25 + | +LL | let ref mut x = &*s.clone().0; + | ^^^^^^^^^^^^^ help: if you would like to reborrow, try removing `&*`: `s.clone().0` + +error: deref on an immutable reference + --> tests/ui/borrow_deref_ref.rs:170:27 + | +LL | let ref mut x = { &*y }; + | ^^^ help: if you would like to reborrow, try removing `&*`: `y` + +error: aborting due to 9 previous errors