Auto merge of #7977 - Jarcho:multi_needless_borrow, r=xFrednet

Improve `needless_borrow` lint

fixes: #5327
fixes: #1726
fixes: #1212

This is merging `needless_borrow` into the `dereference` pass in preparation for `explicit_auto_deref`. `explicit_auto_deref` needs to implement most of what `needless_borrow` implements in order to work.

There is a minor regression here where `let x: &str = &x.deref()` will trigger `needless_borrow` without triggering `explicit_deref_methods`. Removing the redundant borrow will cause `explicit_deref_methods` to trigger. This will be fixed when `explicit_auto_deref` is implemented.

changelog: Lint `needless_borrow` when a borrow is auto-derefed more than once
changelog: Lint `needless_borrow` in the trailing expression of a block for a match arm

clippy_lints/src/bytecount.rs

@@ -74,10 +74,10 @@ impl<'tcx> LateLintPass<'tcx> for ByteCount {
                     if (p == sym::iter || p == sym!(iter_mut)) && args.len() == 1 {
                         &args[0]
                     } else {
-                        &filter_recv
+                        filter_recv
                     }
                 } else {
-                    &filter_recv
+                    filter_recv
                 };
                 let mut applicability = Applicability::MaybeIncorrect;
                 span_lint_and_sugg(

clippy_lints/src/dereference.rs

@@ -1,14 +1,20 @@
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
-use clippy_utils::source::snippet_with_context;
+use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
 use clippy_utils::ty::peel_mid_ty_refs;
-use clippy_utils::{get_parent_node, is_lint_allowed};
+use clippy_utils::{get_parent_expr, get_parent_node, is_lint_allowed, path_to_local};
-use rustc_ast::util::parser::PREC_PREFIX;
+use rustc_ast::util::parser::{PREC_POSTFIX, PREC_PREFIX};
+use rustc_data_structures::fx::FxIndexMap;
 use rustc_errors::Applicability;
-use rustc_hir::{BorrowKind, Expr, ExprKind, HirId, MatchSource, Mutability, Node, UnOp};
+use rustc_hir::{
+    BindingAnnotation, Body, BodyId, BorrowKind, Destination, Expr, ExprKind, HirId, MatchSource, Mutability, Node,
+    Pat, PatKind, UnOp,
+};
 use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow};
 use rustc_middle::ty::{self, Ty, TyCtxt, TyS, TypeckResults};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::{symbol::sym, Span};
+use std::iter;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -40,8 +46,64 @@ declare_clippy_lint! {
     "Explicit use of deref or deref_mut method while not in a method chain."
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for address of operations (`&`) that are going to
+    /// be dereferenced immediately by the compiler.
+    ///
+    /// ### Why is this bad?
+    /// Suggests that the receiver of the expression borrows
+    /// the expression.
+    ///
+    /// ### Example
+    /// ```rust
+    /// fn fun(_a: &i32) {}
+    ///
+    /// // Bad
+    /// let x: &i32 = &&&&&&5;
+    /// fun(&x);
+    ///
+    /// // Good
+    /// let x: &i32 = &5;
+    /// fun(x);
+    /// ```
+    #[clippy::version = "pre 1.29.0"]
+    pub NEEDLESS_BORROW,
+    style,
+    "taking a reference that is going to be automatically dereferenced"
+}
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `ref` bindings which create a reference to a reference.
+    ///
+    /// ### Why is this bad?
+    /// The address-of operator at the use site is clearer about the need for a reference.
+    ///
+    /// ### Example
+    /// ```rust
+    /// // Bad
+    /// let x = Some("");
+    /// if let Some(ref x) = x {
+    ///     // use `x` here
+    /// }
+    ///
+    /// // Good
+    /// let x = Some("");
+    /// if let Some(x) = x {
+    ///     // use `&x` here
+    /// }
+    /// ```
+    #[clippy::version = "1.54.0"]
+    pub REF_BINDING_TO_REFERENCE,
+    pedantic,
+    "`ref` binding to a reference"
+}
+
 impl_lint_pass!(Dereferencing => [
     EXPLICIT_DEREF_METHODS,
+    NEEDLESS_BORROW,
+    REF_BINDING_TO_REFERENCE,
 ]);
 
 #[derive(Default)]
