Merge commit 'a98e7ab8b94485be6bd03e0c6b8682ecab5b52e6' into clippyup

CHANGELOG.md

@@ -981,7 +981,7 @@ Released 2021-03-25
   [#6532](https://github.com/rust-lang/rust-clippy/pull/6532)
 * [`single_match`] Suggest `if` over `if let` when possible
   [#6574](https://github.com/rust-lang/rust-clippy/pull/6574)
-* [`ref_in_deref`] Use parentheses correctly in suggestion
+* `ref_in_deref` Use parentheses correctly in suggestion
   [#6609](https://github.com/rust-lang/rust-clippy/pull/6609)
 * [`stable_sort_primitive`] Clarify error message
   [#6611](https://github.com/rust-lang/rust-clippy/pull/6611)
@@ -3227,7 +3227,6 @@ Released 2018-09-13
 [`redundant_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing
 [`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
 [`ref_binding_to_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_binding_to_reference
-[`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref
 [`ref_option_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_option_ref
 [`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro
 [`repeat_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_once

clippy_dev/src/bless.rs

@@ -9,9 +9,14 @@ use walkdir::WalkDir;
 
 use crate::clippy_project_root;
 
+#[cfg(not(windows))]
+static CARGO_CLIPPY_EXE: &str = "cargo-clippy";
+#[cfg(windows)]
+static CARGO_CLIPPY_EXE: &str = "cargo-clippy.exe";
+
 static CLIPPY_BUILD_TIME: SyncLazy<Option<std::time::SystemTime>> = SyncLazy::new(|| {
     let mut path = std::env::current_exe().unwrap();
-    path.set_file_name("cargo-clippy");
+    path.set_file_name(CARGO_CLIPPY_EXE);
     fs::metadata(path).ok()?.modified().ok()
 });
 

clippy_dev/src/lint.rs

@@ -7,7 +7,6 @@ pub fn run(filename: &str) {
         .args(["-Z", "no-codegen"])
         .args(["--edition", "2021"])
         .arg(filename)
-        .env("__CLIPPY_INTERNAL_TESTS", "true")
         .status()
         .expect("failed to run cargo")
         .code();

clippy_lints/src/borrow_as_ptr.rs

@@ -94,4 +94,6 @@ impl<'tcx> LateLintPass<'tcx> for BorrowAsPtr {
             }
         }
     }
+
+    extract_msrv_attr!(LateContext);
 }

clippy_lints/src/casts/unnecessary_cast.rs

@@ -23,15 +23,14 @@ pub(super) fn check(
 
         if_chain! {
             if let LitKind::Int(n, _) = lit.node;
-            if let Some(src) = snippet_opt(cx, lit.span);
+            if let Some(src) = snippet_opt(cx, cast_expr.span);
             if cast_to.is_floating_point();
             if let Some(num_lit) = NumericLiteral::from_lit_kind(&src, &lit.node);
             let from_nbits = 128 - n.leading_zeros();
             let to_nbits = fp_ty_mantissa_nbits(cast_to);
             if from_nbits != 0 && to_nbits != 0 && from_nbits <= to_nbits && num_lit.is_decimal();
             then {
-                let literal_str = if is_unary_neg(cast_expr) { format!("-{}", num_lit.integer) } else { num_lit.integer.into() };
-                lint_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to);
+                lint_unnecessary_cast(cx, expr, num_lit.integer, cast_from, cast_to);
                 return true
             }
         }
@@ -48,7 +47,7 @@ pub(super) fn check(
             | LitKind::Float(_, LitFloatType::Suffixed(_))
                 if cast_from.kind() == cast_to.kind() =>
             {
-                if let Some(src) = snippet_opt(cx, lit.span) {
+                if let Some(src) = snippet_opt(cx, cast_expr.span) {
                     if let Some(num_lit) = NumericLiteral::from_lit_kind(&src, &lit.node) {
                         lint_unnecessary_cast(cx, expr, num_lit.integer, cast_from, cast_to);
                     }
@@ -113,7 +112,3 @@ fn fp_ty_mantissa_nbits(typ: Ty<'_>) -> u32 {
         _ => 0,
     }
 }
-
-fn is_unary_neg(expr: &Expr<'_>) -> bool {
-    matches!(expr.kind, ExprKind::Unary(UnOp::Neg, _))
-}

clippy_lints/src/dereference.rs

@@ -1,5 +1,6 @@
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
+use clippy_utils::sugg::has_enclosing_paren;
 use clippy_utils::ty::peel_mid_ty_refs;
 use clippy_utils::{get_parent_expr, get_parent_node, is_lint_allowed, path_to_local};
 use rustc_ast::util::parser::{PREC_POSTFIX, PREC_PREFIX};
@@ -10,11 +11,10 @@ use rustc_hir::{
     Pat, PatKind, UnOp,
 };
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow};
+use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
 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
@@ -131,8 +131,6 @@ pub struct Dereferencing {
 struct StateData {
     /// Span of the top level expression
     span: Span,
-    /// The required mutability
-    target_mut: Mutability,
 }
 
 enum State {
@@ -141,9 +139,13 @@ enum State {
         // The number of calls in a sequence which changed the referenced type
         ty_changed_count: usize,
         is_final_ufcs: bool,
+        /// The required mutability
+        target_mut: Mutability,
     },
     DerefedBorrow {
-        count: u32,
+        count: usize,
+        required_precedence: i8,
+        msg: &'static str,
     },
 }
 
@@ -214,59 +216,98 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
                                     1
                                 },
                                 is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)),
-                            },
-                            StateData {
-                                span: expr.span,
                                 target_mut,
                             },
+                            StateData { span: expr.span },
                         ));
                     },
                     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))
+                        let mut deref_count = 0usize;
+                        let next_adjust = loop {
+                            match iter.next() {
+                                Some(adjust) => {
+                                    if !matches!(adjust.kind, Adjust::Deref(_)) {
+                                        break Some(adjust);
+                                    } else if !adjust.target.is_ref() {
+                                        deref_count += 1;
+                                        break iter.next();
+                                    }
+                                    deref_count += 1;
+                                },
+                                None => break None,
+                            };
+                        };
+
+                        // Determine the required number of references before any can be removed. In all cases the
+                        // reference made by the current expression will be removed. After that there are four cases to
+                        // handle.
+                        //
+                        // 1. Auto-borrow will trigger in the current position, so no further references are required.
+                        // 2. Auto-deref ends at a reference, or the underlying type, so one extra needs to be left to
+                        //    handle the automatically inserted re-borrow.
+                        // 3. Auto-deref hits a user-defined `Deref` impl, so at least one reference needs to exist to
+                        //    start auto-deref.
+                        // 4. If the chain of non-user-defined derefs ends with a mutable re-borrow, and re-borrow
+                        //    adjustments will not be inserted automatically, then leave one further reference to avoid
+                        //    moving a mutable borrow.
+                        //    e.g.
+                        //        fn foo<T>(x: &mut Option<&mut T>, y: &mut T) {
+                        //            let x = match x {
+                        //                // Removing the borrow will cause `x` to be moved
+                        //                Some(x) => &mut *x,
+                        //                None => y
+                        //            };
+                        //        }
+                        let deref_msg =
+                            "this expression creates a reference which is immediately dereferenced by the compiler";
+                        let borrow_msg = "this expression borrows a value the compiler would automatically borrow";
+
+                        let (required_refs, required_precedence, msg) = if is_auto_borrow_position(parent, expr.hir_id)
+                        {
+                            (1, PREC_POSTFIX, if deref_count == 1 { borrow_msg } else { deref_msg })
+                        } else if let Some(&Adjust::Borrow(AutoBorrow::Ref(_, mutability))) =
+                            next_adjust.map(|a| &a.kind)
+                        {
+                            if matches!(mutability, AutoBorrowMutability::Mut { .. })
+                                && !is_auto_reborrow_position(parent)
+                            {
+                                (3, 0, deref_msg)
                             } 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,
-                                    },
-                                ));
+                                (2, 0, deref_msg)
                             }
+                        } else {
+                            (2, 0, deref_msg)
+                        };
+
+                        if deref_count >= required_refs {
+                            self.state = Some((
+                                State::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 },
+                            ));
                         }
                     },
                     _ => (),
                 }
             },
-            (Some((State::DerefMethod { ty_changed_count, .. }, data)), RefOp::Method(_)) => {
+            (
+                Some((
+                    State::DerefMethod {
+                        target_mut,
+                        ty_changed_count,
+                        ..
+                    },
+                    data,
+                )),
+                RefOp::Method(_),
+            ) => {
                 self.state = Some((
                     State::DerefMethod {
                         ty_changed_count: if deref_method_same_type(typeck.expr_ty(expr), typeck.expr_ty(sub_expr)) {
@@ -275,12 +316,30 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
                             ty_changed_count + 1
                         },
                         is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)),
+                        target_mut,
                     },
                     data,
                 ));
             },
-            (Some((State::DerefedBorrow { count }, data)), RefOp::AddrOf) if count != 0 => {
-                self.state = Some((State::DerefedBorrow { count: count - 1 }, data));
+            (
+                Some((
+                    State::DerefedBorrow {
+                        count,
+                        required_precedence,
+                        msg,
+                    },
+                    data,
+                )),
+                RefOp::AddrOf,
+            ) if count != 0 => {
+                self.state = Some((
+                    State::DerefedBorrow {
+                        count: count - 1,
+                        required_precedence,
+                        msg,
+                    },
+                    data,
+                ));
             },
 
             (Some((state, data)), _) => report(cx, expr, state, data),
@@ -455,6 +514,30 @@ fn is_linted_explicit_deref_position(parent: Option<Node<'_>>, child_id: HirId,
     }
 }
 
+/// Checks if the given expression is in a position which can be auto-reborrowed.
+/// Note: This is only correct assuming auto-deref is already occurring.
+fn is_auto_reborrow_position(parent: Option<Node<'_>>) -> bool {
+    match parent {
+        Some(Node::Expr(parent)) => matches!(parent.kind, ExprKind::MethodCall(..) | ExprKind::Call(..)),
+        Some(Node::Local(_)) => true,
+        _ => false,
+    }
+}
+
+/// Checks if the given expression is a position which can auto-borrow.
+fn is_auto_borrow_position(parent: Option<Node<'_>>, child_id: HirId) -> bool {
+    if let Some(Node::Expr(parent)) = parent {
+        match parent.kind {
+            ExprKind::MethodCall(_, [self_arg, ..], _) => self_arg.hir_id == child_id,
+            ExprKind::Field(..) => true,
+            ExprKind::Call(f, _) => f.hir_id == child_id,
+            _ => false,
+        }
+    } else {
+        false
+    }
+}
+
 /// Adjustments are sometimes made in the parent block rather than the expression itself.
 fn find_adjustments<'tcx>(
     tcx: TyCtxt<'tcx>,
@@ -503,6 +586,7 @@ fn report(cx: &LateContext<'_>, expr: &Expr<'_>, state: State, data: StateData)
         State::DerefMethod {
             ty_changed_count,
             is_final_ufcs,
+            target_mut,
         } => {
             let mut app = Applicability::MachineApplicable;
             let (expr_str, expr_is_macro_call) = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app);
@@ -517,12 +601,12 @@ fn report(cx: &LateContext<'_>, expr: &Expr<'_>, state: State, data: StateData)
             };
             let addr_of_str = if ty_changed_count < ref_count {
                 // Check if a reborrow from &mut T -> &T is required.
-                if data.target_mut == Mutability::Not && matches!(ty.kind(), ty::Ref(_, _, Mutability::Mut)) {
+                if target_mut == Mutability::Not && matches!(ty.kind(), ty::Ref(_, _, Mutability::Mut)) {
                     "&*"
                 } else {
                     ""
                 }
-            } else if data.target_mut == Mutability::Mut {
+            } else if target_mut == Mutability::Mut {
                 "&mut "
             } else {
                 "&"
@@ -538,7 +622,7 @@ fn report(cx: &LateContext<'_>, expr: &Expr<'_>, state: State, data: StateData)
                 cx,
                 EXPLICIT_DEREF_METHODS,
                 data.span,
-                match data.target_mut {
+                match target_mut {
                     Mutability::Not => "explicit `deref` method call",
                     Mutability::Mut => "explicit `deref_mut` method call",
                 },
@@ -547,19 +631,24 @@ fn report(cx: &LateContext<'_>, expr: &Expr<'_>, state: State, data: StateData)
                 app,
             );
         },
-        State::DerefedBorrow { .. } => {
+        State::DerefedBorrow {
+            required_precedence,
+            msg,
+            ..
+        } => {
             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),
-                ),
+                msg,
                 "change this to",
-                snip.into(),
+                if required_precedence > expr.precedence().order() && !has_enclosing_paren(&snip) {
+                    format!("({})", snip)
+                } else {
+                    snip.into()
+                },
                 app,
             );
         },