Skip to content

Commit 681b46e

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 681b46e

File tree

4 files changed

+123
-4
lines changed

4 files changed

+123
-4
lines changed

clippy_lints/src/borrow_deref_ref.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@ 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::{
6+
ExprUseNode, expr_use_ctxt, get_parent_expr, is_expr_temporary_value, is_from_proc_macro, is_lint_allowed,
7+
is_mutable,
8+
};
69
use rustc_errors::Applicability;
7-
use rustc_hir::{BorrowKind, ExprKind, UnOp};
10+
use rustc_hir::{BorrowKind, Expr, ExprKind, UnOp};
811
use rustc_lint::{LateContext, LateLintPass};
912
use rustc_middle::mir::Mutability;
1013
use rustc_middle::ty;
@@ -48,7 +51,7 @@ declare_clippy_lint! {
4851
declare_lint_pass!(BorrowDerefRef => [BORROW_DEREF_REF]);
4952

5053
impl<'tcx> LateLintPass<'tcx> for BorrowDerefRef {
51-
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &rustc_hir::Expr<'tcx>) {
54+
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) {
5255
if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, addrof_target) = e.kind
5356
&& let ExprKind::Unary(UnOp::Deref, deref_target) = addrof_target.kind
5457
&& !matches!(deref_target.kind, ExprKind::Unary(UnOp::Deref, ..))
@@ -76,6 +79,9 @@ impl<'tcx> LateLintPass<'tcx> for BorrowDerefRef {
7679
&& let e_ty = cx.typeck_results().expr_ty_adjusted(e)
7780
// check if the reference is coercing to a mutable reference
7881
&& (!matches!(e_ty.kind(), ty::Ref(_, _, Mutability::Mut)) || is_mutable(cx, deref_target))
82+
// If the new borrow might be itself borrowed mutably and the original reference is not a temporary
83+
// value, do not propose to use it directly.
84+
&& (is_expr_temporary_value(cx, deref_target) || !potentially_bound_to_mutable_ref(cx, e))
7985
&& let Some(deref_text) = deref_target.span.get_source_text(cx)
8086
{
8187
span_lint_and_then(
@@ -110,3 +116,10 @@ impl<'tcx> LateLintPass<'tcx> for BorrowDerefRef {
110116
}
111117
}
112118
}
119+
120+
/// Checks if `expr` is used as part of a `let` statement containing a `ref mut` binding.
121+
fn potentially_bound_to_mutable_ref<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
122+
matches!(expr_use_ctxt(cx, expr).use_node(cx),
123+
ExprUseNode::LetStmt(let_stmt)
124+
if let_stmt.pat.contains_explicit_ref_binding() == Some(Mutability::Mut))
125+
}

tests/ui/borrow_deref_ref.fixed

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,3 +124,44 @@ 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+
}

tests/ui/borrow_deref_ref.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,3 +124,44 @@ 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+
}

tests/ui/borrow_deref_ref.stderr

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,29 @@ 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: aborting due to 8 previous errors
2953

0 commit comments

Comments
 (0)