when the expr can't be deref, we should not give incorrect help message

Author: lengyijun

clippy_lints/src/needless_deref.rs

@@ -1,5 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::snippet_opt;
+use clippy_utils::ty::implements_trait;
 use rustc_hir::{ExprKind, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::mir::Mutability;
@@ -41,11 +42,11 @@ impl LateLintPass<'_> for NeedlessDeref {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx rustc_hir::Expr<'_>) {
         if_chain! {
             if !e.span.from_expansion();
-            if let ExprKind::AddrOf(_, Mutability::Not, addrof_target) = e.kind;
-            if let ExprKind::Unary(UnOp::Deref, deref_target) = addrof_target.kind;
-            if !matches!(deref_target.kind, ExprKind::Unary(UnOp::Deref, ..) );
-            let inner_ty = cx.typeck_results().expr_ty(deref_target);
-            if let ty::Ref(_, _, Mutability::Not) = inner_ty.kind();
+            if let ExprKind::AddrOf(_, Mutability::Not, addrof_expr) = e.kind;
+            if let ExprKind::Unary(UnOp::Deref, deref_expr) = addrof_expr.kind;
+            if !matches!(deref_expr.kind, ExprKind::Unary(UnOp::Deref, ..) );
+            let ref_ty = cx.typeck_results().expr_ty(deref_expr);
+            if let ty::Ref(_, inner_ty, Mutability::Not) = ref_ty.kind();
             then{
 
                 let map = cx.tcx.hir();
@@ -54,7 +55,7 @@ impl LateLintPass<'_> for NeedlessDeref {
                     if span.from_expansion() {
                         return;
                     }
-                    if matches!(deref_target.kind, ExprKind::Path(..) ){
+                    if matches!(deref_expr.kind, ExprKind::Path(..) ){
                         let parent_node = map.find(parent_hir_id);
                         if let Some(rustc_hir::Node::Expr(parent_expr)) = parent_node {
                             if matches!(parent_expr.kind, ExprKind::Unary(UnOp::Deref, ..)) ||
@@ -65,26 +66,55 @@ impl LateLintPass<'_> for NeedlessDeref {
                     }
                 }
 
-                span_lint_and_then(
-                    cx,
-                    NEEDLESS_DEREF,
-                    e.span,
-                    "deref on an immutable reference",
-                    |diag| {
-                        diag.help(
-                            &format!(
-                                "consider using `{}` if you would like to deref",
-                                "&**".to_owned() + &snippet_opt(cx, deref_target.span).unwrap(),
-                             )
-                        );
-                        diag.help(
-                            &format!(
-                                "consider using `{}` if you would like to reborrow",
-                                &snippet_opt(cx, deref_target.span).unwrap(),
-                             )
-                        );
+                let mut give_2_help = true;
+
+                // if has deref trait, give 2 help
+                // if has no deref trait, give 1 help
+                if let Some(deref_trait_id) = cx.tcx.lang_items().deref_trait(){
+                    if !implements_trait(cx, inner_ty, deref_trait_id, &[]) {
+                         give_2_help = false;
                     }
-                );
+                }
+
+                if give_2_help {
+                    span_lint_and_then(
+                        cx,
+                        NEEDLESS_DEREF,
+                        e.span,
+                        "deref on an immutable reference",
+                        |diag| {
+                            diag.help(
+                                &format!(
+                                    "consider using `{}` if you would like to deref",
+                                    "&**".to_owned() + &snippet_opt(cx, deref_expr.span).unwrap(),
+                                 )
+                            );
+                            diag.help(
+                                &format!(
+                                    "consider using `{}` if you would like to reborrow",
+                                    &snippet_opt(cx, deref_expr.span).unwrap(),
+                                 )
+                            );
+                        }
+                    );
+                }else {
+                    span_lint_and_then(
+                        cx,
+                        NEEDLESS_DEREF,
+                        e.span,
+                        "deref on an immutable reference",
+                        |diag| {
+                            diag.help(
+                                &format!(
+                                    "consider using `{}` if you would like to reborrow",
+                                    &snippet_opt(cx, deref_expr.span).unwrap(),
+                                 )
+                            );
+                        }
+                    );
+                }
+
+
             }
         }
     }

tests/ui/needless_deref.rs

@@ -3,7 +3,7 @@ fn main() {}
 mod should_lint {
     fn foo() {
         let a = &12;
-        let _b = &*a;
+        let b = &*a;
 
         let s = &String::new();
         let a: &str = &*s;

tests/ui/needless_deref.stderr

@@ -1,11 +1,10 @@
 error: deref on an immutable reference
-  --> $DIR/needless_deref.rs:6:18
+  --> $DIR/needless_deref.rs:6:17
    |
-LL |         let _b = &*a;
-   |                  ^^^
+LL |         let b = &*a;
+   |                 ^^^
    |
    = note: `-D clippy::needless-deref` implied by `-D warnings`
-   = help: consider using `&**a` if you would like to deref
    = help: consider using `a` if you would like to reborrow
 
 error: deref on an immutable reference
@@ -23,7 +22,6 @@ error: deref on an immutable reference
 LL |         let b = &mut &*bar(a);
    |                      ^^^^^^^^
    |
-   = help: consider using `&**bar(a)` if you would like to deref
    = help: consider using `bar(a)` if you would like to reborrow
 
 error: aborting due to 3 previous errors