Report using stmts and expr + tests

clippy_lints/src/dereference.rs

@@ -1,9 +1,11 @@
-use rustc_hir::{Expr, ExprKind, QPath};
+use crate::utils::{get_parent_expr, method_calls, snippet, span_lint_and_sugg};
-use rustc_errors::Applicability;
-use rustc_lint::{LateContext, LateLintPass};
-use rustc_session::{declare_tool_lint, declare_lint_pass};
-use crate::utils::{in_macro, span_lint_and_sugg};
 use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir as hir;
+use rustc_hir::{Expr, ExprKind, QPath, StmtKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::source_map::Span;
 
 declare_clippy_lint! {
     /// **What it does:** Checks for explicit `deref()` or `deref_mut()` method calls.
@@ -36,45 +38,105 @@ declare_lint_pass!(Dereferencing => [
 ]);
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
-    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
+    fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx hir::Stmt<'_>) {
-        if in_macro(expr.span) {
+        if_chain! {
+            if let StmtKind::Local(ref local) = stmt.kind;
+            if let Some(ref init) = local.init;
+
+            then {
+                match init.kind {
+                    ExprKind::Call(ref _method, args) => {
+                        for arg in args {
+                            if_chain! {
+                                // Caller must call only one other function (deref or deref_mut)
+                                // otherwise it can lead to error prone suggestions (ex: &*a.len())
+                                let (method_names, arg_list, _) = method_calls(arg, 2);
+                                if method_names.len() == 1;
+                                // Caller must be a variable
+                                let variables = arg_list[0];
+                                if variables.len() == 1;
+                                if let ExprKind::Path(QPath::Resolved(None, _)) = variables[0].kind;
+
+                                then {
+                                    let name = method_names[0].as_str();
+                                    lint_deref(cx, &*name, variables[0].span, arg.span);
+                                }
+                            }
+                        }
+                    }
+                    ExprKind::MethodCall(ref method_name, _, ref args) => {
+                        if init.span.from_expansion() {
                             return;
                         }
-
                         if_chain! {
-            // if this is a method call
+                            if args.len() == 1;
-            if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.kind;
+                            if let ExprKind::Path(QPath::Resolved(None, _)) = args[0].kind;
-            // on a Path (i.e. a variable/name, not another method)
+                            // Caller must call only one other function (deref or deref_mut)
-            if let ExprKind::Path(QPath::Resolved(None, path)) = &args[0].kind;
+                            // otherwise it can lead to error prone suggestions (ex: &*a.len())
+                            let (method_names, arg_list, _) = method_calls(init, 2);
+                            if method_names.len() == 1;
+                            // Caller must be a variable
+                            let variables = arg_list[0];
+                            if variables.len() == 1;
+                            if let ExprKind::Path(QPath::Resolved(None, _)) = variables[0].kind;
+
                             then {
                                 let name = method_name.ident.as_str();
-                // alter help slightly to account for _mut
+                                lint_deref(cx, &*name, args[0].span, init.span);
-                match &*name {
+                            }
+                        }
+                    }
+                    _ => ()
+                }
+            }
+        }
+    }
+
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
+        if_chain! {
+            if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.kind;
+            if args.len() == 1;
+            if let Some(parent) = get_parent_expr(cx, &expr);
+
+            then {
+                // Call and MethodCall exprs are better reported using statements
+                match parent.kind {
+                    ExprKind::Call(_, _) => return,
+                    ExprKind::MethodCall(_, _, _) => return,
+                    _ => {
+                        let name = method_name.ident.as_str();
+                        lint_deref(cx, &*name, args[0].span, expr.span);
+                    }
+                }
+            }
+        }
+    }
+}
+
+fn lint_deref(cx: &LateContext<'_, '_>, fn_name: &str, var_span: Span, expr_span: Span) {
+    match fn_name {
         "deref" => {
             span_lint_and_sugg(
                 cx,
                 EXPLICIT_DEREF_METHOD,
-                            expr.span,
+                expr_span,
                 "explicit deref method call",
                 "try this",
-                            format!("&*{}", path),
+                format!("&*{}", &snippet(cx, var_span, "..")),
-                            Applicability::MachineApplicable
+                Applicability::MachineApplicable,
             );
         },
         "deref_mut" => {
             span_lint_and_sugg(
                 cx,
                 EXPLICIT_DEREF_METHOD,
-                            expr.span,
+                expr_span,
                 "explicit deref_mut method call",
                 "try this",
-                            format!("&mut *{}", path),
+                format!("&mut *{}", &snippet(cx, var_span, "..")),
-                            Applicability::MachineApplicable
+                Applicability::MachineApplicable,
             );
         },
-                    _ => ()
+        _ => (),
-                };
-            }
-        }
     }
 }

