Skip to content

Commit b0fbe55

Browse files
ThibsGThibsG
committed
Use only check_expr with parent expr and precedence
1 parent 961695c commit b0fbe55

File tree

8 files changed

+79
-100
lines changed

8 files changed

+79
-100
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1256,7 +1256,7 @@ Released 2018-09-13
12561256
[`expect_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#expect_fun_call
12571257
[`expl_impl_clone_on_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#expl_impl_clone_on_copy
12581258
[`explicit_counter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_counter_loop
1259-
[`explicit_deref_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_deref_method
1259+
[`explicit_deref_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_deref_methods
12601260
[`explicit_into_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_into_iter_loop
12611261
[`explicit_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_iter_loop
12621262
[`explicit_write`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_write

‎clippy_lints/src/dereference.rs‎

Lines changed: 28 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
1-
use crate::utils::{
2-
get_parent_expr, get_trait_def_id, implements_trait, method_calls, paths, snippet, span_lint_and_sugg,
3-
};
1+
use crate::utils::{get_parent_expr, implements_trait, snippet, span_lint_and_sugg};
42
use if_chain::if_chain;
3+
use rustc_ast::util::parser::ExprPrecedence;
54
use rustc_errors::Applicability;
6-
use rustc_hir as hir;
7-
use rustc_hir::{Expr, ExprKind, QPath, StmtKind};
5+
use rustc_hir::{Expr, ExprKind};
86
use rustc_lint::{LateContext, LateLintPass};
97
use rustc_session::{declare_lint_pass, declare_tool_lint};
108
use rustc_span::source_map::Span;
@@ -31,100 +29,52 @@ declare_clippy_lint! {
3129
/// ```rust,ignore
3230
/// let _ = d.unwrap().deref();
3331
/// ```
34-
pub EXPLICIT_DEREF_METHOD,
32+
pub EXPLICIT_DEREF_METHODS,
3533
pedantic,
3634
"Explicit use of deref or deref_mut method while not in a method chain."
3735
}
3836

3937
declare_lint_pass!(Dereferencing => [
40-
EXPLICIT_DEREF_METHOD
38+
EXPLICIT_DEREF_METHODS
4139
]);
4240

4341
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
44-
fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx hir::Stmt<'_>) {
45-
if_chain! {
46-
if let StmtKind::Local(ref local) = stmt.kind;
47-
if let Some(ref init) = local.init;
48-
49-
then {
50-
match init.kind {
51-
ExprKind::Call(ref _method, args) => {
52-
for arg in args {
53-
if_chain! {
54-
// Caller must call only one other function (deref or deref_mut)
55-
// otherwise it can lead to error prone suggestions (ie: &*a.len())
56-
let (method_names, arg_list, _) = method_calls(arg, 2);
57-
if method_names.len() == 1;
58-
// Caller must be a variable
59-
let variables = arg_list[0];
60-
if variables.len() == 1;
61-
if let ExprKind::Path(QPath::Resolved(None, _)) = variables[0].kind;
62-
63-
then {
64-
let name = method_names[0].as_str();
65-
lint_deref(cx, &*name, &variables[0], variables[0].span, arg.span);
66-
}
67-
}
68-
}
69-
}
70-
ExprKind::MethodCall(ref method_name, _, ref args) => {
71-
if init.span.from_expansion() {
72-
return;
73-
}
74-
if_chain! {
75-
if args.len() == 1;
76-
if let ExprKind::Path(QPath::Resolved(None, _)) = args[0].kind;
77-
// Caller must call only one other function (deref or deref_mut)
78-
// otherwise it can lead to error prone suggestions (ie: &*a.len())
79-
let (method_names, arg_list, _) = method_calls(init, 2);
80-
if method_names.len() == 1;
81-
// Caller must be a variable
82-
let variables = arg_list[0];
83-
if variables.len() == 1;
84-
if let ExprKind::Path(QPath::Resolved(None, _)) = variables[0].kind;
85-
86-
then {
87-
let name = method_name.ident.as_str();
88-
lint_deref(cx, &*name, init, args[0].span, init.span);
89-
}
90-
}
91-
}
92-
_ => ()
93-
}
94-
}
95-
}
96-
}
97-
9842
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
9943
if_chain! {
44+
if !expr.span.from_expansion();
10045
if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.kind;
10146
if args.len() == 1;
102-
if let ExprKind::Path(QPath::Resolved(None, _)) = args[0].kind;
103-
if let Some(parent) = get_parent_expr(cx, &expr);
10447

10548
then {
106-
match parent.kind {
107-
// Already linted using statements
108-
ExprKind::Call(_, _) | ExprKind::MethodCall(_, _, _) => (),
109-
_ => {
110-
let name = method_name.ident.as_str();
111-
lint_deref(cx, &*name, &args[0], args[0].span, expr.span);
49+
if let Some(parent_expr) = get_parent_expr(cx, expr) {
50+
// Check if we have the whole call chain here
51+
if let ExprKind::MethodCall(..) = parent_expr.kind {
52+
return;
53+
}
54+
// Check for unary precedence
55+
if let ExprPrecedence::Unary = parent_expr.precedence() {
56+
return;
11257
}
11358
}
59+
let name = method_name.ident.as_str();
60+
lint_deref(cx, &*name, &args[0], args[0].span, expr.span);
11461
}
11562
}
11663
}
11764
}
11865

119-
fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) {
120-
match fn_name {
66+
fn lint_deref(cx: &LateContext<'_, '_>, method_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) {
67+
match method_name {
12168
"deref" => {
122-
if get_trait_def_id(cx, &paths::DEREF_TRAIT)
69+
if cx
70+
.tcx
71+
.lang_items()
72+
.deref_trait()
12373
.map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[]))
12474
{
12575
span_lint_and_sugg(
12676
cx,
127-
EXPLICIT_DEREF_METHOD,
77+
EXPLICIT_DEREF_METHODS,
12878
expr_span,
12979
"explicit deref method call",
13080
"try this",
@@ -134,12 +84,15 @@ fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, call_expr: &Expr<'_>, var
13484
}
13585
},
13686
"deref_mut" => {
137-
if get_trait_def_id(cx, &paths::DEREF_MUT_TRAIT)
87+
if cx
88+
.tcx
89+
.lang_items()
90+
.deref_mut_trait()
13891
.map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[]))
13992
{
14093
span_lint_and_sugg(
14194
cx,
142-
EXPLICIT_DEREF_METHOD,
95+
EXPLICIT_DEREF_METHODS,
14396
expr_span,
14497
"explicit deref_mut method call",
14598
"try this",

‎clippy_lints/src/lib.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
510510
&copy_iterator::COPY_ITERATOR,
511511
&dbg_macro::DBG_MACRO,
512512
&default_trait_access::DEFAULT_TRAIT_ACCESS,
513-
&dereference::EXPLICIT_DEREF_METHOD,
513+
&dereference::EXPLICIT_DEREF_METHODS,
514514
&derive::DERIVE_HASH_XOR_EQ,
515515
&derive::EXPL_IMPL_CLONE_ON_COPY,
516516
&doc::DOC_MARKDOWN,
@@ -1079,7 +1079,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10791079
LintId::of(&copies::SAME_FUNCTIONS_IN_IF_CONDITION),
10801080
LintId::of(&copy_iterator::COPY_ITERATOR),
10811081
LintId::of(&default_trait_access::DEFAULT_TRAIT_ACCESS),
1082-
LintId::of(&dereference::EXPLICIT_DEREF_METHOD),
1082+
LintId::of(&dereference::EXPLICIT_DEREF_METHODS),
10831083
LintId::of(&derive::EXPL_IMPL_CLONE_ON_COPY),
10841084
LintId::of(&doc::DOC_MARKDOWN),
10851085
LintId::of(&doc::MISSING_ERRORS_DOC),

‎clippy_lints/src/utils/paths.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@ 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"];
2524
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"];
2725
pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"];
2826
pub const DISPLAY_FMT_METHOD: [&str; 4] = ["core", "fmt", "Display", "fmt"];
2927
pub const DISPLAY_TRAIT: [&str; 3] = ["core", "fmt", "Display"];

‎src/lintlist/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,7 @@ pub const ALL_LINTS: [Lint; 362] = [
526526
module: "loops",
527527
},
528528
Lint {
529-
name: "explicit_deref_method",
529+
name: "explicit_deref_methods",
530530
group: "pedantic",
531531
desc: "Explicit use of deref or deref_mut method while not in a method chain.",
532532
deprecation: None,

‎tests/ui/dereference.fixed‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// run-rustfix
22

33
#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)]
4-
#![warn(clippy::explicit_deref_method)]
4+
#![warn(clippy::explicit_deref_methods)]
55

66
use std::ops::{Deref, DerefMut};
77

@@ -34,20 +34,25 @@ fn main() {
3434

3535
let b: String = concat(&*a);
3636

37-
// following should not require linting
37+
let b = &*just_return(a);
38+
39+
let b: String = concat(&*just_return(a));
3840

39-
let b = just_return(a).deref();
41+
let b: &str = &*a.deref();
4042

41-
let b: String = concat(just_return(a).deref());
43+
let opt_a = Some(a.clone());
44+
let b = &*opt_a.unwrap();
45+
46+
// following should not require linting
47+
48+
let b: &str = &*a.deref();
4249

4350
let b: String = a.deref().clone();
4451

4552
let b: usize = a.deref_mut().len();
4653

4754
let b: &usize = &a.deref().len();
4855

49-
let b: &str = a.deref().deref();
50-
5156
let b: &str = &*a;
5257

5358
let b: &mut str = &mut *a;
@@ -59,9 +64,6 @@ fn main() {
5964
}
6065
let b: &str = expr_deref!(a);
6166

62-
let opt_a = Some(a);
63-
let b = opt_a.unwrap().deref();
64-
6567
// The struct does not implement Deref trait
6668
#[derive(Copy, Clone)]
6769
struct NoLint(u32);

‎tests/ui/dereference.rs‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// run-rustfix
22

33
#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)]
4-
#![warn(clippy::explicit_deref_method)]
4+
#![warn(clippy::explicit_deref_methods)]
55

66
use std::ops::{Deref, DerefMut};
77

@@ -34,20 +34,25 @@ fn main() {
3434

3535
let b: String = concat(a.deref());
3636

37-
// following should not require linting
38-
3937
let b = just_return(a).deref();
4038

4139
let b: String = concat(just_return(a).deref());
4240

41+
let b: &str = a.deref().deref();
42+
43+
let opt_a = Some(a.clone());
44+
let b = opt_a.unwrap().deref();
45+
46+
// following should not require linting
47+
48+
let b: &str = &*a.deref();
49+
4350
let b: String = a.deref().clone();
4451

4552
let b: usize = a.deref_mut().len();
4653

4754
let b: &usize = &a.deref().len();
4855

49-
let b: &str = a.deref().deref();
50-
5156
let b: &str = &*a;
5257

5358
let b: &mut str = &mut *a;
@@ -59,9 +64,6 @@ fn main() {
5964
}
6065
let b: &str = expr_deref!(a);
6166

62-
let opt_a = Some(a);
63-
let b = opt_a.unwrap().deref();
64-
6567
// The struct does not implement Deref trait
6668
#[derive(Copy, Clone)]
6769
struct NoLint(u32);

‎tests/ui/dereference.stderr‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ error: explicit deref method call
44
LL | let b: &str = a.deref();
55
| ^^^^^^^^^ help: try this: `&*a`
66
|
7-
= note: `-D clippy::explicit-deref-method` implied by `-D warnings`
7+
= note: `-D clippy::explicit-deref-methods` implied by `-D warnings`
88

99
error: explicit deref_mut method call
1010
--> $DIR/dereference.rs:23:23
@@ -42,5 +42,29 @@ error: explicit deref method call
4242
LL | let b: String = concat(a.deref());
4343
| ^^^^^^^^^ help: try this: `&*a`
4444

45-
error: aborting due to 7 previous errors
45+
error: explicit deref method call
46+
--> $DIR/dereference.rs:37:13
47+
|
48+
LL | let b = just_return(a).deref();
49+
| ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*just_return(a)`
50+
51+
error: explicit deref method call
52+
--> $DIR/dereference.rs:39:28
53+
|
54+
LL | let b: String = concat(just_return(a).deref());
55+
| ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*just_return(a)`
56+
57+
error: explicit deref method call
58+
--> $DIR/dereference.rs:41:19
59+
|
60+
LL | let b: &str = a.deref().deref();
61+
| ^^^^^^^^^^^^^^^^^ help: try this: `&*a.deref()`
62+
63+
error: explicit deref method call
64+
--> $DIR/dereference.rs:44:13
65+
|
66+
LL | let b = opt_a.unwrap().deref();
67+
| ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*opt_a.unwrap()`
68+
69+
error: aborting due to 11 previous errors
4670

0 commit comments

Comments
 (0)