Working basic dereference clip

Author: tommilligan

clippy_lints/src/dereference.rs

@@ -0,0 +1,74 @@
+use crate::rustc::hir::{Expr, ExprKind, QPath};
+use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use crate::rustc::{declare_tool_lint, lint_array};
+use crate::utils::{in_macro, span_lint_and_sugg};
+use if_chain::if_chain;
+
+/// **What it does:** Checks for explicit deref() or deref_mut() method calls.
+///
+/// **Why is this bad?** Derefencing by &*x or &mut *x is clearer and more concise,
+/// when not part of a method chain.
+///
+/// **Example:**
+/// ```rust
+/// let b = a.deref();
+/// let c = a.deref_mut();
+///
+/// // excludes
+/// let e = d.unwrap().deref();
+/// ```
+declare_clippy_lint! {
+    pub EXPLICIT_DEREF_METHOD,
+    pedantic,
+    "Explicit use of deref or deref_mut method while not in a method chain."
+}
+
+pub struct Pass;
+
+impl LintPass for Pass {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(EXPLICIT_DEREF_METHOD)
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
+    fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &Expr) {
+        if in_macro(expr.span) {
+            return;
+        }
+
+        if_chain! {
+            // if this is a method call
+            if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.node;
+            // on a Path (i.e. a variable/name, not another method)
+            if let ExprKind::Path(QPath::Resolved(None, path)) = &args[0].node;
+            then {
+                let name = method_name.ident.as_str();
+                // alter help slightly to account for _mut
+                match &*name {
+                    "deref" => {
+                        span_lint_and_sugg(
+                            cx,
+                            EXPLICIT_DEREF_METHOD,
+                            expr.span,
+                            "explicit deref method call",
+                            "try this",
+                            format!("&*{}", path),
+                        );
+                    },
+                    "deref_mut" => {
+                        span_lint_and_sugg(
+                            cx,
+                            EXPLICIT_DEREF_METHOD,
+                            expr.span,
+                            "explicit deref_mut method call",
+                            "try this",
+                            format!("&mut *{}", path),
+                        );
+                    },
+                    _ => ()
+                };
+            }
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -191,6 +191,7 @@ pub mod copies;
 pub mod copy_iterator;
 pub mod dbg_macro;
 pub mod default_trait_access;
+pub mod dereference;
 pub mod derive;
 pub mod doc;
 pub mod double_comparison;
@@ -391,7 +392,7 @@ pub fn read_conf(args: &[rustc_ast::ast::NestedMetaItem], sess: &Session) -> Con
             }
 
             conf
-        },
+        }
         Err((err, span)) => {
             sess.struct_span_err(span, err)
                 .span_note(span, "Clippy will use default configuration")
@@ -509,6 +510,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &copy_iterator::COPY_ITERATOR,
         &dbg_macro::DBG_MACRO,
         &default_trait_access::DEFAULT_TRAIT_ACCESS,
+        &dereference::DEREF_METHOD_EXPLICIT,
         &derive::DERIVE_HASH_XOR_EQ,
         &derive::EXPL_IMPL_CLONE_ON_COPY,
         &doc::DOC_MARKDOWN,
@@ -1026,6 +1028,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_early_pass(|| box macro_use::MacroUseImports);
     store.register_late_pass(|| box verbose_file_reads::VerboseFileReads);
     store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default());
+    store.register_late_pass(|| box dereference::DerefMethodExplicit);
 
     store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
         LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1076,6 +1079,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&copies::SAME_FUNCTIONS_IN_IF_CONDITION),
         LintId::of(&copy_iterator::COPY_ITERATOR),
         LintId::of(&default_trait_access::DEFAULT_TRAIT_ACCESS),
+        LintId::of(&dereference::EXPLICIT_DEREF_METHOD),
         LintId::of(&derive::EXPL_IMPL_CLONE_ON_COPY),
         LintId::of(&doc::DOC_MARKDOWN),
         LintId::of(&doc::MISSING_ERRORS_DOC),
@@ -1161,6 +1165,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&comparison_chain::COMPARISON_CHAIN),
         LintId::of(&copies::IFS_SAME_COND),
         LintId::of(&copies::IF_SAME_THEN_ELSE),
+        LintId::of(&dereference::EXPLICIT_DEREF_METHOD),
         LintId::of(&derive::DERIVE_HASH_XOR_EQ),
         LintId::of(&doc::MISSING_SAFETY_DOC),
         LintId::of(&doc::NEEDLESS_DOCTEST_MAIN),

tests/ui/dereference.rs

@@ -0,0 +1,55 @@
+#![feature(tool_lints)]
+
+use std::ops::{Deref, DerefMut};
+
+#[allow(clippy::many_single_char_names, clippy::clone_double_ref)]
+#[allow(unused_variables)]
+#[warn(clippy::explicit_deref_method)]
+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();
+    }
+
+    {
+        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;
+    }
+
+    {
+        macro_rules! expr_deref { ($body:expr) => { $body.deref() } }
+        let b: &str = expr_deref!(a);
+    }
+}

tests/ui/dereference.stderr

@@ -0,0 +1,52 @@
+error: explicit deref method call
+  --> $DIR/dereference.rs:13:23
+   |
+13 |         let b: &str = a.deref();
+   |                       ^^^^^^^^^ help: try this: `&*a`
+   |
+   = note: `-D clippy::explicit-deref-method` implied by `-D warnings`
+
+error: explicit deref_mut method call
+  --> $DIR/dereference.rs:17:27
+   |
+17 |         let b: &mut str = a.deref_mut();
+   |                           ^^^^^^^^^^^^^ help: try this: `&mut *a`
+
+error: explicit deref method call
+  --> $DIR/dereference.rs:21:25
+   |
+21 |         let b: String = a.deref().clone();
+   |                         ^^^^^^^^^ help: try this: `&*a`
+
+error: explicit deref_mut method call
+  --> $DIR/dereference.rs:25:24
+   |
+25 |         let b: usize = a.deref_mut().len();
+   |                        ^^^^^^^^^^^^^ help: try this: `&mut *a`
+
+error: explicit deref method call
+  --> $DIR/dereference.rs:29:26
+   |
+29 |         let b: &usize = &a.deref().len();
+   |                          ^^^^^^^^^ help: try this: `&*a`
+
+error: explicit deref method call
+  --> $DIR/dereference.rs:34:23
+   |
+34 |         let b: &str = a.deref().deref();
+   |                       ^^^^^^^^^ help: try this: `&*a`
+
+error: explicit deref method call
+  --> $DIR/dereference.rs:39:43
+   |
+39 |         let b: String = format!("{}, {}", a.deref(), a.deref());
+   |                                           ^^^^^^^^^ help: try this: `&*a`
+
+error: explicit deref method call
+  --> $DIR/dereference.rs:39:54
+   |
+39 |         let b: String = format!("{}, {}", a.deref(), a.deref());
+   |                                                      ^^^^^^^^^ help: try this: `&*a`
+
+error: aborting due to 8 previous errors
+