Account for auto-borrows and precedence in redundant_slicing lint

Jarcho · Jarcho · commit 11f5850eb06e · 2022-01-03T20:35:32.000-05:00
diff --git a/clippy_lints/src/redundant_slicing.rs b/clippy_lints/src/redundant_slicing.rs
@@ -3,9 +3,11 @@ use clippy_utils::get_parent_expr;
 use clippy_utils::source::snippet_with_context;
 use clippy_utils::ty::{is_type_lang_item, peel_mid_ty_refs};
 use if_chain::if_chain;
+use rustc_ast::util::parser::PREC_PREFIX;
 use rustc_errors::Applicability;
 use rustc_hir::{BorrowKind, Expr, ExprKind, LangItem, Mutability};
 use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::adjustment::{Adjust, AutoBorrow, AutoBorrowMutability};
 use rustc_middle::ty::{subst::GenericArg, TyS};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 
@@ -57,44 +59,64 @@ impl LateLintPass<'_> for RedundantSlicing {
             then {
                 let (expr_ty, expr_ref_count) = peel_mid_ty_refs(cx.typeck_results().expr_ty(expr));
                 let (indexed_ty, indexed_ref_count) = peel_mid_ty_refs(cx.typeck_results().expr_ty(indexed));
+                let parent_expr = get_parent_expr(cx, expr);
+                let needs_parens_for_prefix = parent_expr.map_or(false, |parent| {
+                    parent.precedence().order() > PREC_PREFIX
+                });
                 let mut app = Applicability::MachineApplicable;
 
                 let (help, sugg) = if TyS::same_type(expr_ty, indexed_ty) {
                     if expr_ref_count > indexed_ref_count {
+                        // Indexing takes self by reference and can't return a reference to that
+                        // reference as it's a local variable. The only way this could happen is if
+                        // `self` contains a reference to the `Self` type. If this occurs then the
+                        // lint no longer applies as it's essentially a field access, which is not
+                        // redundant.
                         return;
                     }
+                    let deref_count = indexed_ref_count - expr_ref_count;
 
                     let (reborrow_str, help_str) = if mutability == Mutability::Mut {
                         // The slice was used to reborrow the mutable reference.
                         ("&mut *", "reborrow the original value instead")
                     } else if matches!(
-                        get_parent_expr(cx, expr),
+                        parent_expr,
                         Some(Expr {
                             kind: ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, _),
                             ..
                         })
-                    ) {
+                    ) || cx.typeck_results().expr_adjustments(expr).first().map_or(false, |a| {
+                        matches!(a.kind, Adjust::Borrow(AutoBorrow::Ref(_, AutoBorrowMutability::Mut { .. })))
+                    }) {
                         // The slice was used to make a temporary reference.
                         ("&*", "reborrow the original value instead")
-                    } else if expr_ref_count != indexed_ref_count {
+                    } else if deref_count != 0 {
                         ("", "dereference the original value instead")
                     } else {
                         ("", "use the original value instead")
                     };
 
                     let snip = snippet_with_context(cx, indexed.span, ctxt, "..", &mut app).0;
-                    (help_str, format!("{}{}{}", reborrow_str, "*".repeat(indexed_ref_count - expr_ref_count), snip))
+                    let sugg = if (deref_count != 0 || !reborrow_str.is_empty()) && needs_parens_for_prefix {
+                        format!("({}{}{})", reborrow_str, "*".repeat(deref_count), snip)
+                    } else {
+                        format!("{}{}{}", reborrow_str, "*".repeat(deref_count), snip)
+                    };
+
+                    (help_str, sugg)
                 } else if let Some(target_id) = cx.tcx.lang_items().deref_target() {
                     if let Ok(deref_ty) = cx.tcx.try_normalize_erasing_regions(
                         cx.param_env,
                         cx.tcx.mk_projection(target_id, cx.tcx.mk_substs([GenericArg::from(indexed_ty)].into_iter())),
                     ) {
                         if TyS::same_type(deref_ty, expr_ty) {
                             let snip = snippet_with_context(cx, indexed.span, ctxt, "..", &mut app).0;
-                            (
-                                "dereference the original value instead",
-                                format!("&{}{}*{}", mutability.prefix_str(), "*".repeat(indexed_ref_count), snip),
-                            )
+                            let sugg = if needs_parens_for_prefix {
+                                format!("(&{}{}*{})", mutability.prefix_str(), "*".repeat(indexed_ref_count), snip)
+                            } else {
+                                format!("&{}{}*{}", mutability.prefix_str(), "*".repeat(indexed_ref_count), snip)
+                            };
+                            ("dereference the original value instead", sugg)
                         } else {
                             return;
                         }
diff --git a/tests/ui/redundant_slicing.fixed b/tests/ui/redundant_slicing.fixed
@@ -3,6 +3,8 @@
 #![allow(unused)]
 #![warn(clippy::redundant_slicing)]
 
+use std::io::Read;
+
 fn main() {
     let slice: &[u32] = &[0];
     let _ = slice; // Redundant slice
@@ -37,4 +39,8 @@ fn main() {
 
     let slice_ref = &slice;
     let _ = *slice_ref; // Deref instead of slice
+
+    // Issue #7972
+    let bytes: &[u8] = &[];
+    let _ = (&*bytes).read_to_end(&mut vec![]).unwrap();
 }
diff --git a/tests/ui/redundant_slicing.rs b/tests/ui/redundant_slicing.rs
@@ -3,6 +3,8 @@
 #![allow(unused)]
 #![warn(clippy::redundant_slicing)]
 
+use std::io::Read;
+
 fn main() {
     let slice: &[u32] = &[0];
     let _ = &slice[..]; // Redundant slice
@@ -37,4 +39,8 @@ fn main() {
 
     let slice_ref = &slice;
     let _ = &slice_ref[..]; // Deref instead of slice
+
+    // Issue #7972
+    let bytes: &[u8] = &[];
+    let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap();
 }
diff --git a/tests/ui/redundant_slicing.stderr b/tests/ui/redundant_slicing.stderr
@@ -1,58 +1,64 @@
 error: redundant slicing of the whole range
-  --> $DIR/redundant_slicing.rs:8:13
+  --> $DIR/redundant_slicing.rs:10:13
    |
 LL |     let _ = &slice[..]; // Redundant slice
    |             ^^^^^^^^^^ help: use the original value instead: `slice`
    |
    = note: `-D clippy::redundant-slicing` implied by `-D warnings`
 
 error: redundant slicing of the whole range
-  --> $DIR/redundant_slicing.rs:11:13
+  --> $DIR/redundant_slicing.rs:13:13
    |
 LL |     let _ = &v[..]; // Deref instead of slice
    |             ^^^^^^ help: dereference the original value instead: `&*v`
 
 error: redundant slicing of the whole range
-  --> $DIR/redundant_slicing.rs:12:13
+  --> $DIR/redundant_slicing.rs:14:13
    |
 LL |     let _ = &(&*v)[..]; // Outer borrow is redundant
    |             ^^^^^^^^^^ help: use the original value instead: `(&*v)`
 
 error: redundant slicing of the whole range
-  --> $DIR/redundant_slicing.rs:15:20
+  --> $DIR/redundant_slicing.rs:17:20
    |
 LL |     let err = &mut &S[..]; // Should reborrow instead of slice
    |                    ^^^^^^ help: reborrow the original value instead: `&*S`
 
 error: redundant slicing of the whole range
-  --> $DIR/redundant_slicing.rs:18:21
+  --> $DIR/redundant_slicing.rs:20:21
    |
 LL |     let mut_slice = &mut vec[..]; // Deref instead of slice
    |                     ^^^^^^^^^^^^ help: dereference the original value instead: `&mut *vec`
 
 error: redundant slicing of the whole range
-  --> $DIR/redundant_slicing.rs:19:13
+  --> $DIR/redundant_slicing.rs:21:13
    |
 LL |     let _ = &mut mut_slice[..]; // Should reborrow instead of slice
    |             ^^^^^^^^^^^^^^^^^^ help: reborrow the original value instead: `&mut *mut_slice`
 
 error: redundant slicing of the whole range
-  --> $DIR/redundant_slicing.rs:22:13
+  --> $DIR/redundant_slicing.rs:24:13
    |
 LL |     let _ = &ref_vec[..]; // Deref instead of slice
    |             ^^^^^^^^^^^^ help: dereference the original value instead: `&**ref_vec`
 
 error: redundant slicing of the whole range
-  --> $DIR/redundant_slicing.rs:29:13
+  --> $DIR/redundant_slicing.rs:31:13
    |
 LL |     let _ = &m!(slice)[..];
    |             ^^^^^^^^^^^^^^ help: use the original value instead: `slice`
 
 error: redundant slicing of the whole range
-  --> $DIR/redundant_slicing.rs:39:13
+  --> $DIR/redundant_slicing.rs:41:13
    |
 LL |     let _ = &slice_ref[..]; // Deref instead of slice
    |             ^^^^^^^^^^^^^^ help: dereference the original value instead: `*slice_ref`
 
-error: aborting due to 9 previous errors
+error: redundant slicing of the whole range
+  --> $DIR/redundant_slicing.rs:45:13
+   |
+LL |     let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap();
+   |             ^^^^^^^^^^^^ help: reborrow the original value instead: `(&*bytes)`
+
+error: aborting due to 10 previous errors
 
