Add new lint: cloned_ref_to_slice_refs (#14284)

Added lint for catching `&[foo.clone()]` where foo is a reference and
suggests `std::slice::from_ref(foo)`.

changelog: new lint: [`cloned_ref_to_slice_refs`]

Author: llogiq

CHANGELOG.md

@@ -5681,6 +5681,7 @@ Released 2018-09-13
 [`clone_on_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy
 [`clone_on_ref_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_ref_ptr
 [`cloned_instead_of_copied`]: https://rust-lang.github.io/rust-clippy/master/index.html#cloned_instead_of_copied
+[`cloned_ref_to_slice_refs`]: https://rust-lang.github.io/rust-clippy/master/index.html#cloned_ref_to_slice_refs
 [`cmp_nan`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_nan
 [`cmp_null`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_null
 [`cmp_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_owned

clippy_lints/src/cloned_ref_to_slice_refs.rs

@@ -0,0 +1,100 @@
+use clippy_config::Conf;
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::sugg::Sugg;
+use clippy_utils::visitors::is_const_evaluatable;
+use clippy_utils::{is_in_const_context, is_mutable, is_trait_method};
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::impl_lint_pass;
+use rustc_span::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Checks for slice references with cloned references such as `&[f.clone()]`.
+    ///
+    /// ### Why is this bad
+    ///
+    /// A reference does not need to be owned in order to used as a slice.
+    ///
+    /// ### Known problems
+    ///
+    /// This lint does not know whether or not a clone implementation has side effects.
+    ///
+    /// ### Example
+    ///
+    /// ```ignore
+    /// let data = 10;
+    /// let data_ref = &data;
+    /// take_slice(&[data_ref.clone()]);
+    /// ```
+    /// Use instead:
+    /// ```ignore
+    /// use std::slice;
+    /// let data = 10;
+    /// let data_ref = &data;
+    /// take_slice(slice::from_ref(data_ref));
+    /// ```
+    #[clippy::version = "1.87.0"]
+    pub CLONED_REF_TO_SLICE_REFS,
+    perf,
+    "cloning a reference for slice references"
+}
+
+pub struct ClonedRefToSliceRefs<'a> {
+    msrv: &'a Msrv,
+}
+impl<'a> ClonedRefToSliceRefs<'a> {
+    pub fn new(conf: &'a Conf) -> Self {
+        Self { msrv: &conf.msrv }
+    }
+}
+
+impl_lint_pass!(ClonedRefToSliceRefs<'_> => [CLONED_REF_TO_SLICE_REFS]);
+
+impl<'tcx> LateLintPass<'tcx> for ClonedRefToSliceRefs<'_> {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
+        if self.msrv.meets(cx, {
+            if is_in_const_context(cx) {
+                msrvs::CONST_SLICE_FROM_REF
+            } else {
+                msrvs::SLICE_FROM_REF
+            }
+        })
+            // `&[foo.clone()]` expressions
+            && let ExprKind::AddrOf(_, mutability, arr) = &expr.kind
+            // mutable references would have a different meaning
+            && mutability.is_not()
+
+            // check for single item arrays
+            && let ExprKind::Array([item]) = &arr.kind
+
+            // check for clones
+            && let ExprKind::MethodCall(_, val, _, _) = item.kind
+            && is_trait_method(cx, item, sym::Clone)
+
+            // check for immutability or purity
+            && (!is_mutable(cx, val) || is_const_evaluatable(cx, val))
+
+            // get appropriate crate for `slice::from_ref`
+            && let Some(builtin_crate) = clippy_utils::std_or_core(cx)
+        {
+            let mut sugg = Sugg::hir(cx, val, "_");
+            if !cx.typeck_results().expr_ty(val).is_ref() {
+                sugg = sugg.addr();
+            }
+
+            span_lint_and_sugg(
+                cx,
+                CLONED_REF_TO_SLICE_REFS,
+                expr.span,
+                format!("this call to `clone` can be replaced with `{builtin_crate}::slice::from_ref`"),
+                "try",
+                format!("{builtin_crate}::slice::from_ref({sugg})"),
+                Applicability::MaybeIncorrect,
+            );
+        }
+    }
+}

clippy_lints/src/declared_lints.rs

@@ -76,6 +76,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::casts::ZERO_PTR_INFO,
     crate::cfg_not_test::CFG_NOT_TEST_INFO,
     crate::checked_conversions::CHECKED_CONVERSIONS_INFO,
+    crate::cloned_ref_to_slice_refs::CLONED_REF_TO_SLICE_REFS_INFO,
     crate::cognitive_complexity::COGNITIVE_COMPLEXITY_INFO,
     crate::collapsible_if::COLLAPSIBLE_ELSE_IF_INFO,
     crate::collapsible_if::COLLAPSIBLE_IF_INFO,

clippy_lints/src/lib.rs

@@ -94,6 +94,7 @@ mod cargo;
 mod casts;
 mod cfg_not_test;
 mod checked_conversions;
+mod cloned_ref_to_slice_refs;
 mod cognitive_complexity;
 mod collapsible_if;
 mod collection_is_never_read;
@@ -942,5 +943,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(|_| Box::new(manual_option_as_slice::ManualOptionAsSlice::new(conf)));
     store.register_late_pass(|_| Box::new(single_option_map::SingleOptionMap));
     store.register_late_pass(move |_| Box::new(redundant_test_prefix::RedundantTestPrefix));
+    store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf)));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

