diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_lints/src/cloned_ref_to_slice_refs.rs | 100 | ||||
| -rw-r--r-- | clippy_lints/src/declared_lints.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 2 | ||||
| -rw-r--r-- | clippy_utils/src/msrvs.rs | 4 | ||||
| -rw-r--r-- | tests/ui/cloned_ref_to_slice_refs.fixed | 64 | ||||
| -rw-r--r-- | tests/ui/cloned_ref_to_slice_refs.rs | 64 | ||||
| -rw-r--r-- | tests/ui/cloned_ref_to_slice_refs.stderr | 23 |
8 files changed, 257 insertions, 2 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b62c9a59aa..956e31489f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5583,6 +5583,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 diff --git a/clippy_lints/src/cloned_ref_to_slice_refs.rs b/clippy_lints/src/cloned_ref_to_slice_refs.rs new file mode 100644 index 00000000000..6b239a1541b --- /dev/null +++ b/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, + ); + } + } +} diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 2cccd6ba270..fbce468141b 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -75,6 +75,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, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 5fa8f6f4bf3..37669a90eb4 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -96,6 +96,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; @@ -943,5 +944,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` } diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index c2aca5ee539..e6f46f0d1de 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -38,7 +38,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 } @@ -68,7 +68,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 } diff --git a/tests/ui/cloned_ref_to_slice_refs.fixed b/tests/ui/cloned_ref_to_slice_refs.fixed new file mode 100644 index 00000000000..818c6e23259 --- /dev/null +++ b/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()]; + } +} diff --git a/tests/ui/cloned_ref_to_slice_refs.rs b/tests/ui/cloned_ref_to_slice_refs.rs new file mode 100644 index 00000000000..9517dbfd156 --- /dev/null +++ b/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()]; + } +} diff --git a/tests/ui/cloned_ref_to_slice_refs.stderr b/tests/ui/cloned_ref_to_slice_refs.stderr new file mode 100644 index 00000000000..6a31d878239 --- /dev/null +++ b/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 + |