Skip to content

Commit 6129b52

Browse files
relaxcnrelaxcn
committed
allow explicit deref or deref_mut method calls in impl of Deref or DerefMut trait
1 parent 30c73fe commit 6129b52

File tree

4 files changed

+175
-21
lines changed

4 files changed

+175
-21
lines changed

‎clippy_lints/src/dereference.rs‎

Lines changed: 72 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ use rustc_errors::Applicability;
1212
use rustc_hir::def_id::DefId;
1313
use rustc_hir::intravisit::{InferKind, Visitor, VisitorExt, walk_ty};
1414
use rustc_hir::{
15-
self as hir, AmbigArg, BindingMode, Body, BodyId, BorrowKind, Expr, ExprKind, HirId, MatchSource, Mutability, Node,
16-
Pat, PatKind, Path, QPath, TyKind, UnOp,
15+
self as hir, AmbigArg, BindingMode, Body, BodyId, BorrowKind, Expr, ExprKind, HirId, Impl, Item, ItemKind,
16+
MatchSource, Mutability, Node, Pat, PatKind, Path, QPath, TyKind, UnOp,
1717
};
1818
use rustc_lint::{LateContext, LateLintPass};
1919
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
@@ -50,6 +50,8 @@ declare_clippy_lint! {
5050
/// let _ = Foo::deref(&foo);
5151
/// let _ = <Foo as Deref>::deref(&foo);
5252
/// ```
53+
///
54+
/// This lint also excludes explicit `deref` or `deref_mut` method calls inside implementations of the `Deref` or `DerefMut` traits.
5355
#[clippy::version = "1.44.0"]
5456
pub EXPLICIT_DEREF_METHODS,
5557
pedantic,
@@ -169,6 +171,9 @@ pub struct Dereferencing<'tcx> {
169171
///
170172
/// e.g. `m!(x) | Foo::Bar(ref x)`
171173
ref_locals: FxIndexMap<HirId, Option<RefPat>>,
174+
175+
/// Whether the current context is within a `Deref` or `DerefMut` impl.
176+
in_deref_or_deref_mut_impl: bool,
172177
}
173178

174179
#[derive(Debug)]
@@ -246,7 +251,14 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
246251
// Stop processing sub expressions when a macro call is seen
247252
if expr.span.from_expansion() {
248253
if let Some((state, data)) = self.state.take() {
249-
report(cx, expr, state, data, cx.typeck_results());
254+
report(
255+
cx,
256+
expr,
257+
state,
258+
data,
259+
cx.typeck_results(),
260+
self.in_deref_or_deref_mut_impl,
261+
);
250262
}
251263
return;
252264
}
@@ -255,15 +267,22 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
255267
let Some((kind, sub_expr, skip_expr)) = try_parse_ref_op(cx.tcx, typeck, expr) else {
256268
// The whole chain of reference operations has been seen
257269
if let Some((state, data)) = self.state.take() {
258-
report(cx, expr, state, data, typeck);
270+
report(cx, expr, state, data, typeck, self.in_deref_or_deref_mut_impl);
259271
}
260272
return;
261273
};
262274
self.skip_expr = skip_expr;
263275

264276
if is_from_proc_macro(cx, expr) {
265277
if let Some((state, data)) = self.state.take() {
266-
report(cx, expr, state, data, cx.typeck_results());
278+
report(
279+
cx,
280+
expr,
281+
state,
282+
data,
283+
cx.typeck_results(),
284+
self.in_deref_or_deref_mut_impl,
285+
);
267286
}
268287
return;
269288
}
@@ -515,7 +534,14 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
515534
(Some((State::DerefedBorrow(state), data)), RefOp::AddrOf(mutability)) => {
516535
let adjusted_ty = data.adjusted_ty;
517536
let stability = state.stability;
518-
report(cx, expr, State::DerefedBorrow(state), data, typeck);
537+
report(
538+
cx,
539+
expr,
540+
State::DerefedBorrow(state),
541+
data,
542+
typeck,
543+
self.in_deref_or_deref_mut_impl,
544+
);
519545
if stability.is_deref_stable() {
520546
self.state = Some((
521547
State::Borrow { mutability },
@@ -530,7 +556,14 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
530556
let adjusted_ty = data.adjusted_ty;
531557
let stability = state.stability;
532558
let for_field_access = state.for_field_access;
533-
report(cx, expr, State::DerefedBorrow(state), data, typeck);
559+
report(
560+
cx,
561+
expr,
562+
State::DerefedBorrow(state),
563+
data,
564+
typeck,
565+
self.in_deref_or_deref_mut_impl,
566+
);
534567
if let Some(name) = for_field_access
535568
&& let sub_expr_ty = typeck.expr_ty(sub_expr)
536569
&& !ty_contains_field(sub_expr_ty, name)
@@ -602,7 +635,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
602635
));
603636
},
604637

605-
(Some((state, data)), _) => report(cx, expr, state, data, typeck),
638+
(Some((state, data)), _) => report(cx, expr, state, data, typeck, self.in_deref_or_deref_mut_impl),
606639
}
607640
}
608641

