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,113 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_opt;
+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 parameter.
+    ///
+    /// ### 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(_, _, 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{
+                        span_lint_and_sugg(
+                            cx,
+                            NEEDLESS_DEREF,
+                            argument.span,
+                            "needless explicit deref in function parameter",
+                            "try remove the `&**` and just keep",
+                            snippet_opt(cx, child3.span).unwrap(),
+                            Applicability::MachineApplicable,
+                        );
+                    }
+                }
+
+                // a: T
+                // foo(&*a) -> foo(&a)
+                if_chain! {
+                    if let ExprKind::AddrOf(_, _, 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{
+                        span_lint_and_sugg(
+                            cx,
+                            NEEDLESS_DEREF,
+                            argument.span,
+                            "needless explicit deref in function parameter",
+                            "you can replace this with",
+                            ("&".to_owned()+&snippet_opt(cx, child2.span).unwrap()).clone(),
+                            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.rs

@@ -0,0 +1,85 @@
+#![warn(clippy::needless_deref)]
+
+use std::path::Path;
+use std::path::PathBuf;
+use std::sync::Arc;
+
+fn main() {
+    test_call();
+    test_method_call();
+}
+
+// Arc<PathBuf> -> PathBuf -> Path
+fn test_call() {
+    fn foo(_: &Path) {}
+    fn bar<T>(_: T) {}
+
+    {
+        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
+    }
+}
+
+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
+    }
+}

tests/ui/needless_deref.stderr

@@ -0,0 +1,28 @@
+error: needless explicit deref in function parameter
+  --> $DIR/needless_deref.rs:24:13
+   |
+LL |         foo(&*PathBuf::new()); // should lint
+   |             ^^^^^^^^^^^^^^^^ help: you can replace this with: `&PathBuf::new()`
+   |
+   = note: `-D clippy::needless-deref` implied by `-D warnings`
+
+error: needless explicit deref in function parameter
+  --> $DIR/needless_deref.rs:29:13
+   |
+LL |         foo(&**b); // should lint
+   |             ^^^^ help: try remove the `&**` and just keep: `b`
+
+error: needless explicit deref in function parameter
+  --> $DIR/needless_deref.rs:63:15
+   |
+LL |         s.foo(&*PathBuf::new()); // should lint
+   |               ^^^^^^^^^^^^^^^^ help: you can replace this with: `&PathBuf::new()`
+
+error: needless explicit deref in function parameter
+  --> $DIR/needless_deref.rs:68:15
+   |
+LL |         s.foo(&**b); // should lint
+   |               ^^^^ help: try remove the `&**` and just keep: `b`
+
+error: aborting due to 4 previous errors
+