@@ -52,6 +114,18 @@ pub struct Dereferencing {
     // expression. This is to store the id of that expression so it can be skipped when
     // `check_expr` is called for it.
     skip_expr: Option<HirId>,
+
+    /// The body the first local was found in. Used to emit lints when the traversal of the body has
+    /// been finished. Note we can't lint at the end of every body as they can be nested within each
+    /// other.
+    current_body: Option<BodyId>,
+    /// The list of locals currently being checked by the lint.
+    /// If the value is `None`, then the binding has been seen as a ref pattern, but is not linted.
+    /// This is needed for or patterns where one of the branches can be linted, but another can not
+    /// be.
+    ///
+    /// e.g. `m!(x) | Foo::Bar(ref x)`
+    ref_locals: FxIndexMap<HirId, Option<RefPat>>,
 }
 
 struct StateData {
@@ -68,6 +142,9 @@ enum State {
         ty_changed_count: usize,
         is_final_ufcs: bool,
     },
+    DerefedBorrow {
+        count: u32,
+    },
 }
 
 // A reference operation considered by this lint pass
@@ -77,13 +154,29 @@ enum RefOp {
     AddrOf,
 }
 
+struct RefPat {
+    /// Whether every usage of the binding is dereferenced.
+    always_deref: bool,
+    /// The spans of all the ref bindings for this local.
+    spans: Vec<Span>,
+    /// The applicability of this suggestion.
+    app: Applicability,
+    /// All the replacements which need to be made.
+    replacements: Vec<(Span, String)>,
+}
+
 impl<'tcx> LateLintPass<'tcx> for Dereferencing {
+    #[allow(clippy::too_many_lines)]
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         // Skip path expressions from deref calls. e.g. `Deref::deref(e)`
         if Some(expr.hir_id) == self.skip_expr.take() {
             return;
         }
 
+        if let Some(local) = path_to_local(expr) {
+            self.check_local_usage(cx, expr, local);
+        }
+
         // Stop processing sub expressions when a macro call is seen
         if expr.span.from_expansion() {
             if let Some((state, data)) = self.state.take() {
@@ -128,6 +221,48 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
                             },
                         ));
                     },
+                    RefOp::AddrOf => {
+                        // Find the number of times the borrow is auto-derefed.
+                        let mut iter = find_adjustments(cx.tcx, typeck, expr).iter();
+                        if let Some((i, adjust)) = iter.by_ref().enumerate().find_map(|(i, adjust)| {
+                            if !matches!(adjust.kind, Adjust::Deref(_)) {
+                                Some((i, adjust))
+                            } else if !adjust.target.is_ref() {
+                                // Add one to the number of references found.
+                                Some((i + 1, adjust))
+                            } else {
+                                None
+                            }
+                        }) {
+                            // Found two consecutive derefs. At least one can be removed.
+                            if i > 1 {
+                                let target_mut = iter::once(adjust)
+                                    .chain(iter)
+                                    .find_map(|adjust| match adjust.kind {
+                                        Adjust::Borrow(AutoBorrow::Ref(_, m)) => Some(m.into()),
+                                        _ => None,
+                                    })
+                                    // This default should never happen. Auto-deref always reborrows.
+                                    .unwrap_or(Mutability::Not);
+                                self.state = Some((
+                                    // Subtract one for the current borrow expression, and one to cover the last
+                                    // reference which can't be removed (it's either reborrowed, or needed for
+                                    // auto-deref to happen).
+                                    State::DerefedBorrow {
+                                        count:
+                                            // Truncation here would require more than a `u32::MAX` level reference. The compiler
+                                            // does not support this.
+                                            #[allow(clippy::cast_possible_truncation)]
+                                            { i as u32 - 2 }
+                                    },
+                                    StateData {
+                                        span: expr.span,
+                                        target_mut,
+                                    },
+                                ));
+                            }
+                        }
+                    },
                     _ => (),
                 }
             },
@@ -144,10 +279,80 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
                     data,
                 ));
             },
+            (Some((State::DerefedBorrow { count }, data)), RefOp::AddrOf) if count != 0 => {
+                self.state = Some((State::DerefedBorrow { count: count - 1 }, data));
+            },
 
             (Some((state, data)), _) => report(cx, expr, state, data),
         }
     }
