Auto merge of rust-lang#6975 - Jarcho:redundant_slicing_fp, r=llogiq

Improve `redundant_slicing`

fixes: rust-lang#6968

changelog: Fix `redundant_slicing` suggestion when a reborrow might be required or when the value is from a macro call

clippy_lints/src/redundant_slicing.rs

@@ -1,11 +1,12 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::source::snippet_with_context;
 use clippy_utils::ty::is_type_lang_item;
+use clippy_utils::{get_parent_expr, in_macro};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
-use rustc_hir::{Expr, ExprKind, LangItem};
-use rustc_lint::{LateContext, LateLintPass, LintContext};
-use rustc_middle::{lint::in_external_macro, ty::TyS};
+use rustc_hir::{BorrowKind, Expr, ExprKind, LangItem, Mutability};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::TyS;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 
 declare_clippy_lint! {
@@ -40,26 +41,44 @@ declare_lint_pass!(RedundantSlicing => [REDUNDANT_SLICING]);
 
 impl LateLintPass<'_> for RedundantSlicing {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        if in_external_macro(cx.sess(), expr.span) {
+        if in_macro(expr.span) {
             return;
         }
 
+        let ctxt = expr.span.ctxt();
         if_chain! {
-            if let ExprKind::AddrOf(_, _, addressee) = expr.kind;
+            if let ExprKind::AddrOf(BorrowKind::Ref, mutability, addressee) = expr.kind;
+            if addressee.span.ctxt() == ctxt;
             if let ExprKind::Index(indexed, range) = addressee.kind;
             if is_type_lang_item(cx, cx.typeck_results().expr_ty_adjusted(range), LangItem::RangeFull);
             if TyS::same_type(cx.typeck_results().expr_ty(expr), cx.typeck_results().expr_ty(indexed));
             then {
                 let mut app = Applicability::MachineApplicable;
-                let hint = snippet_with_applicability(cx, indexed.span, "..", &mut app).into_owned();
+                let snip = snippet_with_context(cx, indexed.span, ctxt, "..", &mut app).0;
+
+                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),
+                    Some(Expr {
+                        kind: ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, _),
+                        ..
+                    })
+                ) {
+                    // The slice was used to make a temporary reference.
+                    ("&*", "reborrow the original value instead")
+                } else {
+                    ("", "use the original value instead")
+                };
 
                 span_lint_and_sugg(
                     cx,
                     REDUNDANT_SLICING,
                     expr.span,
                     "redundant slicing of the whole range",
-                    "use the original slice instead",
-                    hint,
+                    help_str,
+                    format!("{}{}", reborrow_str, snip),
                     app,
                 );
             }

tests/ui/redundant_slicing.rs

@@ -2,10 +2,31 @@
 #![warn(clippy::redundant_slicing)]
 
 fn main() {
-    let x: &[u32] = &[0];
-    let err = &x[..];
+    let slice: &[u32] = &[0];
+    let _ = &slice[..];
 
     let v = vec![0];
-    let ok = &v[..];
-    let err = &(&v[..])[..];
+    let _ = &v[..]; // Changes the type
+    let _ = &(&v[..])[..]; // Outer borrow is redundant
+
+    static S: &[u8] = &[0, 1, 2];
+    let err = &mut &S[..]; // Should reborrow instead of slice
+
+    let mut vec = vec![0];
+    let mut_slice = &mut *vec;
+    let _ = &mut mut_slice[..]; // Should reborrow instead of slice
+
+    macro_rules! m {
+        ($e:expr) => {
+            $e
+        };
+    }
+    let _ = &m!(slice)[..];
+
+    macro_rules! m2 {
+        ($e:expr) => {
+            &$e[..]
+        };
+    }
+    let _ = m2!(slice); // Don't lint in a macro
 }

tests/ui/redundant_slicing.stderr

@@ -1,16 +1,34 @@
 error: redundant slicing of the whole range
-  --> $DIR/redundant_slicing.rs:6:15
+  --> $DIR/redundant_slicing.rs:6:13
    |
-LL |     let err = &x[..];
-   |               ^^^^^^ help: use the original slice instead: `x`
+LL |     let _ = &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:10:15
+  --> $DIR/redundant_slicing.rs:10:13
    |
-LL |     let err = &(&v[..])[..];
-   |               ^^^^^^^^^^^^^ help: use the original slice instead: `(&v[..])`
+LL |     let _ = &(&v[..])[..]; // Outer borrow is redundant
+   |             ^^^^^^^^^^^^^ help: use the original value instead: `(&v[..])`
 
-error: aborting due to 2 previous errors
+error: redundant slicing of the whole range
+  --> $DIR/redundant_slicing.rs:13: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:17: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:24:13
+   |
+LL |     let _ = &m!(slice)[..];
+   |             ^^^^^^^^^^^^^^ help: use the original value instead: `slice`
+
+error: aborting due to 5 previous errors