Add new lint suspicious_slice_copies

CHANGELOG.md

@@ -7052,6 +7052,7 @@ Released 2018-09-13
 [`suspicious_op_assign_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_op_assign_impl
 [`suspicious_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_open_options
 [`suspicious_operation_groupings`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_operation_groupings
+[`suspicious_slice_copies`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_slice_copies
 [`suspicious_splitn`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_splitn
 [`suspicious_to_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_to_owned
 [`suspicious_unary_op_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_unary_op_formatting

clippy_lints/src/declared_lints.rs

@@ -476,6 +476,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::methods::SUSPICIOUS_COMMAND_ARG_SPACE_INFO,
     crate::methods::SUSPICIOUS_MAP_INFO,
     crate::methods::SUSPICIOUS_OPEN_OPTIONS_INFO,
+    crate::methods::SUSPICIOUS_SLICE_COPIES_INFO,
     crate::methods::SUSPICIOUS_SPLITN_INFO,
     crate::methods::SUSPICIOUS_TO_OWNED_INFO,
     crate::methods::SWAP_WITH_TEMPORARY_INFO,

clippy_lints/src/methods/mod.rs

@@ -117,6 +117,7 @@ mod string_extend_chars;
 mod string_lit_chars_any;
 mod suspicious_command_arg_space;
 mod suspicious_map;
+mod suspicious_slice_copies;
 mod suspicious_splitn;
 mod suspicious_to_owned;
 mod swap_with_temporary;
@@ -3754,6 +3755,42 @@ declare_clippy_lint! {
     "suspicious combination of options for opening a file"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for taking a mutable reference to a temporary array created by `arr[N..M].try_into()`
+    /// instead of borrowing the original slice data.
+    ///
+    /// ### Why is this bad?
+    /// Slices have two relevant implementations of the `TryInto` trait:
+    /// 1. `TryInto<[T; N]>`, which creates a new array by copying elements from the slice
+    /// 2. `TryInto<&'a mut [T; N]>`, which creates a mutable array reference from slice
+    ///
+    /// When writing `&mut arr[N..M].try_into().unwrap()`, the compiler selects the first implementation
+    /// and creates a new array. As a result, any mutations are applied to this temporary array, while
+    /// the original data remains unchanged. This is usually unintended.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let mut arr: [i32; 5] = [0; 5];
+    /// let arr_mut_ref: &mut [i32; 3] = &mut arr[..3].try_into().unwrap(); //copied
+    /// arr_mut_ref[0] = 1;
+    ///
+    /// assert_eq!(arr[0], 0);
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let mut arr: [i32; 5] = [0; 5];
+    /// let arr_mut_ref: &mut [i32; 3] = (&mut arr[..3]).try_into().unwrap(); //borrowed
+    /// arr_mut_ref[0] = 1;
+    ///
+    /// assert_eq!(arr[0], 1);
+    /// ```
+    #[clippy::version = "1.96.0"]
+    pub SUSPICIOUS_SLICE_COPIES,
+    suspicious,
+    "detects taking a `&mut` to a temporary array created from a slice"
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for calls to [`splitn`]
@@ -4873,6 +4910,7 @@ impl_lint_pass!(Methods => [
     SUSPICIOUS_COMMAND_ARG_SPACE,
     SUSPICIOUS_MAP,
     SUSPICIOUS_OPEN_OPTIONS,
+    SUSPICIOUS_SLICE_COPIES,
     SUSPICIOUS_SPLITN,
     SUSPICIOUS_TO_OWNED,
     SWAP_WITH_TEMPORARY,
@@ -5618,6 +5656,7 @@ impl Methods {
                 },
                 (sym::try_into, []) if cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::TryInto) => {
                     unnecessary_fallible_conversions::check_method(cx, expr);
+                    suspicious_slice_copies::check(cx, expr, recv, call_span, self.msrv);
                 },
                 (sym::to_owned, []) =>
                 {

clippy_lints/src/methods/suspicious_slice_copies.rs

@@ -0,0 +1,110 @@
+use super::SUSPICIOUS_SLICE_COPIES;
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::msrvs::Msrv;
+use clippy_utils::source::{IntoSpan, SpanRangeExt, snippet_with_context};
+use clippy_utils::{msrvs, sym};
+use rustc_ast::{BorrowKind, Mutability};
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, Node};
+use rustc_lint::LateContext;
+use rustc_middle::ty;
+use rustc_middle::ty::{GenericArgKind, Ty};
+use rustc_span::Span;
+
+enum TryIntoReceiver {
+    Slice,
+    SliceMutRef,
+}
+
+impl TryIntoReceiver {
+    fn from_ty(ty: Ty<'_>) -> Option<Self> {
+        match ty.kind() {
+            ty::Slice(_) => Some(TryIntoReceiver::Slice),
+            ty::Ref(_, ref_ty, Mutability::Mut) if let ty::Slice(_) = ref_ty.kind() => {
+                Some(TryIntoReceiver::SliceMutRef)
+            },
+            _ => None,
+        }
+    }
+}
+
+pub(super) fn check(
+    cx: &LateContext<'_>,
+    try_into_expr: &Expr<'_>,
+    recv_expr: &Expr<'_>,
+    try_into_span: Span,
+    msrv: Msrv,
+) {
+    if let Some(receiver) = TryIntoReceiver::from_ty(cx.typeck_results().expr_ty(recv_expr))
+
+        // try_into was instantiated with [T; N]
+        && let [_, dst_ty] = &**cx.typeck_results().node_args(try_into_expr.hir_id)
+        && let GenericArgKind::Type(arg_ty) = dst_ty.kind()
+        && let ty::Array(..) = arg_ty.kind()
+
+        && let mut parent_iter = cx.tcx.hir_parent_iter(try_into_expr.hir_id)
+
+        // calls unwrap() or expect()
+        && let Some((_, Node::Expr(unwrap_expr))) = parent_iter.next()
+        && let ExprKind::MethodCall(path, .., unwrap_span) = unwrap_expr.kind
+        && let sym::unwrap | sym::expect = path.ident.name
+
+        // use mut ref
+        && let Some((_, Node::Expr(mut_ref_expr))) = parent_iter.next()
+        && let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, inner_expr) = mut_ref_expr.kind
+    {
+        let span = mut_ref_expr.span.with_hi(inner_expr.span.hi());
+
+        span_lint_and_then(
+            cx,
+            SUSPICIOUS_SLICE_COPIES,
+            span,
+            "using a mutable reference to temporary array",
+            |diag| {
+                let mut applicability = Applicability::MaybeIncorrect;
+
+                let (recv_snippet, _) =
+                    snippet_with_context(cx, recv_expr.span, try_into_span.ctxt(), "..", &mut applicability);
+                let (unwrap_snippet, _) =
+                    snippet_with_context(cx, unwrap_span, try_into_span.ctxt(), "..", &mut applicability);
+                let (mut_ref_snippet, _) = snippet_with_context(
+                    cx,
+                    mut_ref_expr
+                        .span
+                        .until(inner_expr.span.with_leading_whitespace(cx).into_span()),
+                    try_into_span.ctxt(),
+                    "..",
+                    &mut applicability,
+                );
+
+                let borrow_sugg = if msrv.meets(cx, msrvs::SLICE_AS_MUT_ARRAY) {
+                    format!("{recv_snippet}.as_mut_array().{unwrap_snippet}")
+                } else {
+                    match receiver {
+                        TryIntoReceiver::Slice => {
+                            format!("({mut_ref_snippet} {recv_snippet}).try_into().{unwrap_snippet}")
+                        },
+                        TryIntoReceiver::SliceMutRef => format!("{recv_snippet}.try_into().{unwrap_snippet}"),
+                    }
+                };
+
+                let copy_sugg = match receiver {
+                    TryIntoReceiver::Slice => {
+                        format!("{mut_ref_snippet} <[_; _]>::try_from(&{recv_snippet}).{unwrap_snippet}")
+                    },
+                    TryIntoReceiver::SliceMutRef => {
+                        format!("{mut_ref_snippet} <[_; _]>::try_from({recv_snippet}).{unwrap_snippet}")
+                    },
+                };
+
+                diag.span_suggestion(span, "borrow the slice as an array", borrow_sugg, applicability);
+                diag.span_suggestion(
+                    span,
+                    "explicitly create a new array using `TryFrom`",
+                    copy_sugg,
+                    applicability,
+                );
+            },
+        );
+    }
+}

clippy_utils/src/msrvs.rs

@@ -84,6 +84,7 @@ msrv_aliases! {
     1,15,0 { MAYBE_BOUND_IN_WHERE }
     1,13,0 { QUESTION_MARK_OPERATOR }
     1,3,0 { DURATION_FROM_MILLIS_SECS }
+    1,93,0 { SLICE_AS_MUT_ARRAY }
 }
 
 /// `#[clippy::msrv]` attributes are rarely used outside of Clippy's test suite, as a basic

tests/ui/suspicious_slice_copies.1.fixed

@@ -0,0 +1,58 @@
+#![warn(clippy::suspicious_slice_copies)]
+
+const K: usize = 3;
+const N: usize = 5;
+
+fn arr_incr(arr: &mut [i32; K]) {
+    for x in arr.iter_mut() {
+        *x += 1;
+    }
+}
+
+#[clippy::msrv = "1.92.0"]
+fn test_msrv_1_92() {
+    let mut arr: [i32; N] = [0; 5];
+    let slice: &mut [i32] = &mut arr[..K];
+
+    let mut_arr_ref: &mut [i32; K] = slice.try_into().unwrap();
+    //~^ suspicious_slice_copies
+
+    let mut_arr_ref: &mut [i32; K] = (&mut arr[..K]).try_into().unwrap();
+    //~^ suspicious_slice_copies
+
+    let mut_arr_ref: &mut [i32; K] = (&mut arr[..K]).try_into().unwrap();
+    //~^ suspicious_slice_copies
+
+    arr_incr((&mut arr[..K]).try_into().expect("never happen"));
+    //~^ suspicious_slice_copies
+}
+
+#[clippy::msrv = "1.93.0"]
+fn test_msrv_1_93() {
+    let mut arr: [i32; N] = [0; 5];
+    let slice: &mut [i32] = &mut arr[..K];
+
+    let mut_arr_ref: &mut [i32; K] = slice.as_mut_array().unwrap();
+    //~^ suspicious_slice_copies
+
+    let mut_arr_ref: &mut [i32; K] = arr[..K].as_mut_array().unwrap();
+    //~^ suspicious_slice_copies
+
+    let mut_arr_ref: &mut [i32; K] = arr[..K].as_mut_array().unwrap();
+    //~^ suspicious_slice_copies
+
+    arr_incr(arr[..K].as_mut_array().expect("never happen"));
+    //~^ suspicious_slice_copies
+}
+
+fn main() {
+    let mut arr: [i32; N] = [0; 5];
+
+    let arr_ref: &[i32; K] = &arr[..K].try_into().unwrap();
+    let arr_mut_ref: &mut [i32; K] = (&mut arr[..K]).try_into().unwrap();
+
+    let arr_copy: [i32; K] = arr[..K].try_into().unwrap();
+
+    let slice: &[i32] = &arr[..K];
+    let arr_mut_ref: &mut [i32; K] = &mut slice.try_into().unwrap();
+}

tests/ui/suspicious_slice_copies.2.fixed

@@ -0,0 +1,58 @@
+#![warn(clippy::suspicious_slice_copies)]
+
+const K: usize = 3;
+const N: usize = 5;
+
+fn arr_incr(arr: &mut [i32; K]) {
+    for x in arr.iter_mut() {
+        *x += 1;
+    }
+}
+
+#[clippy::msrv = "1.92.0"]
+fn test_msrv_1_92() {
+    let mut arr: [i32; N] = [0; 5];
+    let slice: &mut [i32] = &mut arr[..K];
+
+    let mut_arr_ref: &mut [i32; K] = &mut <[_; _]>::try_from(slice).unwrap();
+    //~^ suspicious_slice_copies
+
+    let mut_arr_ref: &mut [i32; K] = &mut <[_; _]>::try_from(&arr[..K]).unwrap();
+    //~^ suspicious_slice_copies
+
+    let mut_arr_ref: &mut [i32; K] = &mut <[_; _]>::try_from(&arr[..K]).unwrap();
+    //~^ suspicious_slice_copies
+
+    arr_incr(&mut <[_; _]>::try_from(&arr[..K]).expect("never happen"));
+    //~^ suspicious_slice_copies
+}
+
+#[clippy::msrv = "1.93.0"]
+fn test_msrv_1_93() {
+    let mut arr: [i32; N] = [0; 5];
+    let slice: &mut [i32] = &mut arr[..K];
+
+    let mut_arr_ref: &mut [i32; K] = &mut <[_; _]>::try_from(slice).unwrap();
+    //~^ suspicious_slice_copies
+
+    let mut_arr_ref: &mut [i32; K] = &mut <[_; _]>::try_from(&arr[..K]).unwrap();
+    //~^ suspicious_slice_copies
+
+    let mut_arr_ref: &mut [i32; K] = &mut <[_; _]>::try_from(&arr[..K]).unwrap();
+    //~^ suspicious_slice_copies
+
+    arr_incr(&mut <[_; _]>::try_from(&arr[..K]).expect("never happen"));
+    //~^ suspicious_slice_copies
+}
+
+fn main() {
+    let mut arr: [i32; N] = [0; 5];
+
+    let arr_ref: &[i32; K] = &arr[..K].try_into().unwrap();
+    let arr_mut_ref: &mut [i32; K] = (&mut arr[..K]).try_into().unwrap();
+
+    let arr_copy: [i32; K] = arr[..K].try_into().unwrap();
+
+    let slice: &[i32] = &arr[..K];
+    let arr_mut_ref: &mut [i32; K] = &mut slice.try_into().unwrap();
+}

tests/ui/suspicious_slice_copies.rs

@@ -0,0 +1,58 @@
+#![warn(clippy::suspicious_slice_copies)]
+
+const K: usize = 3;
+const N: usize = 5;
+
+fn arr_incr(arr: &mut [i32; K]) {
+    for x in arr.iter_mut() {
+        *x += 1;
+    }
+}
+
+#[clippy::msrv = "1.92.0"]
+fn test_msrv_1_92() {
+    let mut arr: [i32; N] = [0; 5];
+    let slice: &mut [i32] = &mut arr[..K];
+
+    let mut_arr_ref: &mut [i32; K] = &mut slice.try_into().unwrap();
+    //~^ suspicious_slice_copies
+
+    let mut_arr_ref: &mut [i32; K] = &mut arr[..K].try_into().unwrap();
+    //~^ suspicious_slice_copies
+
+    let mut_arr_ref: &mut [i32; K] = &mut (arr[..K].try_into().unwrap());
+    //~^ suspicious_slice_copies
+
+    arr_incr(&mut arr[..K].try_into().expect("never happen"));
+    //~^ suspicious_slice_copies
+}
+
+#[clippy::msrv = "1.93.0"]
+fn test_msrv_1_93() {
+    let mut arr: [i32; N] = [0; 5];
+    let slice: &mut [i32] = &mut arr[..K];
+
+    let mut_arr_ref: &mut [i32; K] = &mut slice.try_into().unwrap();
+    //~^ suspicious_slice_copies
+
+    let mut_arr_ref: &mut [i32; K] = &mut arr[..K].try_into().unwrap();
+    //~^ suspicious_slice_copies
+
+    let mut_arr_ref: &mut [i32; K] = &mut (arr[..K].try_into().unwrap());
+    //~^ suspicious_slice_copies
+
+    arr_incr(&mut arr[..K].try_into().expect("never happen"));
+    //~^ suspicious_slice_copies
+}
+
+fn main() {
+    let mut arr: [i32; N] = [0; 5];
+
+    let arr_ref: &[i32; K] = &arr[..K].try_into().unwrap();
+    let arr_mut_ref: &mut [i32; K] = (&mut arr[..K]).try_into().unwrap();
+
+    let arr_copy: [i32; K] = arr[..K].try_into().unwrap();
+
+    let slice: &[i32] = &arr[..K];
+    let arr_mut_ref: &mut [i32; K] = &mut slice.try_into().unwrap();
+}