+
+    fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
+        if let PatKind::Binding(BindingAnnotation::Ref, id, name, _) = pat.kind {
+            if let Some(opt_prev_pat) = self.ref_locals.get_mut(&id) {
+                // This binding id has been seen before. Add this pattern to the list of changes.
+                if let Some(prev_pat) = opt_prev_pat {
+                    if pat.span.from_expansion() {
+                        // Doesn't match the context of the previous pattern. Can't lint here.
+                        *opt_prev_pat = None;
+                    } else {
+                        prev_pat.spans.push(pat.span);
+                        prev_pat.replacements.push((
+                            pat.span,
+                            snippet_with_context(cx, name.span, pat.span.ctxt(), "..", &mut prev_pat.app)
+                                .0
+                                .into(),
+                        ));
+                    }
+                }
+                return;
+            }
+
+            if_chain! {
+                if !pat.span.from_expansion();
+                if let ty::Ref(_, tam, _) = *cx.typeck_results().pat_ty(pat).kind();
+                // only lint immutable refs, because borrowed `&mut T` cannot be moved out
+                if let ty::Ref(_, _, Mutability::Not) = *tam.kind();
+                then {
+                    let mut app = Applicability::MachineApplicable;
+                    let snip = snippet_with_context(cx, name.span, pat.span.ctxt(), "..", &mut app).0;
+                    self.current_body = self.current_body.or(cx.enclosing_body);
+                    self.ref_locals.insert(
+                        id,
+                        Some(RefPat {
+                            always_deref: true,
+                            spans: vec![pat.span],
+                            app,
+                            replacements: vec![(pat.span, snip.into())],
+                        }),
+                    );
+                }
+            }
+        }
+    }
+
+    fn check_body_post(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'_>) {
+        if Some(body.id()) == self.current_body {
+            for pat in self.ref_locals.drain(..).filter_map(|(_, x)| x) {
+                let replacements = pat.replacements;
+                let app = pat.app;
+                span_lint_and_then(
+                    cx,
+                    if pat.always_deref {
+                        NEEDLESS_BORROW
+                    } else {
+                        REF_BINDING_TO_REFERENCE
+                    },
+                    pat.spans,
+                    "this pattern creates a reference to a reference",
+                    |diag| {
+                        diag.multipart_suggestion("try this", replacements, app);
+                    },
+                );
+            }
+            self.current_body = None;
+        }
+    }
 }
 
 fn try_parse_ref_op(
@@ -251,6 +456,48 @@ fn is_linted_explicit_deref_position(parent: Option<Node<'_>>, child_id: HirId,
     }
 }
 