clippy_utils/src/msrvs.rs

@@ -39,7 +39,7 @@ msrv_aliases! {
     1,70,0 { OPTION_RESULT_IS_VARIANT_AND, BINARY_HEAP_RETAIN }
     1,68,0 { PATH_MAIN_SEPARATOR_STR }
     1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS }
-    1,63,0 { CLONE_INTO }
+    1,63,0 { CLONE_INTO, CONST_SLICE_FROM_REF }
     1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE, CONST_EXTERN_C_FN }
     1,60,0 { ABS_DIFF }
     1,59,0 { THREAD_LOCAL_CONST_INIT }
@@ -69,7 +69,7 @@ msrv_aliases! {
     1,31,0 { OPTION_REPLACE }
     1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES }
     1,29,0 { ITER_FLATTEN }
-    1,28,0 { FROM_BOOL, REPEAT_WITH }
+    1,28,0 { FROM_BOOL, REPEAT_WITH, SLICE_FROM_REF }
     1,27,0 { ITERATOR_TRY_FOLD }
     1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN }
     1,24,0 { IS_ASCII_DIGIT }

tests/ui/cloned_ref_to_slice_refs.fixed

@@ -0,0 +1,64 @@
+#![warn(clippy::cloned_ref_to_slice_refs)]
+
+#[derive(Clone)]
+struct Data;
+
+fn main() {
+    {
+        let data = Data;
+        let data_ref = &data;
+        let _ = std::slice::from_ref(data_ref); //~ ERROR: this call to `clone` can be replaced with `std::slice::from_ref`
+    }
+
+    {
+        let _ = std::slice::from_ref(&Data); //~ ERROR: this call to `clone` can be replaced with `std::slice::from_ref`
+    }
+
+    {
+        #[derive(Clone)]
+        struct Point(i32, i32);
+
+        let _ = std::slice::from_ref(&Point(0, 0)); //~ ERROR: this call to `clone` can be replaced with `std::slice::from_ref`
+    }
+
+    // the string was cloned with the intention to not mutate
+    {
+        struct BetterString(String);
+
+        let mut message = String::from("good");
+        let sender = BetterString(message.clone());
+
+        message.push_str("bye!");
+
+        println!("{} {}", message, sender.0)
+    }
+
+    // the string was cloned with the intention to not mutate
+    {
+        let mut x = String::from("Hello");
+        let r = &[x.clone()];
+        x.push('!');
+        println!("r = `{}', x = `{x}'", r[0]);
+    }
+
+    // mutable borrows may have the intention to clone
+    {
+        let data = Data;
+        let data_ref = &data;
+        let _ = &mut [data_ref.clone()];
+    }
+
+    // `T::clone` is used to denote a clone with side effects
+    {
+        use std::sync::Arc;
+        let data = Arc::new(Data);
+        let _ = &[Arc::clone(&data)];
+    }
+
+    // slices with multiple members can only be made from a singular reference
+    {
+        let data_1 = Data;
+        let data_2 = Data;
+        let _ = &[data_1.clone(), data_2.clone()];
+    }
+}

