Skip to content

Commit 8e0ba68

Browse files
ThibsGThibsG
committed
Check for Deref trait impl + add fixed version
1 parent 172fd00 commit 8e0ba68

File tree

5 files changed

+132
-22
lines changed

5 files changed

+132
-22
lines changed

‎clippy_lints/src/dereference.rs‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
use crate::utils::{get_parent_expr, method_calls, snippet, span_lint_and_sugg};
1+
use crate::utils::{
2+
get_parent_expr, get_trait_def_id, implements_trait, method_calls, paths, snippet, span_lint_and_sugg,
3+
};
24
use if_chain::if_chain;
35
use rustc_errors::Applicability;
46
use rustc_hir as hir;
@@ -15,18 +17,19 @@ declare_clippy_lint! {
1517
///
1618
/// **Example:**
1719
/// ```rust
18-
/// let b = a.deref();
19-
/// let c = a.deref_mut();
20+
/// use std::ops::Deref;
21+
/// let a: &mut String = &mut String::from("foo");
22+
/// let b: &str = a.deref();
2023
/// ```
2124
/// Could be written as:
2225
/// ```rust
26+
/// let a: &mut String = &mut String::from("foo");
2327
/// let b = &*a;
24-
/// let c = &mut *a;
2528
/// ```
2629
///
2730
/// This lint excludes
28-
/// ```rust
29-
/// let e = d.unwrap().deref();
31+
/// ```rust,ignore
32+
/// let _ = d.unwrap().deref();
3033
/// ```
3134
pub EXPLICIT_DEREF_METHOD,
3235
pedantic,
@@ -49,7 +52,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
4952
for arg in args {
5053
if_chain! {
5154
// Caller must call only one other function (deref or deref_mut)
52-
// otherwise it can lead to error prone suggestions (ex: &*a.len())
55+
// otherwise it can lead to error prone suggestions (ie: &*a.len())
5356
let (method_names, arg_list, _) = method_calls(arg, 2);
5457
if method_names.len() == 1;
5558
// Caller must be a variable
@@ -59,7 +62,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
5962

6063
then {
6164
let name = method_names[0].as_str();
62-
lint_deref(cx, &*name, variables[0].span, arg.span);
65+
lint_deref(cx, &*name, &variables[0], variables[0].span, arg.span);
6366
}
6467
}
6568
}
@@ -72,7 +75,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
7275
if args.len() == 1;
7376
if let ExprKind::Path(QPath::Resolved(None, _)) = args[0].kind;
7477
// Caller must call only one other function (deref or deref_mut)
75-
// otherwise it can lead to error prone suggestions (ex: &*a.len())
78+
// otherwise it can lead to error prone suggestions (ie: &*a.len())
7679
let (method_names, arg_list, _) = method_calls(init, 2);
7780
if method_names.len() == 1;
7881
// Caller must be a variable
@@ -82,7 +85,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
8285

8386
then {
8487
let name = method_name.ident.as_str();
85-
lint_deref(cx, &*name, args[0].span, init.span);
88+
lint_deref(cx, &*name, init, args[0].span, init.span);
8689
}
8790
}
8891
}
@@ -99,23 +102,27 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
99102
if let Some(parent) = get_parent_expr(cx, &expr);
100103

101104
then {
102-
// Call and MethodCall exprs are better reported using statements
103105
match parent.kind {
104-
ExprKind::Call(_, _) => return,
105-
ExprKind::MethodCall(_, _, _) => return,
106+
// Already linted using statements
107+
ExprKind::Call(_, _) | ExprKind::MethodCall(_, _, _) => (),
106108
_ => {
107109
let name = method_name.ident.as_str();
108-
lint_deref(cx, &*name, args[0].span, expr.span);
110+
lint_deref(cx, &*name, &args[0], args[0].span, expr.span);
109111
}
110112
}
111113
}
112114
}
113115
}
114116
}
115117