+/// Adjustments are sometimes made in the parent block rather than the expression itself.
+fn find_adjustments(
+    tcx: TyCtxt<'tcx>,
+    typeck: &'tcx TypeckResults<'_>,
+    expr: &'tcx Expr<'_>,
+) -> &'tcx [Adjustment<'tcx>] {
+    let map = tcx.hir();
+    let mut iter = map.parent_iter(expr.hir_id);
+    let mut prev = expr;
+
+    loop {
+        match typeck.expr_adjustments(prev) {
+            [] => (),
+            a => break a,
+        };
+
+        match iter.next().map(|(_, x)| x) {
+            Some(Node::Block(_)) => {
+                if let Some((_, Node::Expr(e))) = iter.next() {
+                    prev = e;
+                } else {
+                    // This shouldn't happen. Blocks are always contained in an expression.
+                    break &[];
+                }
+            },
+            Some(Node::Expr(&Expr {
+                kind: ExprKind::Break(Destination { target_id: Ok(id), .. }, _),
+                ..
+            })) => {
+                if let Some(Node::Expr(e)) = map.find(id) {
+                    prev = e;
+                    iter = map.parent_iter(id);
+                } else {
+                    // This shouldn't happen. The destination should exist.
+                    break &[];
+                }
+            },
+            _ => break &[],
+        }
+    }
+}
+
 #[allow(clippy::needless_pass_by_value)]
 fn report(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data: StateData) {
     match state {
@@ -301,5 +548,83 @@ fn report(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data: Stat
                 app,
             );
         },
+        State::DerefedBorrow { .. } => {
+            let mut app = Applicability::MachineApplicable;
+            let snip = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app).0;
+            span_lint_and_sugg(
+                cx,
+                NEEDLESS_BORROW,
+                data.span,
+                &format!(
+                    "this expression borrows a reference (`{}`) that is immediately dereferenced by the compiler",
+                    cx.typeck_results().expr_ty(expr),
+                ),
+                "change this to",
+                snip.into(),
+                app,
+            );
+        },
+    }
+}
+
+impl Dereferencing {
+    fn check_local_usage(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, local: HirId) {
+        if let Some(outer_pat) = self.ref_locals.get_mut(&local) {
+            if let Some(pat) = outer_pat {
+                // Check for auto-deref
+                if !matches!(
+                    cx.typeck_results().expr_adjustments(e),
+                    [
+                        Adjustment {
+                            kind: Adjust::Deref(_),
+                            ..
+                        },
+                        Adjustment {
+                            kind: Adjust::Deref(_),
+                            ..
+                        },
+                        ..
+                    ]
+                ) {
+                    match get_parent_expr(cx, e) {
+                        // Field accesses are the same no matter the number of references.
+                        Some(Expr {
+                            kind: ExprKind::Field(..),
+                            ..
+                        }) => (),
+                        Some(&Expr {
+                            span,
+                            kind: ExprKind::Unary(UnOp::Deref, _),
+                            ..
+                        }) if !span.from_expansion() => {
+                            // Remove explicit deref.
+                            let snip = snippet_with_context(cx, e.span, span.ctxt(), "..", &mut pat.app).0;
+                            pat.replacements.push((span, snip.into()));
+                        },
+                        Some(parent) if !parent.span.from_expansion() => {
+                            // Double reference might be needed at this point.
+                            if parent.precedence().order() == PREC_POSTFIX {
+                                // Parentheses would be needed here, don't lint.
+                                *outer_pat = None;
+                            } else {
+                                pat.always_deref = false;
+                                let snip = snippet_with_context(cx, e.span, parent.span.ctxt(), "..", &mut pat.app).0;
+                                pat.replacements.push((e.span, format!("&{}", snip)));
+                            }
+                        },
+                        _ if !e.span.from_expansion() => {
+                            // Double reference might be needed at this point.
+                            pat.always_deref = false;
+                            let snip = snippet_with_applicability(cx, e.span, "..", &mut pat.app);
+                            pat.replacements.push((e.span, format!("&{}", snip)));
+                        },
+                        // Edge case for macros. The span of the identifier will usually match the context of the
+                        // binding, but not if the identifier was created in a macro. e.g. `concat_idents` and proc
+                        // macros
+                        _ => *outer_pat = None,
+                    }
+                }
+            }
+        }
     }
 }

clippy_lints/src/lib.register_all.rs

@@ -33,6 +33,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(copies::IFS_SAME_COND),
     LintId::of(copies::IF_SAME_THEN_ELSE),
     LintId::of(default::FIELD_REASSIGN_WITH_DEFAULT),
+    LintId::of(dereference::NEEDLESS_BORROW),
     LintId::of(derivable_impls::DERIVABLE_IMPLS),
     LintId::of(derive::DERIVE_HASH_XOR_EQ),
     LintId::of(derive::DERIVE_ORD_XOR_PARTIAL_ORD),
@@ -204,7 +205,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE),
     LintId::of(needless_bool::BOOL_COMPARISON),
     LintId::of(needless_bool::NEEDLESS_BOOL),
-    LintId::of(needless_borrow::NEEDLESS_BORROW),
     LintId::of(needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),
     LintId::of(needless_option_as_deref::NEEDLESS_OPTION_AS_DEREF),
     LintId::of(needless_question_mark::NEEDLESS_QUESTION_MARK),

clippy_lints/src/lib.register_lints.rs

