add manual_slice_fill lint

CHANGELOG.md

@@ -5772,6 +5772,7 @@ Released 2018-09-13
 [`manual_retain`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain
 [`manual_rotate`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rotate
 [`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
+[`manual_slice_fill`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_slice_fill
 [`manual_slice_size_calculation`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_slice_size_calculation
 [`manual_split_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_split_once
 [`manual_str_repeat`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_str_repeat

book/src/lint_configuration.md

@@ -751,6 +751,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
 * [`manual_range_contains`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains)
 * [`manual_rem_euclid`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid)
 * [`manual_retain`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain)
+* [`manual_slice_fill`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_slice_fill)
 * [`manual_split_once`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_split_once)
 * [`manual_str_repeat`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_str_repeat)
 * [`manual_strip`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_strip)

clippy_config/src/conf.rs

@@ -620,6 +620,7 @@ define_Conf! {
         manual_range_contains,
         manual_rem_euclid,
         manual_retain,
+        manual_slice_fill,
         manual_split_once,
         manual_str_repeat,
         manual_strip,

clippy_lints/src/declared_lints.rs

@@ -291,6 +291,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::loops::MANUAL_FIND_INFO,
     crate::loops::MANUAL_FLATTEN_INFO,
     crate::loops::MANUAL_MEMCPY_INFO,
+    crate::loops::MANUAL_SLICE_FILL_INFO,
     crate::loops::MANUAL_WHILE_LET_SOME_INFO,
     crate::loops::MISSING_SPIN_LOOP_INFO,
     crate::loops::MUT_RANGE_BOUND_INFO,

clippy_lints/src/loops/manual_slice_fill.rs

@@ -0,0 +1,73 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::eager_or_lazy::switch_to_eager_eval;
+use clippy_utils::macros::span_is_local;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::source::{HasSession, snippet_with_applicability};
+use clippy_utils::ty::implements_trait;
+use clippy_utils::{higher, peel_blocks_with_stmt, span_contains_comment};
+use rustc_ast::RangeLimits;
+use rustc_ast::ast::LitKind;
+use rustc_data_structures::packed::Pu128;
+use rustc_errors::Applicability;
+use rustc_hir::QPath::Resolved;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_span::source_map::Spanned;
+use rustc_span::sym;
+
+use super::MANUAL_SLICE_FILL;
+
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    arg: &'tcx Expr<'_>,
+    body: &'tcx Expr<'_>,
+    expr: &'tcx Expr<'_>,
+    msrv: &Msrv,
+) {
+    if msrv.meets(msrvs::SLICE_FILL)
+        && let Some(higher::Range {
+            start: Some(start),
+            end: Some(end),
+            limits: RangeLimits::HalfOpen,
+        }) = higher::Range::hir(arg)
+        && let ExprKind::Lit(Spanned {
+            node: LitKind::Int(Pu128(0), _),
+            ..
+        }) = start.kind
+        && let ExprKind::Block(_, _) = body.kind
+        && let ExprKind::Assign(assignee, assignval, _) = peel_blocks_with_stmt(body).kind
+        && let ExprKind::Index(slice, _, _) = assignee.kind
+        && let ExprKind::MethodCall(path, recv, _, _) = end.kind
+        && let ExprKind::Path(Resolved(_, recvpath)) = recv.kind
+        && let ExprKind::Path(Resolved(_, slicepath)) = slice.kind
+        && switch_to_eager_eval(cx, assignval)
+        && span_is_local(assignval.span)
+        && path.ident.name == sym::len
+        // Ensure that the slice used as the range in the loop matches the slice being manipulated inside the loop.
+        && recvpath.res == slicepath.res
+        && slice.span.eq_ctxt(path.ident.span)
+        // The `fill` method cannot be used if the slice's element type does not implement the `Clone` trait.
+        && let Some(clone_trait) = cx.tcx.lang_items().clone_trait()
+        && implements_trait(cx, cx.typeck_results().expr_ty(slice), clone_trait, &[])
+    {
+        let mut app = if span_contains_comment(cx.sess().source_map(), body.span) {
+            Applicability::MaybeIncorrect // Comments may be informational.
+        } else {
+            Applicability::MachineApplicable
+        };
+
+        span_lint_and_sugg(
+            cx,
+            MANUAL_SLICE_FILL,
+            expr.span,
+            "manually filling a slice",
+            "try",
+            format!(
+                "{}.fill({});",
+                snippet_with_applicability(cx, slice.span, "..", &mut app),
+                snippet_with_applicability(cx, assignval.span, "..", &mut app),
+            ),
+            app,
+        );
+    }
+}

clippy_lints/src/loops/mod.rs

@@ -8,6 +8,7 @@ mod iter_next_loop;
 mod manual_find;
 mod manual_flatten;
 mod manual_memcpy;
+mod manual_slice_fill;
 mod manual_while_let_some;
 mod missing_spin_loop;
 mod mut_range_bound;
@@ -714,6 +715,31 @@ declare_clippy_lint! {
     "possibly unintended infinite loop"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for manually filling a slice with a value.
+    ///
+    /// ### Why is this bad?
+    /// Using the `fill` method is more idiomatic and concise.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let mut some_slice = [1, 2, 3, 4, 5];
+    /// for i in 0..some_slice.len() {
+    ///     some_slice[i] = 0;
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let mut some_slice = [1, 2, 3, 4, 5];
+    /// some_slice.fill(0);
+    /// ```
+    #[clippy::version = "1.86.0"]
+    pub MANUAL_SLICE_FILL,
+    style,
+    "manually filling a slice with a value"
+}
+
 pub struct Loops {
     msrv: Msrv,
     enforce_iter_loop_reborrow: bool,
@@ -750,6 +776,7 @@ impl_lint_pass!(Loops => [
     MANUAL_WHILE_LET_SOME,
     UNUSED_ENUMERATE_INDEX,
     INFINITE_LOOP,
+    MANUAL_SLICE_FILL,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Loops {
@@ -823,6 +850,7 @@ impl Loops {
     ) {
         let is_manual_memcpy_triggered = manual_memcpy::check(cx, pat, arg, body, expr);
         if !is_manual_memcpy_triggered {
+            manual_slice_fill::check(cx, arg, body, expr, &self.msrv);
             needless_range_loop::check(cx, pat, arg, body, expr);
             explicit_counter_loop::check(cx, pat, arg, body, expr, label);
         }

tests/ui/manual_slice_fill.fixed

@@ -0,0 +1,67 @@
+#![warn(clippy::manual_slice_fill)]
+#![allow(clippy::needless_range_loop)]
+
+macro_rules! assign_element {
+    ($slice:ident, $index:expr) => {
+        $slice[$index] = 0;
+    };
+}
+
+fn num() -> usize {
+    5
+}
+
+fn should_lint() {
+    let mut some_slice = [1, 2, 3, 4, 5];
+
+    some_slice.fill(0);
+
+    let x = 5;
+    some_slice.fill(x);
+
+    // This should trigger `manual_slice_fill`, but the applicability is `MaybeIncorrect` since comments
+    // within the loop might be purely informational.
+    some_slice.fill(0);
+}
+
+fn should_not_lint() {
+    let mut some_slice = [1, 2, 3, 4, 5];
+
+    // Should not lint because we can't determine if the scope of the loop is intended to access all the
+    // elements of the slice.
+    for i in 0..5 {
+        some_slice[i] = 0;
+    }
+
+    // Should not lint, as using a function to assign values to elements might be
+    // intentional. For example, the contents of `num()` could be temporary and subject to change
+    // later.
+    for i in 0..some_slice.len() {
+        some_slice[i] = num();
+    }
+
+    // Should not lint because this loop isn't equivalent to `fill`.
+    for i in 0..some_slice.len() {
+        some_slice[i] = 0;
+        println!("foo");
+    }
+
+    // Should not lint because it may be intentional to use a macro to perform an operation equivalent
+    // to `fill`.
+    for i in 0..some_slice.len() {
+        assign_element!(some_slice, i);
+    }
+
+    let another_slice = [1, 2, 3];
+    // Should not lint because the range is not for `some_slice`.
+    for i in 0..another_slice.len() {
+        some_slice[i] = 0;
+    }
+
+    struct NoClone;
+    let mut vec: Vec<Option<NoClone>> = Vec::with_capacity(5);
+    // Should not lint because `NoClone` does not have `Clone` trait.
+    for i in 0..vec.len() {
+        vec[i] = None;
+    }
+}

tests/ui/manual_slice_fill.rs

@@ -0,0 +1,74 @@
+#![warn(clippy::manual_slice_fill)]
+#![allow(clippy::needless_range_loop)]
+
+macro_rules! assign_element {
+    ($slice:ident, $index:expr) => {
+        $slice[$index] = 0;
+    };
+}
+
+fn num() -> usize {
+    5
+}
+
+fn should_lint() {
+    let mut some_slice = [1, 2, 3, 4, 5];
+
+    for i in 0..some_slice.len() {
+        some_slice[i] = 0;
+    }
+
+    let x = 5;
+    for i in 0..some_slice.len() {
+        some_slice[i] = x;
+    }
+
+    // This should trigger `manual_slice_fill`, but the applicability is `MaybeIncorrect` since comments
+    // within the loop might be purely informational.
+    for i in 0..some_slice.len() {
+        some_slice[i] = 0;
+        // foo
+    }
+}
+
+fn should_not_lint() {
+    let mut some_slice = [1, 2, 3, 4, 5];
+
+    // Should not lint because we can't determine if the scope of the loop is intended to access all the
+    // elements of the slice.
+    for i in 0..5 {
+        some_slice[i] = 0;
+    }
+
+    // Should not lint, as using a function to assign values to elements might be
+    // intentional. For example, the contents of `num()` could be temporary and subject to change
+    // later.
+    for i in 0..some_slice.len() {
+        some_slice[i] = num();
+    }
+
+    // Should not lint because this loop isn't equivalent to `fill`.
+    for i in 0..some_slice.len() {
+        some_slice[i] = 0;
+        println!("foo");
+    }
+
+    // Should not lint because it may be intentional to use a macro to perform an operation equivalent
+    // to `fill`.
+    for i in 0..some_slice.len() {
+        assign_element!(some_slice, i);
+    }
+
+    let another_slice = [1, 2, 3];
+    // Should not lint because the range is not for `some_slice`.
+    for i in 0..another_slice.len() {
+        some_slice[i] = 0;
+    }
+
+    struct NoClone;
+    let mut vec: Vec<Option<NoClone>> = Vec::with_capacity(5);
+    // Should not lint because `NoClone` does not have `Clone` trait.
+    for i in 0..vec.len() {
+        vec[i] = None;
+    }
+}

tests/ui/manual_slice_fill.stderr

@@ -0,0 +1,30 @@
+error: manually filling a slice
+  --> tests/ui/manual_slice_fill.rs:17:5
+   |
+LL | /     for i in 0..some_slice.len() {
+LL | |         some_slice[i] = 0;
+LL | |     }
+   | |_____^ help: try: `some_slice.fill(0);`
+   |
+   = note: `-D clippy::manual-slice-fill` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::manual_slice_fill)]`
+
+error: manually filling a slice
+  --> tests/ui/manual_slice_fill.rs:22:5
+   |
+LL | /     for i in 0..some_slice.len() {
+LL | |         some_slice[i] = x;
+LL | |     }
+   | |_____^ help: try: `some_slice.fill(x);`
+
+error: manually filling a slice
+  --> tests/ui/manual_slice_fill.rs:28:5
+   |
+LL | /     for i in 0..some_slice.len() {
+LL | |         some_slice[i] = 0;
+LL | |         // foo
+LL | |     }
+   | |_____^ help: try: `some_slice.fill(0);`
+
+error: aborting due to 3 previous errors
+