Skip to content

Commit 35026c7

Browse files
authored
Fix false positive of borrow_deref_ref (#14967)
If a reborrow is itself borrowed mutably, do not propose to replace it by the original reference. Fixes: #14934 changelog: [`borrow_deref_ref`]: do not propose replacing a reborrow by the original reference if the reborrow is itself mutably borrowed
2 parents 37cb834 + 322e139 commit 35026c7

File tree

4 files changed

+137
-4
lines changed

4 files changed

+137
-4
lines changed

clippy_lints/src/borrow_deref_ref.rs

Lines changed: 12 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::{get_parent_expr, is_expr_temporary_value, 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, Node, 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 and the original reference is not a temporary
80+
// value, do not propose to use it directly.
81+
&& (is_expr_temporary_value(cx, deref_target) || !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,9 @@ 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!(cx.tcx.parent_hir_node(expr.hir_id), Node::LetStmt(let_stmt)
120+
if let_stmt.pat.contains_explicit_ref_binding() == Some(Mutability::Mut))
121+
}

tests/ui/borrow_deref_ref.fixed

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,3 +124,50 @@ 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+
{
147+
#[expect(clippy::toplevel_ref_arg)]
148+
let ref mut x = std::convert::identity(x);
149+
//~^ borrow_deref_ref
150+
*x = &*y;
151+
}
152+
{
153+
#[derive(Clone)]
154+
struct S(&'static str);
155+
let s = S("foo");
156+
#[expect(clippy::toplevel_ref_arg)]
157+
let ref mut x = &*s.0; // Do not lint
158+
*x = "bar";
159+
#[expect(clippy::toplevel_ref_arg)]
160+
let ref mut x = s.clone().0;
161+
//~^ borrow_deref_ref
162+
*x = "bar";
163+
#[expect(clippy::toplevel_ref_arg)]
164+
let ref mut x = &*std::convert::identity(&s).0;
165+
*x = "bar";
166+
}
167+
{
168+
let y = &1;
169+
#[expect(clippy::toplevel_ref_arg)]
170+
let ref mut x = { y };
171+
//~^ borrow_deref_ref
172+
}
173+
}

tests/ui/borrow_deref_ref.rs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,3 +124,50 @@ 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+
{
147+
#[expect(clippy::toplevel_ref_arg)]
148+
let ref mut x = &*std::convert::identity(x);
149+
//~^ borrow_deref_ref
150+
*x = &*y;
151+
}
152+
{
153+
#[derive(Clone)]
154+
struct S(&'static str);
155+
let s = S("foo");
156+
#[expect(clippy::toplevel_ref_arg)]
157+
let ref mut x = &*s.0; // Do not lint
158+
*x = "bar";
159+
#[expect(clippy::toplevel_ref_arg)]
160+
let ref mut x = &*s.clone().0;
161+
//~^ borrow_deref_ref
162+
*x = "bar";
163+
#[expect(clippy::toplevel_ref_arg)]
164+
let ref mut x = &*std::convert::identity(&s).0;
165+
*x = "bar";
166+
}
167+
{
168+
let y = &1;
169+
#[expect(clippy::toplevel_ref_arg)]
170+
let ref mut x = { &*y };
171+
//~^ borrow_deref_ref
172+
}
173+
}

tests/ui/borrow_deref_ref.stderr

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,35 @@ 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: deref on an immutable reference
41+
--> tests/ui/borrow_deref_ref.rs:148:25
42+
|
43+
LL | let ref mut x = &*std::convert::identity(x);
44+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: if you would like to reborrow, try removing `&*`: `std::convert::identity(x)`
45+
46+
error: deref on an immutable reference
47+
--> tests/ui/borrow_deref_ref.rs:160:25
48+
|
49+
LL | let ref mut x = &*s.clone().0;
50+
| ^^^^^^^^^^^^^ help: if you would like to reborrow, try removing `&*`: `s.clone().0`
51+
52+
error: deref on an immutable reference
53+
--> tests/ui/borrow_deref_ref.rs:170:27
54+
|
55+
LL | let ref mut x = { &*y };
56+
| ^^^ help: if you would like to reborrow, try removing `&*`: `y`
57+
58+
error: aborting due to 9 previous errors
2959

0 commit comments

Comments
 (0)