@@ -673,6 +706,31 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
673706
self.current_body = None;
674707
}
675708
}
709+
710+
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
711+
if is_deref_or_deref_mut_impl(cx, item) {
712+
self.in_deref_or_deref_mut_impl = true;
713+
}
714+
}
715+
716+
fn check_item_post(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
717+
if is_deref_or_deref_mut_impl(cx, item) {
718+
self.in_deref_or_deref_mut_impl = false;
719+
}
720+
}
721+
}
722+
723+
fn is_deref_or_deref_mut_impl(cx: &LateContext<'_>, item: &Item<'_>) -> bool {
724+
if let ItemKind::Impl(Impl {
725+
of_trait: Some(of_trait),
726+
..
727+
}) = &item.kind
728+
&& let Some(trait_id) = of_trait.trait_ref.trait_def_id()
729+
{
730+
cx.tcx.lang_items().deref_trait() == Some(trait_id) || cx.tcx.lang_items().deref_mut_trait() == Some(trait_id)
731+
} else {
732+
false
733+
}
676734
}
677735

678736
fn try_parse_ref_op<'tcx>(
@@ -938,13 +996,19 @@ fn report<'tcx>(
938996
state: State,
939997
data: StateData<'tcx>,
940998
typeck: &'tcx TypeckResults<'tcx>,
999+
in_deref_or_deref_mut_impl: bool,
9411000
) {
9421001
match state {
9431002
State::DerefMethod {
9441003
ty_changed_count,
9451004
is_ufcs,
9461005
mutbl,
9471006
} => {
1007+
// Skip when inside implementation of the `Deref` or `DerefMut` trait.
1008+
if in_deref_or_deref_mut_impl {
1009+
return;
1010+
}
1011+
9481012
let mut app = Applicability::MachineApplicable;
9491013
let (expr_str, expr_is_macro_call) =
9501014
snippet_with_context(cx, expr.span, data.first_expr.span.ctxt(), "..", &mut app);

‎tests/ui/explicit_deref_methods.fixed‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,51 @@ impl DerefMut for Aaa {
5050
}
5151
}
5252

53+
mod issue_15392 {
54+
use std::ops::{Deref, DerefMut};
55+
56+
struct Inner;
57+
58+
struct WrapperA(Inner);
59+
60+
impl Deref for WrapperA {
61+
type Target = Inner;
62+
fn deref(&self) -> &Inner {
63+
&self.0
64+
}
65+
}
66+
67+
impl DerefMut for WrapperA {
68+
fn deref_mut(&mut self) -> &mut Inner {
69+
&mut self.0
70+
}
71+
}
72+
73+
enum MyEnum {
74+
A(WrapperA),
75+
}
76+
77+
impl Deref for MyEnum {
78+
type Target = Inner;
79+
80+
fn deref(&self) -> &Inner {
81+
// forwarding is ok
82+
match self {
83+
MyEnum::A(wrap_a) => Deref::deref(wrap_a),
84+
}
85+
}
86+
}
87+
88+
impl DerefMut for MyEnum {
89+
fn deref_mut(&mut self) -> &mut Inner {
90+
// forwarding is ok
91+
match self {
92+
MyEnum::A(wrap_a) => DerefMut::deref_mut(wrap_a),
93+
}
94+
}
95+
}
96+
}
97+
5398
fn main() {
5499
let a: &mut String = &mut String::from("foo");
55100

‎tests/ui/explicit_deref_methods.rs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,51 @@ impl DerefMut for Aaa {
5050
}
5151
}
5252

53+
mod issue_15392 {
54+
use std::ops::{Deref, DerefMut};
55+
56+
struct Inner;
57+
58+
struct WrapperA(Inner);
59+
60+
impl Deref for WrapperA {
61+
type Target = Inner;
62+
fn deref(&self) -> &Inner {
63+
&self.0
64+
}
65+
}
66+
67+
impl DerefMut for WrapperA {
68+
fn deref_mut(&mut self) -> &mut Inner {
69+
&mut self.0
70+
}
71+
}
72+
73+
enum MyEnum {
74+
A(WrapperA),
75+
}
76+
77+
impl Deref for MyEnum {
78+
type Target = Inner;
79+
80+
fn deref(&self) -> &Inner {
81+
// forwarding is ok
82+
match self {
83+
MyEnum::A(wrap_a) => Deref::deref(wrap_a),
84+
}
85+
}
86+
}
87+
88+
impl DerefMut for MyEnum {
89+
fn deref_mut(&mut self) -> &mut Inner {
90+
// forwarding is ok
91+
match self {
92+
MyEnum::A(wrap_a) => DerefMut::deref_mut(wrap_a),
93+
}
94+
}
95+
}
96+
}
97+
5398
fn main() {
5499
let a: &mut String = &mut String::from("foo");
55100

‎tests/ui/explicit_deref_methods.stderr‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: explicit `deref` method call
2-
--> tests/ui/explicit_deref_methods.rs:58:19
2+
--> tests/ui/explicit_deref_methods.rs:103:19
33
|
44
LL | let b: &str = a.deref();
55
| ^^^^^^^^^ help: try: `&*a`
@@ -8,73 +8,73 @@ LL | let b: &str = a.deref();
88
= help: to override `-D warnings` add `#[allow(clippy::explicit_deref_methods)]`
99

1010
error: explicit `deref_mut` method call
11-
--> tests/ui/explicit_deref_methods.rs:61:23
11+
--> tests/ui/explicit_deref_methods.rs:106:23
1212
|
1313
LL | let b: &mut str = a.deref_mut();
1414
| ^^^^^^^^^^^^^ help: try: `&mut **a`
1515

1616
error: explicit `deref` method call
17-
--> tests/ui/explicit_deref_methods.rs:65:39
17+
--> tests/ui/explicit_deref_methods.rs:110:39
1818
|
1919
LL | let b: String = format!("{}, {}", a.deref(), a.deref());
2020
| ^^^^^^^^^ help: try: `&*a`
2121

2222
error: explicit `deref` method call
23-
--> tests/ui/explicit_deref_methods.rs:65:50
23+
--> tests/ui/explicit_deref_methods.rs:110:50
2424
|
2525
LL | let b: String = format!("{}, {}", a.deref(), a.deref());
2626
| ^^^^^^^^^ help: try: `&*a`
2727

2828
error: explicit `deref` method call
29-
--> tests/ui/explicit_deref_methods.rs:69:20
29+
--> tests/ui/explicit_deref_methods.rs:114:20
3030
|
3131
LL | println!("{}", a.deref());
3232
| ^^^^^^^^^ help: try: `&*a`
3333

3434
error: explicit `deref` method call
35-
--> tests/ui/explicit_deref_methods.rs:73:11
35+
--> tests/ui/explicit_deref_methods.rs:118:11
3636
|
3737
LL | match a.deref() {
3838
| ^^^^^^^^^ help: try: `&*a`
3939

4040
error: explicit `deref` method call
41-
--> tests/ui/explicit_deref_methods.rs:78:28
41+
--> tests/ui/explicit_deref_methods.rs:123:28
4242
|
4343
LL | let b: String = concat(a.deref());
4444
| ^^^^^^^^^ help: try: `&*a`
4545

4646
error: explicit `deref` method call
47-
--> tests/ui/explicit_deref_methods.rs:81:13
47+
--> tests/ui/explicit_deref_methods.rs:126:13
4848
|
4949
LL | let b = just_return(a).deref();
5050
| ^^^^^^^^^^^^^^^^^^^^^^ help: try: `just_return(a)`
5151

5252
error: explicit `deref` method call
53-
--> tests/ui/explicit_deref_methods.rs:84:28
53+
--> tests/ui/explicit_deref_methods.rs:129:28
5454
|
5555
LL | let b: String = concat(just_return(a).deref());
5656
| ^^^^^^^^^^^^^^^^^^^^^^ help: try: `just_return(a)`
5757

5858
error: explicit `deref` method call
59-
--> tests/ui/explicit_deref_methods.rs:124:31
59+
--> tests/ui/explicit_deref_methods.rs:169:31
6060
|
6161
LL | let b: &str = expr_deref!(a.deref());
6262
| ^^^^^^^^^ help: try: `&*a`
6363

6464
error: explicit `deref` method call
65-
--> tests/ui/explicit_deref_methods.rs:154:14
65+
--> tests/ui/explicit_deref_methods.rs:199:14
6666
|
6767
LL | let _ = &Deref::deref(&"foo");
6868
| ^^^^^^^^^^^^^^^^^^^^ help: try: `*&"foo"`
6969

7070
error: explicit `deref_mut` method call
71-
--> tests/ui/explicit_deref_methods.rs:156:14
71+
--> tests/ui/explicit_deref_methods.rs:201:14
7272
|
7373
LL | let _ = &DerefMut::deref_mut(&mut x);
7474
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&mut **&mut x`
7575

7676
error: explicit `deref_mut` method call
77-
--> tests/ui/explicit_deref_methods.rs:157:14
77+
--> tests/ui/explicit_deref_methods.rs:202:14
7878
|
7979
LL | let _ = &DerefMut::deref_mut((&mut &mut x).deref_mut());
8080
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&mut ***(&mut &mut x)`

0 commit comments

Comments
 (0)