New Lint: needless_deref

CHANGELOG.md

@@ -2818,6 +2818,7 @@ Released 2018-09-13
 [`needless_borrowed_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrowed_reference
 [`needless_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_collect
 [`needless_continue`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_continue
+[`needless_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_deref
 [`needless_doctest_main`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_doctest_main
 [`needless_for_each`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_for_each
 [`needless_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes

clippy_lints/src/attrs.rs

@@ -603,7 +603,7 @@ fn check_mismatched_target_os(cx: &EarlyContext<'_>, attr: &Attribute) {
                     MetaItemKind::Word => {
                         if_chain! {
                             if let Some(ident) = meta.ident();
-                            if let Some(os) = find_os(&*ident.name.as_str());
+                            if let Some(os) = find_os(&ident.name.as_str());
                             then {
                                 mismatched.push((os, ident.span));
                             }

clippy_lints/src/lib.rs

@@ -288,6 +288,7 @@ mod needless_bool;
 mod needless_borrow;
 mod needless_borrowed_ref;
 mod needless_continue;
+mod needless_deref;
 mod needless_for_each;
 mod needless_pass_by_value;
 mod needless_question_mark;
@@ -842,6 +843,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         needless_borrow::REF_BINDING_TO_REFERENCE,
         needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE,
         needless_continue::NEEDLESS_CONTINUE,
+        needless_deref::NEEDLESS_DEREF,
         needless_for_each::NEEDLESS_FOR_EACH,
         needless_pass_by_value::NEEDLESS_PASS_BY_VALUE,
         needless_question_mark::NEEDLESS_QUESTION_MARK,
@@ -1122,6 +1124,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(needless_bitwise_bool::NEEDLESS_BITWISE_BOOL),
         LintId::of(needless_borrow::REF_BINDING_TO_REFERENCE),
         LintId::of(needless_continue::NEEDLESS_CONTINUE),
+        LintId::of(needless_deref::NEEDLESS_DEREF),
         LintId::of(needless_for_each::NEEDLESS_FOR_EACH),
         LintId::of(needless_pass_by_value::NEEDLESS_PASS_BY_VALUE),
         LintId::of(non_expressive_names::SIMILAR_NAMES),
@@ -1918,6 +1921,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box mutex_atomic::Mutex);
     store.register_late_pass(|| box needless_update::NeedlessUpdate);
     store.register_late_pass(|| box needless_borrow::NeedlessBorrow::default());
+    store.register_late_pass(|| box needless_deref::NeedlessDeref);
     store.register_late_pass(|| box needless_borrowed_ref::NeedlessBorrowedRef);
     store.register_late_pass(|| box no_effect::NoEffect);
     store.register_late_pass(|| box temporary_assignment::TemporaryAssignment);

clippy_lints/src/needless_bool.rs

@@ -328,7 +328,7 @@ enum Expression {
 
 fn fetch_bool_block(block: &Block<'_>) -> Expression {
     match (&*block.stmts, block.expr.as_ref()) {
-        (&[], Some(e)) => fetch_bool_expr(&**e),
+        (&[], Some(e)) => fetch_bool_expr(e),
         (&[ref e], None) => {
             if let StmtKind::Semi(e) = e.kind {
                 if let ExprKind::Ret(_) = e.kind {

clippy_lints/src/needless_deref.rs

@@ -0,0 +1,122 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_opt;
+use rustc_ast::ast::Mutability;
+use rustc_errors::Applicability;
+use rustc_hir::UnOp;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::{self, Ty};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for manual deref in function(no generic type) parameters.
+    ///
+    /// ### Why is this bad?
+    /// There is no need to deref manually. Compiler will auto deref.
+    ///
+    /// ### Known problems
+    /// Complicate type is not handled. For example `foo(&*****(&&T))`.
+    ///
+    /// ### Example
+    /// ```rust
+    /// fn foo(_: &str) {}
+    /// let pf = &String::new();
+    /// // Bad
+    /// foo(&**pf);
+    /// foo(&*String::new());
+    ///
+    /// // Good
+    /// foo(pf);
+    /// foo(&String::new());
+    /// ```
+    pub NEEDLESS_DEREF,
+    pedantic,
+    "remove needless deref"
+}
+
+declare_lint_pass!(NeedlessDeref => [NEEDLESS_DEREF]);
+
+impl LateLintPass<'_> for NeedlessDeref {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
+        match e.kind {
+            ExprKind::Call(fn_expr, arguments) => {
+                if let ExprKind::Path(..) = fn_expr.kind {
+                    // contain no generic parameter
+                    if cx.typeck_results().node_substs_opt(fn_expr.hir_id).is_none() {
+                        check_arguments(cx, arguments, cx.typeck_results().expr_ty(fn_expr));
+                    }
+                }
+            },
+            ExprKind::MethodCall(path, _, arguments, _) => {
+                // contain no generic parameter
+                if path.args.is_none() && cx.typeck_results().node_substs_opt(e.hir_id).is_none() {
+                    let def_id = cx.typeck_results().type_dependent_def_id(e.hir_id).unwrap();
+                    let method_type = cx.tcx.type_of(def_id);
+                    check_arguments(cx, arguments, method_type);
+                }
+            },
+            _ => (),
+        }
+    }
+}
+
+fn check_arguments<'tcx>(cx: &LateContext<'tcx>, arguments: &[Expr<'_>], type_definition: Ty<'tcx>) {
+    match type_definition.kind() {
+        ty::FnDef(..) | ty::FnPtr(_) => {
+            for argument in arguments.iter() {
+                // a: &T
+                // foo(&** a) -> foo(a)
+                if_chain! {
+                    if let ExprKind::AddrOf(_, mutability, child1) = argument.kind ;
+                    if let ExprKind::Unary(UnOp::Deref, child2) = child1.kind ;
+                    if let ExprKind::Unary(UnOp::Deref, child3) = child2.kind ;
+                    if !matches!(child3.kind,ExprKind::Unary(UnOp::Deref, ..) );
+                    let ty = cx.typeck_results().expr_ty(child3);
+                    if matches!(ty.kind(),ty::Ref(..));
+                    then{
+                        let help=match mutability{
+                            Mutability::Not=> "try remove the `&**` and just keep",
+                            Mutability::Mut=> "try remove the `&mut **` and just keep",
+                        };
+                        span_lint_and_sugg(
+                            cx,
+                            NEEDLESS_DEREF,
+                            argument.span,
+                            "needless explicit deref in function parameter",
+                            help,
+                            snippet_opt(cx, child3.span).unwrap(),
+                            Applicability::MachineApplicable,
+                        );
+                    }
+                }
+
+                // a: T
+                // foo(&*a) -> foo(&a)
+                if_chain! {
+                    if let ExprKind::AddrOf(_, mutability, child1) = argument.kind ;
+                    if let ExprKind::Unary(UnOp::Deref, child2) = child1.kind ;
+                    if !matches!(child2.kind,ExprKind::Unary(UnOp::Deref, ..) );
+                    let ty = cx.typeck_results().expr_ty(child2);
+                    if !matches!(ty.kind(),ty::Ref(..));
+                    then{
+                        let sugg= match mutability{
+                            Mutability::Not=> ("&".to_owned()+&snippet_opt(cx, child2.span).unwrap()).clone(),
+                            Mutability::Mut=> ("&mut ".to_owned()+&snippet_opt(cx, child2.span).unwrap()).clone(),
+                        };
+                        span_lint_and_sugg(
+                            cx,
+                            NEEDLESS_DEREF,
+                            argument.span,
+                            "needless explicit deref in function parameter",
+                            "you can replace this with",
+                            sugg,
+                            Applicability::MachineApplicable,
+                        );
+                    }
+                }
+            }
+        },
+        _ => (),
+    }
+}

clippy_lints/src/redundant_static_lifetimes.rs

@@ -86,7 +86,7 @@ impl RedundantStaticLifetimes {
                         _ => {},
                     }
                 }
-                self.visit_type(&*borrow_type.ty, cx, reason);
+                self.visit_type(&borrow_type.ty, cx, reason);
             },
             TyKind::Slice(ref ty) => {
                 self.visit_type(ty, cx, reason);

tests/ui/needless_deref.fixed

@@ -0,0 +1,169 @@
+// run-rustfix
+
+#![allow(dead_code)]
+#![warn(clippy::needless_deref)]
+
+fn main() {}
+
+mod immutable {
+    use std::path::Path;
+    use std::path::PathBuf;
+    use std::sync::Arc;
+
+    fn foo(_: &Path) {}
+    fn bar<T>(_: T) {}
+
+    // Arc<PathBuf> -> PathBuf -> Path
+    fn test_call() {
+        {
+            let a = Arc::new(PathBuf::new());
+            foo(&**a); // should not lint
+
+            let a = &Arc::new(PathBuf::new());
+            foo(&***a); // should not lint
+
+            foo(&PathBuf::new()); // should lint
+
+            let b = &PathBuf::new();
+            foo(b); // should not lint
+            foo(&*b); // should not lint
+            foo(b); // should lint
+        }
+        {
+            let a = Arc::new(PathBuf::new());
+            bar(&**a); // should not lint
+
+            let a = &Arc::new(PathBuf::new());
+            bar(&***a); // should not lint
+
+            bar(&*PathBuf::new()); // should not lint
+
+            let b = &PathBuf::new();
+            bar(b); // should not lint
+            bar(&*b); // should not lint
+            bar(&**b); // should not lint
+        }
+        {
+            let b = &PathBuf::new();
+            let z = &**b;
+            foo(z); // should lint, false negative
+        }
+    }
+
+    struct S;
+    impl S {
+        fn foo(&self, _a: &Path) {}
+        fn bar<T>(&self, _a: T) {}
+    }
+
+    fn test_method_call() {
+        let s = S;
+        {
+            let a = Arc::new(PathBuf::new());
+            s.foo(&**a); // should not lint
+
+            let a = &Arc::new(PathBuf::new());
+            s.foo(&***a); // should not lint
+
+            s.foo(&PathBuf::new()); // should lint
+
+            let b = &PathBuf::new();
+            s.foo(b); // should not lint
+            s.foo(&*b); // should not lint
+            s.foo(b); // should lint
+        }
+        {
+            let a = Arc::new(PathBuf::new());
+            s.bar(&**a); // should not lint
+
+            let a = &Arc::new(PathBuf::new());
+            s.bar(&***a); // should not lint
+
+            s.bar(&*PathBuf::new()); // should not lint
+
+            let b = &PathBuf::new();
+            s.bar(b); // should not lint
+            s.bar(&*b); // should not lint
+            s.bar(&**b); // should not lint
+        }
+    }
+}
+
+mod mutable {
+    fn foo(_: &mut usize) {}
+    fn bar<T>(_: T) {}
+
+    fn test_call() {
+        {
+            let mut b = Box::new(0);
+            foo(&mut b); // should not lint
+            foo(&mut b); // should lint
+            let b = &mut Box::new(0);
+            foo(b); // should not lint
+            foo(&mut *b); // should not lint
+            foo(b); // should lint
+        }
+        {
+            let mut b = Box::new(0);
+            bar(&mut b); // should not lint
+            bar(&mut *b); // should not lint
+            let b = &mut Box::new(0);
+            bar(b); // should not lint
+
+            let b = &mut Box::new(0);
+            bar(&mut *b); // should not lint
+
+            let b = &mut Box::new(0);
+            bar(&mut **b); // should not lint
+        }
+    }
+
+    struct S;
+    impl S {
+        fn foo(&self, _a: &mut usize) {}
+        fn bar<T>(&self, _a: T) {}
+    }
+
+    fn test_method_call() {
+        let s = S;
+        {
+            let mut b = Box::new(0);
+            s.foo(&mut b); // should not lint
+            s.foo(&mut b); // should lint
+            let b = &mut Box::new(0);
+            s.foo(b); // should not lint
+            s.foo(&mut *b); // should not lint
+            s.foo(b); // should lint
+        }
+        {
+            let mut b = Box::new(0);
+            s.bar(&mut b); // should not lint
+            s.bar(&mut *b); // should not lint
+            let b = &mut Box::new(0);
+            s.bar(b); // should not lint
+
+            let b = &mut Box::new(0);
+            s.bar(&mut *b); // should not lint
+
+            let b = &mut Box::new(0);
+            s.bar(&mut **b); // should not lint
+        }
+    }
+}
+
+mod code_bloating {
+    use std::fmt::Display;
+
+    fn main() {
+        let a = &String::new();
+        foo(a); // should lint
+        bar(&**a); // should not lint. Otherwise `bar` has to be generialized twice for `bar(&**a)`(T:&String) and `bar(b)`(T:&str)
+        // This is a code bloating problem.
+        let b: &str = "";
+        bar(b);
+    }
+
+    fn foo(_: &str) {}
+
+    fn bar<T: Display>(_: T) {}
+}