Skip to content

Commit 772a3ee

Browse files
relaxcnrelaxcn
committed
allow explicit deref method call in impl of Deref trait
1 parent 30c73fe commit 772a3ee

File tree

4 files changed

+115
-21
lines changed

4 files changed

+115
-21
lines changed

‎clippy_lints/src/dereference.rs‎

Lines changed: 42 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};
@@ -169,6 +169,9 @@ pub struct Dereferencing<'tcx> {
169169
///
170170
/// e.g. `m!(x) | Foo::Bar(ref x)`
171171
ref_locals: FxIndexMap<HirId, Option<RefPat>>,
172+
173+
/// Whether the current context is within a `Deref` impl.
174+
in_deref_impl: bool,
172175
}
173176

174177
#[derive(Debug)]
@@ -246,7 +249,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
246249
// Stop processing sub expressions when a macro call is seen
247250
if expr.span.from_expansion() {
248251
if let Some((state, data)) = self.state.take() {
249-
report(cx, expr, state, data, cx.typeck_results());
252+
report(cx, expr, state, data, cx.typeck_results(), self.in_deref_impl);
250253
}
251254
return;
252255
}
@@ -255,15 +258,15 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
255258
let Some((kind, sub_expr, skip_expr)) = try_parse_ref_op(cx.tcx, typeck, expr) else {
256259
// The whole chain of reference operations has been seen
257260
if let Some((state, data)) = self.state.take() {
258-
report(cx, expr, state, data, typeck);
261+
report(cx, expr, state, data, typeck, self.in_deref_impl);
259262
}
260263
return;
261264
};
262265
self.skip_expr = skip_expr;
263266

264267
if is_from_proc_macro(cx, expr) {
265268
if let Some((state, data)) = self.state.take() {
266-
report(cx, expr, state, data, cx.typeck_results());
269+
report(cx, expr, state, data, cx.typeck_results(), self.in_deref_impl);
267270
}
268271
return;
269272
}
@@ -515,7 +518,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
515518
(Some((State::DerefedBorrow(state), data)), RefOp::AddrOf(mutability)) => {
516519
let adjusted_ty = data.adjusted_ty;
517520
let stability = state.stability;
518-
report(cx, expr, State::DerefedBorrow(state), data, typeck);
521+
report(cx, expr, State::DerefedBorrow(state), data, typeck, self.in_deref_impl);
519522
if stability.is_deref_stable() {
520523
self.state = Some((
521524
State::Borrow { mutability },
@@ -530,7 +533,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
530533
let adjusted_ty = data.adjusted_ty;
531534
let stability = state.stability;
532535
let for_field_access = state.for_field_access;
533-
report(cx, expr, State::DerefedBorrow(state), data, typeck);
536+
report(cx, expr, State::DerefedBorrow(state), data, typeck, self.in_deref_impl);
534537
if let Some(name) = for_field_access
535538
&& let sub_expr_ty = typeck.expr_ty(sub_expr)
536539
&& !ty_contains_field(sub_expr_ty, name)
@@ -602,7 +605,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
602605
));
603606
},
604607

605-
(Some((state, data)), _) => report(cx, expr, state, data, typeck),
608+
(Some((state, data)), _) => report(cx, expr, state, data, typeck, self.in_deref_impl),
606609
}
607610
}
608611

@@ -673,6 +676,31 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
673676
self.current_body = None;
674677
}
675678
}
679+
680+
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
681+
if is_deref_impl(cx, item) {
682+
self.in_deref_impl = true;
683+
}
684+
}
685+
686+
fn check_item_post(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
687+
if is_deref_impl(cx, item) {
688+
self.in_deref_impl = false;
689+
}
690+
}
691+
}
692+
693+
fn is_deref_impl(cx: &LateContext<'_>, item: &Item<'_>) -> bool {
694+
if let ItemKind::Impl(Impl {
695+
of_trait: Some(of_trait),
696+
..
697+
}) = &item.kind
698+
&& let Some(trait_id) = of_trait.trait_ref.trait_def_id()
699+
{
700+
cx.tcx.is_diagnostic_item(sym::Deref, trait_id)
701+
} else {
702+
false
703+
}
676704
}
677705

678706
fn try_parse_ref_op<'tcx>(
@@ -938,13 +966,19 @@ fn report<'tcx>(
938966
state: State,
939967
data: StateData<'tcx>,
940968
typeck: &'tcx TypeckResults<'tcx>,
969+
in_deref_impl: bool,
941970
) {
942971
match state {
943972
State::DerefMethod {
944973
ty_changed_count,
945974
is_ufcs,
946975
mutbl,
947976
} => {
977+
// Skip when inside implementation of the `Deref` trait.
978+
if in_deref_impl {
979+
return;
980+
}
981+
948982
let mut app = Applicability::MachineApplicable;
949983
let (expr_str, expr_is_macro_call) =
950984
snippet_with_context(cx, expr.span, data.first_expr.span.ctxt(), "..", &mut app);

‎tests/ui/explicit_deref_methods.fixed‎

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

53+
mod issue_15392 {
54+
use std::ops::Deref;
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+
enum MyEnum {
68+
A(WrapperA),
69+
}
70+
71+
impl Deref for MyEnum {
72+
type Target = Inner;
73+
74+
fn deref(&self) -> &Inner {
75+
// forwarding is ok
76+
match self {
77+
MyEnum::A(wrap_a) => Deref::deref(wrap_a),
78+
}
79+
}
80+
}
81+
}
82+
5383
fn main() {
5484
let a: &mut String = &mut String::from("foo");
5585

‎tests/ui/explicit_deref_methods.rs‎

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

53+
mod issue_15392 {
54+
use std::ops::Deref;
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+
enum MyEnum {
68+
A(WrapperA),
69+
}
70+
71+
impl Deref for MyEnum {
72+
type Target = Inner;
73+
74+
fn deref(&self) -> &Inner {
75+
// forwarding is ok
76+
match self {
77+
MyEnum::A(wrap_a) => Deref::deref(wrap_a),
78+
}
79+
}
80+
}
81+
}
82+
5383
fn main() {
5484
let a: &mut String = &mut String::from("foo");
5585

‎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:88: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:91: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:95: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:95: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:99: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:103: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:108: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:111: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:114: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:154: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:184: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:186: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:187: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)