tests/ui/cloned_ref_to_slice_refs.rs

@@ -0,0 +1,64 @@
+#![warn(clippy::cloned_ref_to_slice_refs)]
+
+#[derive(Clone)]
+struct Data;
+
+fn main() {
+    {
+        let data = Data;
+        let data_ref = &data;
+        let _ = &[data_ref.clone()]; //~ ERROR: this call to `clone` can be replaced with `std::slice::from_ref`
+    }
+
+    {
+        let _ = &[Data.clone()]; //~ ERROR: this call to `clone` can be replaced with `std::slice::from_ref`
+    }
+
+    {
+        #[derive(Clone)]
+        struct Point(i32, i32);
+
+        let _ = &[Point(0, 0).clone()]; //~ ERROR: this call to `clone` can be replaced with `std::slice::from_ref`
+    }
+
+    // the string was cloned with the intention to not mutate
+    {
+        struct BetterString(String);
+
+        let mut message = String::from("good");
+        let sender = BetterString(message.clone());
+
+        message.push_str("bye!");
+
+        println!("{} {}", message, sender.0)
+    }
+
+    // the string was cloned with the intention to not mutate
+    {
+        let mut x = String::from("Hello");
+        let r = &[x.clone()];
+        x.push('!');
+        println!("r = `{}', x = `{x}'", r[0]);
+    }
+
+    // mutable borrows may have the intention to clone
+    {
+        let data = Data;
+        let data_ref = &data;
+        let _ = &mut [data_ref.clone()];
+    }
+
+    // `T::clone` is used to denote a clone with side effects
+    {
+        use std::sync::Arc;
+        let data = Arc::new(Data);
+        let _ = &[Arc::clone(&data)];
+    }
+
+    // slices with multiple members can only be made from a singular reference
+    {
+        let data_1 = Data;
+        let data_2 = Data;
+        let _ = &[data_1.clone(), data_2.clone()];
+    }
+}

tests/ui/cloned_ref_to_slice_refs.stderr

@@ -0,0 +1,23 @@
+error: this call to `clone` can be replaced with `std::slice::from_ref`
+  --> tests/ui/cloned_ref_to_slice_refs.rs:10:17
+   |
+LL |         let _ = &[data_ref.clone()];
+   |                 ^^^^^^^^^^^^^^^^^^^ help: try: `std::slice::from_ref(data_ref)`
+   |
+   = note: `-D clippy::cloned-ref-to-slice-refs` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::cloned_ref_to_slice_refs)]`
+
+error: this call to `clone` can be replaced with `std::slice::from_ref`
+  --> tests/ui/cloned_ref_to_slice_refs.rs:14:17
+   |
+LL |         let _ = &[Data.clone()];
+   |                 ^^^^^^^^^^^^^^^ help: try: `std::slice::from_ref(&Data)`
+
+error: this call to `clone` can be replaced with `std::slice::from_ref`
+  --> tests/ui/cloned_ref_to_slice_refs.rs:21:17
+   |
+LL |         let _ = &[Point(0, 0).clone()];
+   |                 ^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::slice::from_ref(&Point(0, 0))`
+
+error: aborting due to 3 previous errors
+