@@ -92,6 +92,8 @@ store.register_lints(&[
     default::FIELD_REASSIGN_WITH_DEFAULT,
     default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK,
     dereference::EXPLICIT_DEREF_METHODS,
+    dereference::NEEDLESS_BORROW,
+    dereference::REF_BINDING_TO_REFERENCE,
     derivable_impls::DERIVABLE_IMPLS,
     derive::DERIVE_HASH_XOR_EQ,
     derive::DERIVE_ORD_XOR_PARTIAL_ORD,
@@ -357,8 +359,6 @@ store.register_lints(&[
     needless_bitwise_bool::NEEDLESS_BITWISE_BOOL,
     needless_bool::BOOL_COMPARISON,
     needless_bool::NEEDLESS_BOOL,
-    needless_borrow::NEEDLESS_BORROW,
-    needless_borrow::REF_BINDING_TO_REFERENCE,
     needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE,
     needless_continue::NEEDLESS_CONTINUE,
     needless_for_each::NEEDLESS_FOR_EACH,

clippy_lints/src/lib.register_pedantic.rs

@@ -21,6 +21,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
     LintId::of(copy_iterator::COPY_ITERATOR),
     LintId::of(default::DEFAULT_TRAIT_ACCESS),
     LintId::of(dereference::EXPLICIT_DEREF_METHODS),
+    LintId::of(dereference::REF_BINDING_TO_REFERENCE),
     LintId::of(derive::EXPL_IMPL_CLONE_ON_COPY),
     LintId::of(derive::UNSAFE_DERIVE_DESERIALIZE),
     LintId::of(doc::DOC_MARKDOWN),
@@ -68,7 +69,6 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
     LintId::of(misc::USED_UNDERSCORE_BINDING),
     LintId::of(mut_mut::MUT_MUT),
     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_for_each::NEEDLESS_FOR_EACH),
     LintId::of(needless_pass_by_value::NEEDLESS_PASS_BY_VALUE),

clippy_lints/src/lib.register_style.rs

@@ -15,6 +15,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(collapsible_match::COLLAPSIBLE_MATCH),
     LintId::of(comparison_chain::COMPARISON_CHAIN),
     LintId::of(default::FIELD_REASSIGN_WITH_DEFAULT),
+    LintId::of(dereference::NEEDLESS_BORROW),
     LintId::of(doc::MISSING_SAFETY_DOC),
     LintId::of(doc::NEEDLESS_DOCTEST_MAIN),
     LintId::of(enum_variants::ENUM_VARIANT_NAMES),
@@ -81,7 +82,6 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(misc_early::REDUNDANT_PATTERN),
     LintId::of(mut_mutex_lock::MUT_MUTEX_LOCK),
     LintId::of(mut_reference::UNNECESSARY_MUT_PASSED),
-    LintId::of(needless_borrow::NEEDLESS_BORROW),
     LintId::of(neg_multiply::NEG_MULTIPLY),
     LintId::of(new_without_default::NEW_WITHOUT_DEFAULT),
     LintId::of(non_copy_const::BORROW_INTERIOR_MUTABLE_CONST),

clippy_lints/src/lib.rs

@@ -296,7 +296,6 @@ mod mutex_atomic;
 mod needless_arbitrary_self_type;
 mod needless_bitwise_bool;
 mod needless_bool;
-mod needless_borrow;
 mod needless_borrowed_ref;
 mod needless_continue;
 mod needless_for_each;
@@ -601,7 +600,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(zero_div_zero::ZeroDiv));
     store.register_late_pass(|| Box::new(mutex_atomic::Mutex));
     store.register_late_pass(|| Box::new(needless_update::NeedlessUpdate));
-    store.register_late_pass(|| Box::new(needless_borrow::NeedlessBorrow::default()));
     store.register_late_pass(|| Box::new(needless_borrowed_ref::NeedlessBorrowedRef));
     store.register_late_pass(|| Box::new(no_effect::NoEffect));
     store.register_late_pass(|| Box::new(temporary_assignment::TemporaryAssignment));

clippy_lints/src/needless_borrow.rs

@@ -1,284 +0,0 @@
-//! Checks for needless address of operations (`&`)
-//!
-//! This lint is **warn** by default
-
-use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::source::{snippet_opt, snippet_with_applicability, snippet_with_context};
-use clippy_utils::{get_parent_expr, path_to_local};
-use if_chain::if_chain;
-use rustc_ast::util::parser::PREC_POSTFIX;
-use rustc_data_structures::fx::FxIndexMap;
-use rustc_errors::Applicability;
-use rustc_hir::{BindingAnnotation, Body, BodyId, BorrowKind, Expr, ExprKind, HirId, Mutability, Pat, PatKind, UnOp};
-use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty;
-use rustc_middle::ty::adjustment::{Adjust, Adjustment};
-use rustc_session::{declare_tool_lint, impl_lint_pass};
-use rustc_span::Span;
-
-declare_clippy_lint! {
-    /// ### What it does
-    /// Checks for address of operations (`&`) that are going to
-    /// be dereferenced immediately by the compiler.
-    ///
-    /// ### Why is this bad?
-    /// Suggests that the receiver of the expression borrows
-    /// the expression.
-    ///
-    /// ### Example
-    /// ```rust
-    /// fn fun(_a: &i32) {}
-    ///
-    /// // Bad
-    /// let x: &i32 = &&&&&&5;
-    /// fun(&x);
-    ///
-    /// // Good
-    /// let x: &i32 = &5;
-    /// fun(x);
-    /// ```
-    #[clippy::version = "pre 1.29.0"]
-    pub NEEDLESS_BORROW,
-    style,
-    "taking a reference that is going to be automatically dereferenced"
-}
-
-declare_clippy_lint! {
-    /// ### What it does
-    /// Checks for `ref` bindings which create a reference to a reference.
-    ///
-    /// ### Why is this bad?
-    /// The address-of operator at the use site is clearer about the need for a reference.
-    ///
-    /// ### Example
-    /// ```rust
-    /// // Bad
-    /// let x = Some("");
-    /// if let Some(ref x) = x {
-    ///     // use `x` here
-    /// }
-    ///
-    /// // Good
-    /// let x = Some("");
-    /// if let Some(x) = x {
-    ///     // use `&x` here
-    /// }
-    /// ```
-    #[clippy::version = "1.54.0"]
-    pub REF_BINDING_TO_REFERENCE,
-    pedantic,
-    "`ref` binding to a reference"
-}
-
-impl_lint_pass!(NeedlessBorrow => [NEEDLESS_BORROW, REF_BINDING_TO_REFERENCE]);
-#[derive(Default)]
-pub struct NeedlessBorrow {
-    /// The body the first local was found in. Used to emit lints when the traversal of the body has
-    /// been finished. Note we can't lint at the end of every body as they can be nested within each
-    /// other.
-    current_body: Option<BodyId>,
-    /// The list of locals currently being checked by the lint.
-    /// If the value is `None`, then the binding has been seen as a ref pattern, but is not linted.
-    /// This is needed for or patterns where one of the branches can be linted, but another can not
-    /// be.
-    ///
-    /// e.g. `m!(x) | Foo::Bar(ref x)`
-    ref_locals: FxIndexMap<HirId, Option<RefPat>>,
-}
-
-struct RefPat {
-    /// Whether every usage of the binding is dereferenced.
-    always_deref: bool,
-    /// The spans of all the ref bindings for this local.
-    spans: Vec<Span>,
-    /// The applicability of this suggestion.
-    app: Applicability,
-    /// All the replacements which need to be made.
-    replacements: Vec<(Span, String)>,
-}
-
-impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow {
-    fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
-        if let Some(local) = path_to_local(e) {
-            self.check_local_usage(cx, e, local);
-        }
-
-        if e.span.from_expansion() {
-            return;
-        }
-        if let ExprKind::AddrOf(BorrowKind::Ref, mutability, inner) = e.kind {
-            if let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty(inner).kind() {
-                for adj3 in cx.typeck_results().expr_adjustments(e).windows(3) {
-                    if let [
-                        Adjustment {
-                            kind: Adjust::Deref(_), ..
-                        },
-                        Adjustment {
-                            kind: Adjust::Deref(_), ..
-                        },
-                        Adjustment {
-                            kind: Adjust::Borrow(_),
-                            ..
-                        },
-                    ] = *adj3
-                    {
-                        let help_msg_ty = if matches!(mutability, Mutability::Not) {
-                            format!("&{}", ty)
-                        } else {
-                            format!("&mut {}", ty)
-                        };
-
-                        span_lint_and_then(
-                            cx,
-                            NEEDLESS_BORROW,
-                            e.span,
-                            &format!(
-                                "this expression borrows a reference (`{}`) that is immediately dereferenced \
-                             by the compiler",
-                                help_msg_ty
-                            ),
-                            |diag| {
-                                if let Some(snippet) = snippet_opt(cx, inner.span) {
-                                    diag.span_suggestion(
-                                        e.span,
-                                        "change this to",
-                                        snippet,
-                                        Applicability::MachineApplicable,
-                                    );
-                                }
-                            },
-                        );
-                    }
-                }
-            }
-        }
-    }
-
-    fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
-        if let PatKind::Binding(BindingAnnotation::Ref, id, name, _) = pat.kind {
-            if let Some(opt_prev_pat) = self.ref_locals.get_mut(&id) {
-                // This binding id has been seen before. Add this pattern to the list of changes.
-                if let Some(prev_pat) = opt_prev_pat {
-                    if pat.span.from_expansion() {
-                        // Doesn't match the context of the previous pattern. Can't lint here.
-                        *opt_prev_pat = None;
-                    } else {
-                        prev_pat.spans.push(pat.span);
-                        prev_pat.replacements.push((
-                            pat.span,
-                            snippet_with_context(cx, name.span, pat.span.ctxt(), "..", &mut prev_pat.app)
-                                .0
-                                .into(),
-                        ));
-                    }
-                }
-                return;
-            }
-
-            if_chain! {
-                if !pat.span.from_expansion();
-                if let ty::Ref(_, tam, _) = *cx.typeck_results().pat_ty(pat).kind();
-                // only lint immutable refs, because borrowed `&mut T` cannot be moved out
-                if let ty::Ref(_, _, Mutability::Not) = *tam.kind();
-                then {
-                    let mut app = Applicability::MachineApplicable;
-                    let snip = snippet_with_context(cx, name.span, pat.span.ctxt(), "..", &mut app).0;
-                    self.current_body = self.current_body.or(cx.enclosing_body);
-                    self.ref_locals.insert(
-                        id,
-                        Some(RefPat {
-                            always_deref: true,
-                            spans: vec![pat.span],
-                            app,
-                            replacements: vec![(pat.span, snip.into())],
-                        }),
-                    );
-                }
-            }
-        }
-    }
-
-    fn check_body_post(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'_>) {
-        if Some(body.id()) == self.current_body {
-            for pat in self.ref_locals.drain(..).filter_map(|(_, x)| x) {
-                let replacements = pat.replacements;
-                let app = pat.app;
-                span_lint_and_then(
-                    cx,
-                    if pat.always_deref {
-                        NEEDLESS_BORROW
-                    } else {
-                        REF_BINDING_TO_REFERENCE
-                    },
-                    pat.spans,
-                    "this pattern creates a reference to a reference",
-                    |diag| {
-                        diag.multipart_suggestion("try this", replacements, app);
-                    },
-                );
-            }
-            self.current_body = None;
-        }
-    }
-}
-impl NeedlessBorrow {
-    fn check_local_usage(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, local: HirId) {
-        if let Some(outer_pat) = self.ref_locals.get_mut(&local) {
-            if let Some(pat) = outer_pat {
-                // Check for auto-deref
-                if !matches!(
-                    cx.typeck_results().expr_adjustments(e),
-                    [
-                        Adjustment {
-                            kind: Adjust::Deref(_),
-                            ..
-                        },
-                        Adjustment {
-                            kind: Adjust::Deref(_),
-                            ..
-                        },
-                        ..
-                    ]
-                ) {
-                    match get_parent_expr(cx, e) {
-                        // Field accesses are the same no matter the number of references.
-                        Some(Expr {
-                            kind: ExprKind::Field(..),
-                            ..
-                        }) => (),
-                        Some(&Expr {
-                            span,
-                            kind: ExprKind::Unary(UnOp::Deref, _),
-                            ..
-                        }) if !span.from_expansion() => {
-                            // Remove explicit deref.
-                            let snip = snippet_with_context(cx, e.span, span.ctxt(), "..", &mut pat.app).0;
-                            pat.replacements.push((span, snip.into()));
-                        },
-                        Some(parent) if !parent.span.from_expansion() => {
-                            // Double reference might be needed at this point.
-                            if parent.precedence().order() == PREC_POSTFIX {
-                                // Parentheses would be needed here, don't lint.
-                                *outer_pat = None;
-                            } else {
-                                pat.always_deref = false;
-                                let snip = snippet_with_context(cx, e.span, parent.span.ctxt(), "..", &mut pat.app).0;
-                                pat.replacements.push((e.span, format!("&{}", snip)));
-                            }
-                        },
-                        _ if !e.span.from_expansion() => {
-                            // Double reference might be needed at this point.
-                            pat.always_deref = false;
-                            let snip = snippet_with_applicability(cx, e.span, "..", &mut pat.app);
-                            pat.replacements.push((e.span, format!("&{}", snip)));
-                        },
-                        // Edge case for macros. The span of the identifier will usually match the context of the
-                        // binding, but not if the identifier was created in a macro. e.g. `concat_idents` and proc
-                        // macros
-                        _ => *outer_pat = None,
-                    }
-                }
-            }
-        }
-    }
-}