tests/ui/dereference.rs

@@ -3,28 +3,49 @@
 
 use std::ops::{Deref, DerefMut};
 
+fn concat(deref_str: &str) -> String {
+    format!("{}bar", deref_str)
+}
+
+fn just_return(deref_str: &str) -> &str {
+    deref_str
+}
+
 fn main() {
     let a: &mut String = &mut String::from("foo");
 
     // these should require linting
+
     let b: &str = a.deref();
 
     let b: &mut str = a.deref_mut();
 
+    // both derefs should get linted here
+    let b: String = format!("{}, {}", a.deref(), a.deref());
+
+    println!("{}", a.deref());
+
+    #[allow(clippy::match_single_binding)]
+    match a.deref() {
+        _ => (),
+    }
+
+    let b: String = concat(a.deref());
+
+    // following should not require linting
+
+    let b = just_return(a).deref();
+
+    let b: String = concat(just_return(a).deref());
+
     let b: String = a.deref().clone();
 
     let b: usize = a.deref_mut().len();
 
     let b: &usize = &a.deref().len();
 
-    // only first deref should get linted here
     let b: &str = a.deref().deref();
 
-    // both derefs should get linted here
-    let b: String = format!("{}, {}", a.deref(), a.deref());
-
-    // these should not require linting
-
     let b: &str = &*a;
 
     let b: &mut str = &mut *a;
@@ -35,4 +56,7 @@ fn main() {
         };
     }
     let b: &str = expr_deref!(a);
+
+    let opt_a = Some(a);
+    let b = opt_a.unwrap().deref();
 }

tests/ui/dereference.stderr

@@ -1,5 +1,5 @@
 error: explicit deref method call
-  --> $DIR/dereference.rs:10:19
+  --> $DIR/dereference.rs:19:19
    |
 LL |     let b: &str = a.deref();
    |                   ^^^^^^^^^ help: try this: `&*a`
@@ -7,35 +7,11 @@ LL |     let b: &str = a.deref();
    = note: `-D clippy::explicit-deref-method` implied by `-D warnings`
 
 error: explicit deref_mut method call
-  --> $DIR/dereference.rs:12:23
+  --> $DIR/dereference.rs:21:23
    |
 LL |     let b: &mut str = a.deref_mut();
    |                       ^^^^^^^^^^^^^ help: try this: `&mut *a`
 
-error: explicit deref method call
-  --> $DIR/dereference.rs:14:21
-   |
-LL |     let b: String = a.deref().clone();
-   |                     ^^^^^^^^^ help: try this: `&*a`
-
-error: explicit deref_mut method call
-  --> $DIR/dereference.rs:16:20
-   |
-LL |     let b: usize = a.deref_mut().len();
-   |                    ^^^^^^^^^^^^^ help: try this: `&mut *a`
-
-error: explicit deref method call
-  --> $DIR/dereference.rs:18:22
-   |
-LL |     let b: &usize = &a.deref().len();
-   |                      ^^^^^^^^^ help: try this: `&*a`
-
-error: explicit deref method call
-  --> $DIR/dereference.rs:21:19
-   |
-LL |     let b: &str = a.deref().deref();
-   |                   ^^^^^^^^^ help: try this: `&*a`
-
 error: explicit deref method call
   --> $DIR/dereference.rs:24:39
    |
@@ -48,5 +24,23 @@ error: explicit deref method call
 LL |     let b: String = format!("{}, {}", a.deref(), a.deref());
    |                                                  ^^^^^^^^^ help: try this: `&*a`
 
-error: aborting due to 8 previous errors
+error: explicit deref method call
+  --> $DIR/dereference.rs:26:20
+   |
+LL |     println!("{}", a.deref());
+   |                    ^^^^^^^^^ help: try this: `&*a`
+
+error: explicit deref method call
+  --> $DIR/dereference.rs:29:11
+   |
+LL |     match a.deref() {
+   |           ^^^^^^^^^ help: try this: `&*a`
+
+error: explicit deref method call
+  --> $DIR/dereference.rs:33:28
+   |
+LL |     let b: String = concat(a.deref());
+   |                            ^^^^^^^^^ help: try this: `&*a`
+
+error: aborting due to 7 previous errors