116-
fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, var_span: Span, expr_span: Span) {
118+
fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) {
117119
match fn_name {
118120
"deref" => {
121+
if !get_trait_def_id(cx, &paths::DEREF_TRAIT)
122+
.map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[]))
123+
{
124+
return;
125+
}
119126
span_lint_and_sugg(
120127
cx,
121128
EXPLICIT_DEREF_METHOD,
@@ -127,6 +134,11 @@ fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, var_span: Span, expr_span
127134
);
128135
},
129136
"deref_mut" => {
137+
if !get_trait_def_id(cx, &paths::DEREF_MUT_TRAIT)
138+
.map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[]))
139+
{
140+
return;
141+
}
130142
span_lint_and_sugg(
131143
cx,
132144
EXPLICIT_DEREF_METHOD,

‎clippy_lints/src/utils/paths.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ pub const CSTRING: [&str; 4] = ["std", "ffi", "c_str", "CString"];
2121
pub const CSTRING_AS_C_STR: [&str; 5] = ["std", "ffi", "c_str", "CString", "as_c_str"];
2222
pub const DEFAULT_TRAIT: [&str; 3] = ["core", "default", "Default"];
2323
pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"];
24+
pub const DEREF_MUT_TRAIT: [&str; 4] = ["core", "ops", "deref", "DerefMut"];
2425
pub const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"];
26+
pub const DEREF_TRAIT: [&str; 4] = ["core", "ops", "deref", "Deref"];
2527
pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"];
2628
pub const DISPLAY_FMT_METHOD: [&str; 4] = ["core", "fmt", "Display", "fmt"];
2729
pub const DOUBLE_ENDED_ITERATOR: [&str; 4] = ["core", "iter", "traits", "DoubleEndedIterator"];

‎tests/ui/dereference.fixed‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// run-rustfix
2+
3+
#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)]
4+
#![warn(clippy::explicit_deref_method)]
5+
6+
use std::ops::{Deref, DerefMut};
7+
8+
fn concat(deref_str: &str) -> String {
9+
format!("{}bar", deref_str)
10+
}
11+
12+
fn just_return(deref_str: &str) -> &str {
13+
deref_str
14+
}
15+
16+
fn main() {
17+
let a: &mut String = &mut String::from("foo");
18+
19+
// these should require linting
20+
21+
let b: &str = &*a;
22+
23+
let b: &mut str = &mut *a;
24+
25+
// both derefs should get linted here
26+
let b: String = format!("{}, {}", &*a, &*a);
27+
28+
println!("{}", &*a);
29+
30+
#[allow(clippy::match_single_binding)]
31+
match &*a {
32+
_ => (),
33+
}
34+
35+
let b: String = concat(&*a);
36+
37+
// following should not require linting
38+
39+
let b = just_return(a).deref();
40+
41+
let b: String = concat(just_return(a).deref());
42+
43+
let b: String = a.deref().clone();
44+
45+
let b: usize = a.deref_mut().len();
46+
47+
let b: &usize = &a.deref().len();
48+
49+
let b: &str = a.deref().deref();
50+
51+
let b: &str = &*a;
52+
53+
let b: &mut str = &mut *a;
54+
55+
macro_rules! expr_deref {
56+
($body:expr) => {
57+
$body.deref()
58+
};
59+
}
60+
let b: &str = expr_deref!(a);
61+
62+
let opt_a = Some(a);
63+
let b = opt_a.unwrap().deref();
64+
65+
// The struct does not implement Deref trait
66+
#[derive(Copy, Clone)]
67+
struct NoLint(u32);
68+
impl NoLint {
69+
pub fn deref(self) -> u32 {
70+
self.0
71+
}
72+
pub fn deref_mut(self) -> u32 {
73+
self.0
74+
}
75+
}
76+
let no_lint = NoLint(42);
77+
let b = no_lint.deref();
78+
let b = no_lint.deref_mut();
79+
}

‎tests/ui/dereference.rs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// run-rustfix
2+
13
#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)]
24
#![warn(clippy::explicit_deref_method)]
35

@@ -59,4 +61,19 @@ fn main() {
5961

6062
let opt_a = Some(a);
6163
let b = opt_a.unwrap().deref();
64+
65+
// The struct does not implement Deref trait
66+
#[derive(Copy, Clone)]
67+
struct NoLint(u32);
68+
impl NoLint {
69+
pub fn deref(self) -> u32 {
70+
self.0
71+
}
72+
pub fn deref_mut(self) -> u32 {
73+
self.0
74+
}
75+
}
76+
let no_lint = NoLint(42);
77+
let b = no_lint.deref();
78+
let b = no_lint.deref_mut();
6279
}

‎tests/ui/dereference.stderr‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,43 @@
11
error: explicit deref method call
2-
--> $DIR/dereference.rs:19:19
2+
--> $DIR/dereference.rs:21:19
33
|
44
LL | let b: &str = a.deref();
55
| ^^^^^^^^^ help: try this: `&*a`
66
|
77
= note: `-D clippy::explicit-deref-method` implied by `-D warnings`
88

99
error: explicit deref_mut method call
10-
--> $DIR/dereference.rs:21:23
10+
--> $DIR/dereference.rs:23:23
1111
|
1212
LL | let b: &mut str = a.deref_mut();
1313
| ^^^^^^^^^^^^^ help: try this: `&mut *a`
1414

1515
error: explicit deref method call
16-
--> $DIR/dereference.rs:24:39
16+
--> $DIR/dereference.rs:26:39
1717
|
1818
LL | let b: String = format!("{}, {}", a.deref(), a.deref());
1919
| ^^^^^^^^^ help: try this: `&*a`
2020

2121
error: explicit deref method call
22-
--> $DIR/dereference.rs:24:50
22+
--> $DIR/dereference.rs:26:50
2323
|
2424
LL | let b: String = format!("{}, {}", a.deref(), a.deref());
2525
| ^^^^^^^^^ help: try this: `&*a`
2626

2727
error: explicit deref method call
28-
--> $DIR/dereference.rs:26:20
28+
--> $DIR/dereference.rs:28:20
2929
|
3030
LL | println!("{}", a.deref());
3131
| ^^^^^^^^^ help: try this: `&*a`
3232

3333
error: explicit deref method call
34-
--> $DIR/dereference.rs:29:11
34+
--> $DIR/dereference.rs:31:11
3535
|
3636
LL | match a.deref() {
3737
| ^^^^^^^^^ help: try this: `&*a`
3838

3939
error: explicit deref method call
40-
--> $DIR/dereference.rs:33:28
40+
--> $DIR/dereference.rs:35:28
4141
|
4242
LL | let b: String = concat(a.deref());
4343
| ^^^^^^^^^ help: try this: `&*a`

0 commit comments

Comments
 (0)