tests/ui/needless_borrow.fixed

@@ -1,9 +1,10 @@
 // run-rustfix
 
 #[warn(clippy::all, clippy::needless_borrow)]
-#[allow(unused_variables)]
+#[allow(unused_variables, clippy::unnecessary_mut_passed)]
 fn main() {
     let a = 5;
+    let ref_a = &a;
     let _ = x(&a); // no warning
     let _ = x(&a); // warn
 
@@ -21,11 +22,29 @@ fn main() {
         44 => &a,
         45 => {
             println!("foo");
-            &&a // FIXME: this should lint, too
+            &a
         },
         46 => &a,
+        47 => {
+            println!("foo");
+            loop {
+                println!("{}", a);
+                if a == 25 {
+                    break ref_a;
+                }
+            }
+        },
         _ => panic!(),
     };
+
+    let _ = x(&a);
+    let _ = x(&a);
+    let _ = x(&mut b);
+    let _ = x(ref_a);
+    {
+        let b = &mut b;
+        x(b);
+    }
 }
 
 #[allow(clippy::needless_borrowed_reference)]

tests/ui/needless_borrow.rs

@@ -1,9 +1,10 @@
 // run-rustfix
 
 #[warn(clippy::all, clippy::needless_borrow)]
-#[allow(unused_variables)]
+#[allow(unused_variables, clippy::unnecessary_mut_passed)]
 fn main() {
     let a = 5;
+    let ref_a = &a;
     let _ = x(&a); // no warning
     let _ = x(&&a); // warn
 
@@ -21,11 +22,29 @@ fn main() {
         44 => &a,
         45 => {
             println!("foo");
-            &&a // FIXME: this should lint, too
+            &&a
         },
         46 => &&a,
+        47 => {
+            println!("foo");
+            loop {
+                println!("{}", a);
+                if a == 25 {
+                    break &ref_a;
+                }
+            }
+        },
         _ => panic!(),
     };
+
+    let _ = x(&&&a);
+    let _ = x(&mut &&a);
+    let _ = x(&&&mut b);
+    let _ = x(&&ref_a);
+    {
+        let b = &mut b;
+        x(&b);
+    }
 }
 
 #[allow(clippy::needless_borrowed_reference)]

