Skip to content

Commit 26ca250

Browse files
committed
Fix false positive of borrow_deref_ref
If a reborrow is itself borrowed mutably, do not propose to replace it by the original reference.
1 parent ed143af commit 26ca250

File tree

4 files changed

+66
-4
lines changed

4 files changed

+66
-4
lines changed

clippy_lints/src/borrow_deref_ref.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ use crate::reference::DEREF_ADDROF;
22
use clippy_utils::diagnostics::span_lint_and_then;
33
use clippy_utils::source::SpanRangeExt;
44
use clippy_utils::ty::implements_trait;
5-
use clippy_utils::{get_parent_expr, is_from_proc_macro, is_lint_allowed, is_mutable};
5+
use clippy_utils::{ExprUseNode, expr_use_ctxt, get_parent_expr, is_from_proc_macro, is_lint_allowed, is_mutable};
66
use rustc_errors::Applicability;
7-
use rustc_hir::{BorrowKind, ExprKind, UnOp};
7+
use rustc_hir::{BorrowKind, Expr, ExprKind, UnOp};
88
use rustc_lint::{LateContext, LateLintPass};
99
use rustc_middle::mir::Mutability;
1010
use rustc_middle::ty;
@@ -48,7 +48,7 @@ declare_clippy_lint! {
4848
declare_lint_pass!(BorrowDerefRef => [BORROW_DEREF_REF]);
4949

5050
impl<'tcx> LateLintPass<'tcx> for BorrowDerefRef {
51-
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &rustc_hir::Expr<'tcx>) {
51+
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) {
5252
if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, addrof_target) = e.kind
5353
&& let ExprKind::Unary(UnOp::Deref, deref_target) = addrof_target.kind
5454
&& !matches!(deref_target.kind, ExprKind::Unary(UnOp::Deref, ..))
@@ -76,6 +76,9 @@ impl<'tcx> LateLintPass<'tcx> for BorrowDerefRef {
7676
&& let e_ty = cx.typeck_results().expr_ty_adjusted(e)
7777
// check if the reference is coercing to a mutable reference
7878
&& (!matches!(e_ty.kind(), ty::Ref(_, _, Mutability::Mut)) || is_mutable(cx, deref_target))
79+
// If the new borrow might be itself borrowed mutably, do not propose to use
80+
// the original reference instead.
81+
&& !potentially_bound_to_mutable_ref(cx, e)
7982
&& let Some(deref_text) = deref_target.span.get_source_text(cx)
8083
{
8184
span_lint_and_then(
@@ -110,3 +113,10 @@ impl<'tcx> LateLintPass<'tcx> for BorrowDerefRef {
110113
}
111114
}
112115
}
116+
117+
/// Checks if `expr` is used as part of a `let` statement containing a `ref mut` binding.
118+
fn potentially_bound_to_mutable_ref<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
119+
matches!(expr_use_ctxt(cx, expr).use_node(cx),
120+
ExprUseNode::LetStmt(let_stmt)
121+
if let_stmt.pat.contains_explicit_ref_binding() == Some(Mutability::Mut))
122+
}

tests/ui/borrow_deref_ref.fixed

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,3 +124,23 @@ mod issue_11346 {
124124
//~^ borrow_deref_ref
125125
}
126126
}
127+
128+
fn issue_14934() {
129+
let x: &'static str = "x";
130+
let y = "y".to_string();
131+
{
132+
#[expect(clippy::toplevel_ref_arg)]
133+
let ref mut x = &*x; // Do not lint
134+
*x = &*y;
135+
}
136+
{
137+
let mut x = x;
138+
//~^ borrow_deref_ref
139+
x = &*y;
140+
}
141+
{
142+
#[expect(clippy::toplevel_ref_arg, clippy::needless_borrow)]
143+
let ref x = x;
144+
//~^ borrow_deref_ref
145+
}
146+
}

tests/ui/borrow_deref_ref.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,3 +124,23 @@ mod issue_11346 {
124124
//~^ borrow_deref_ref
125125
}
126126
}
127+
128+
fn issue_14934() {
129+
let x: &'static str = "x";
130+
let y = "y".to_string();
131+
{
132+
#[expect(clippy::toplevel_ref_arg)]
133+
let ref mut x = &*x; // Do not lint
134+
*x = &*y;
135+
}
136+
{
137+
let mut x = &*x;
138+
//~^ borrow_deref_ref
139+
x = &*y;
140+
}
141+
{
142+
#[expect(clippy::toplevel_ref_arg, clippy::needless_borrow)]
143+
let ref x = &*x;
144+
//~^ borrow_deref_ref
145+
}
146+
}

tests/ui/borrow_deref_ref.stderr

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,17 @@ error: deref on an immutable reference
2525
LL | (&*s).foo();
2626
| ^^^^^ help: if you would like to reborrow, try removing `&*`: `s`
2727

28-
error: aborting due to 4 previous errors
28+
error: deref on an immutable reference
29+
--> tests/ui/borrow_deref_ref.rs:137:21
30+
|
31+
LL | let mut x = &*x;
32+
| ^^^ help: if you would like to reborrow, try removing `&*`: `x`
33+
34+
error: deref on an immutable reference
35+
--> tests/ui/borrow_deref_ref.rs:143:21
36+
|
37+
LL | let ref x = &*x;
38+
| ^^^ help: if you would like to reborrow, try removing `&*`: `x`
39+
40+
error: aborting due to 6 previous errors
2941

0 commit comments

Comments
 (0)