Lint `explicit_auto_deref` immediately after `needless_borrow`

diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs
index 32d1e26b74cf6c3b747ab8e6db5da5f773a7f35e..cac99c21e9e459f731b8d9b2130e5d877ece52ff 100644 (file)
--- a/clippy_lints/src/dereference.rs
+++ b/clippy_lints/src/dereference.rs
@@ -163,6 +163,14 @@ struct StateData {
     hir_id: HirId,
 }
 
+struct DerefedBorrow {
+    count: usize,
+    required_precedence: i8,
+    msg: &'static str,
+    stability: AutoDerefStability,
+    position: Position,
+}
+
 enum State {
     // Any number of deref method calls.
     DerefMethod {
@@ -172,11 +180,7 @@ enum State {
         /// The required mutability
         target_mut: Mutability,
     },
-    DerefedBorrow {
-        count: usize,
-        required_precedence: i8,
-        msg: &'static str,
-    },
+    DerefedBorrow(DerefedBorrow),
     ExplicitDeref {
         deref_span: Span,
         deref_hir_id: HirId,
@@ -344,17 +348,16 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
 
                         if deref_count >= required_refs {
                             self.state = Some((
-                                State::DerefedBorrow {
+                                State::DerefedBorrow(DerefedBorrow {
                                     // One of the required refs is for the current borrow expression, the remaining ones
                                     // can't be removed without breaking the code. See earlier comment.
                                     count: deref_count - required_refs,
                                     required_precedence,
                                     msg,
-                                },
-                                StateData {
-                                    span: expr.span,
-                                    hir_id: expr.hir_id,
-                                },
+                                    stability,
+                                    position,
+                                }),
+                                StateData { span: expr.span, hir_id: expr.hir_id },
                             ));
                         } else if stability.is_deref_stable() {
                             self.state = Some((
@@ -393,26 +396,47 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                     data,
                 ));
             },
-            (
-                Some((
-                    State::DerefedBorrow {
-                        count,
-                        required_precedence,
-                        msg,
-                    },
-                    data,
-                )),
-                RefOp::AddrOf,
-            ) if count != 0 => {
+            (Some((State::DerefedBorrow(state), data)), RefOp::AddrOf) if state.count != 0 => {
                 self.state = Some((
-                    State::DerefedBorrow {
-                        count: count - 1,
-                        required_precedence,
-                        msg,
-                    },
+                    State::DerefedBorrow(DerefedBorrow {
+                        count: state.count - 1,
+                        ..state
+                    }),
                     data,
                 ));
             },
+            (Some((State::DerefedBorrow(state), data)), RefOp::AddrOf) => {
+                let stability = state.stability;
+                report(cx, expr, State::DerefedBorrow(state), data);
+                if stability.is_deref_stable() {
+                    self.state = Some((
+                        State::Borrow,
+                        StateData {
+                            span: expr.span,
+                            hir_id: expr.hir_id,
+                        },
+                    ));
+                }
+            },
+            (Some((State::DerefedBorrow(state), data)), RefOp::Deref) => {
+                let stability = state.stability;
+                let position = state.position;
+                report(cx, expr, State::DerefedBorrow(state), data);
+                if let Position::FieldAccess(name) = position
+                    && !ty_contains_field(typeck.expr_ty(sub_expr), name)
+                {
+                    self.state = Some((
+                        State::ExplicitDerefField { name },
+                        StateData { span: expr.span, hir_id: expr.hir_id },
+                    ));
+                } else if stability.is_deref_stable() {
+                    self.state = Some((
+                        State::ExplicitDeref { deref_span: expr.span, deref_hir_id: expr.hir_id },
+                        StateData { span: expr.span, hir_id: expr.hir_id },
+                    ));
+                }
+            },
+
             (Some((State::Borrow, data)), RefOp::Deref) => {
                 if typeck.expr_ty(sub_expr).is_ref() {
                     self.state = Some((
@@ -942,15 +966,11 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data
                 app,
             );
         },
-        State::DerefedBorrow {
-            required_precedence,
-            msg,
-            ..
-        } => {
+        State::DerefedBorrow(state) => {
             let mut app = Applicability::MachineApplicable;
             let snip = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app).0;
-            span_lint_hir_and_then(cx, NEEDLESS_BORROW, data.hir_id, data.span, msg, |diag| {
-                let sugg = if required_precedence > expr.precedence().order() && !has_enclosing_paren(&snip) {
+            span_lint_hir_and_then(cx, NEEDLESS_BORROW, data.hir_id, data.span, state.msg, |diag| {
+                let sugg = if state.required_precedence > expr.precedence().order() && !has_enclosing_paren(&snip) {
                     format!("({})", snip)
                 } else {
                     snip.into()

diff --git a/tests/ui/explicit_auto_deref.fixed b/tests/ui/explicit_auto_deref.fixed
index d20a58e53bd1259ba0b397e4184b977600d7509e..f534714bc659ae6d9f51bc3a5e5ee11f2edfa4b0 100644 (file)
--- a/tests/ui/explicit_auto_deref.fixed
+++ b/tests/ui/explicit_auto_deref.fixed
@@ -198,4 +198,7 @@ fn main() {
             return *x;
         }
     }
+
+    f_str(&&ref_str); // `needless_borrow` will suggest removing both references
+    f_str(&ref_str); // `needless_borrow` will suggest removing only one reference
 }

diff --git a/tests/ui/explicit_auto_deref.rs b/tests/ui/explicit_auto_deref.rs
index f49bc8bb9f0120307a9c4de899a54de620d61b23..4781962882bcb7c091bdb9ac158c7d93ad0e2c44 100644 (file)
--- a/tests/ui/explicit_auto_deref.rs
+++ b/tests/ui/explicit_auto_deref.rs
@@ -198,4 +198,7 @@ fn _f5(x: &u32) -> u32 {
             return *x;
         }
     }
+
+    f_str(&&*ref_str); // `needless_borrow` will suggest removing both references
+    f_str(&&**ref_str); // `needless_borrow` will suggest removing only one reference
 }

diff --git a/tests/ui/explicit_auto_deref.stderr b/tests/ui/explicit_auto_deref.stderr
index de8c40ce5be1d4fe71b29a1f4bbdd2edd19eaef7..26357ffd96a0fdfbc5d174816875ac9226ce6586 100644 (file)
--- a/tests/ui/explicit_auto_deref.stderr
+++ b/tests/ui/explicit_auto_deref.stderr
@@ -180,5 +180,17 @@ error: deref which would be done by auto-deref
 LL |     let _ = f_str(**ref_ref_str);
    |                   ^^^^^^^^^^^^^ help: try this: `ref_ref_str`
 
-error: aborting due to 30 previous errors
+error: deref which would be done by auto-deref
+  --> $DIR/explicit_auto_deref.rs:201:13
+   |
+LL |     f_str(&&*ref_str); // `needless_borrow` will suggest removing both references
+   |             ^^^^^^^^ help: try this: `ref_str`
+
+error: deref which would be done by auto-deref
+  --> $DIR/explicit_auto_deref.rs:202:12
+   |
+LL |     f_str(&&**ref_str); // `needless_borrow` will suggest removing only one reference
+   |            ^^^^^^^^^^ help: try this: `ref_str`
+
+error: aborting due to 32 previous errors