tests/ui/needless_borrow.stderr

@@ -1,5 +1,5 @@
 error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler
-  --> $DIR/needless_borrow.rs:8:15
+  --> $DIR/needless_borrow.rs:9:15
    |
 LL |     let _ = x(&&a); // warn
    |               ^^^ help: change this to: `&a`
@@ -7,16 +7,58 @@ LL |     let _ = x(&&a); // warn
    = note: `-D clippy::needless-borrow` implied by `-D warnings`
 
 error: this expression borrows a reference (`&mut i32`) that is immediately dereferenced by the compiler
-  --> $DIR/needless_borrow.rs:12:13
+  --> $DIR/needless_borrow.rs:13:13
    |
 LL |     mut_ref(&mut &mut b); // warn
    |             ^^^^^^^^^^^ help: change this to: `&mut b`
 
 error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler
-  --> $DIR/needless_borrow.rs:26:15
+  --> $DIR/needless_borrow.rs:25:13
+   |
+LL |             &&a
+   |             ^^^ help: change this to: `&a`
+
+error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler
+  --> $DIR/needless_borrow.rs:27:15
    |
 LL |         46 => &&a,
    |               ^^^ help: change this to: `&a`
 
-error: aborting due to 3 previous errors
+error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler
+  --> $DIR/needless_borrow.rs:33:27
+   |
+LL |                     break &ref_a;
+   |                           ^^^^^^ help: change this to: `ref_a`
+
+error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler
+  --> $DIR/needless_borrow.rs:40:15
+   |
+LL |     let _ = x(&&&a);
+   |               ^^^^ help: change this to: `&a`
+
+error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler
+  --> $DIR/needless_borrow.rs:41:15
+   |
+LL |     let _ = x(&mut &&a);
+   |               ^^^^^^^^ help: change this to: `&a`
+
+error: this expression borrows a reference (`&mut i32`) that is immediately dereferenced by the compiler
+  --> $DIR/needless_borrow.rs:42:15
+   |
+LL |     let _ = x(&&&mut b);
+   |               ^^^^^^^^ help: change this to: `&mut b`
+
+error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler
+  --> $DIR/needless_borrow.rs:43:15
+   |
+LL |     let _ = x(&&ref_a);
+   |               ^^^^^^^ help: change this to: `ref_a`
+
+error: this expression borrows a reference (`&mut i32`) that is immediately dereferenced by the compiler
+  --> $DIR/needless_borrow.rs:46:11
+   |
+LL |         x(&b);
+   |           ^^ help: change this to: `b`
+
+error: aborting due to 10 previous errors