about summary refs log tree commit diff
path: root/clippy_lints/src/loops
diff options
context:
space:
mode:
authorPhilipp Krones <hello@philkrones.com>2025-02-06 14:31:01 +0100
committerPhilipp Krones <hello@philkrones.com>2025-02-06 14:31:01 +0100
commitf549562b81b6d3bfe347b32335bef26f9a051429 (patch)
tree1f67a5f1155b060e62de5ffa4a2051ed9034bdf4 /clippy_lints/src/loops
parent4c11087e517f10ee35bbe1829e3e580fc5ca1e01 (diff)
parent660d86105891a4b2a686e56ad2171c743faca88c (diff)
Merge remote-tracking branch 'upstream/master' into rustup
Diffstat (limited to 'clippy_lints/src/loops')
-rw-r--r--clippy_lints/src/loops/manual_slice_fill.rs111
-rw-r--r--clippy_lints/src/loops/mod.rs28
2 files changed, 139 insertions, 0 deletions
diff --git a/clippy_lints/src/loops/manual_slice_fill.rs b/clippy_lints/src/loops/manual_slice_fill.rs
new file mode 100644
index 00000000000..7c656423579
--- /dev/null
+++ b/clippy_lints/src/loops/manual_slice_fill.rs
@@ -0,0 +1,111 @@
+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::ast::LitKind;
+use rustc_ast::{RangeLimits, UnOp};
+use rustc_data_structures::packed::Pu128;
+use rustc_errors::Applicability;
+use rustc_hir::QPath::Resolved;
+use rustc_hir::def::Res;
+use rustc_hir::{Expr, ExprKind, Pat};
+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>,
+    pat: &'tcx Pat<'_>,
+    arg: &'tcx Expr<'_>,
+    body: &'tcx Expr<'_>,
+    expr: &'tcx Expr<'_>,
+    msrv: &Msrv,
+) {
+    if !msrv.meets(msrvs::SLICE_FILL) {
+        return;
+    }
+
+    // `for _ in 0..slice.len() { slice[_] = value; }`
+    if 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
+        // Check if the body is an assignment to a slice element.
+        && let ExprKind::Assign(assignee, assignval, _) = peel_blocks_with_stmt(body).kind
+        && let ExprKind::Index(slice, _, _) = assignee.kind
+        // Check if `len()` is used for the range end.
+        && let ExprKind::MethodCall(path, recv,..) = end.kind
+        && path.ident.name == sym::len
+        // Check if the slice which is being assigned to is the same as the one being iterated over.
+        && let ExprKind::Path(Resolved(_, recv_path)) = recv.kind
+        && let ExprKind::Path(Resolved(_, slice_path)) = slice.kind
+        && recv_path.res == slice_path.res
+        && !assignval.span.from_expansion()
+        // It is generally not equivalent to use the `fill` method if `assignval` can have side effects
+        && switch_to_eager_eval(cx, assignval)
+        && span_is_local(assignval.span)
+        // The `fill` method requires that the slice's element type implements the `Clone` trait.
+        && let Some(clone_trait) = cx.tcx.lang_items().clone_trait()
+        && implements_trait(cx, cx.typeck_results().expr_ty(slice), clone_trait, &[])
+    {
+        sugg(cx, body, expr, slice.span, assignval.span);
+    }
+    // `for _ in &mut slice { *_ = value; }`
+    else if let ExprKind::AddrOf(_, _, recv) = arg.kind
+        // Check if the body is an assignment to a slice element.
+        && let ExprKind::Assign(assignee, assignval, _) = peel_blocks_with_stmt(body).kind
+        && let ExprKind::Unary(UnOp::Deref, slice_iter) = assignee.kind
+        && let ExprKind::Path(Resolved(_, recv_path)) = recv.kind
+        // Check if the slice which is being assigned to is the same as the one being iterated over.
+        && let ExprKind::Path(Resolved(_, slice_path)) = slice_iter.kind
+        && let Res::Local(local) = slice_path.res
+        && local == pat.hir_id
+        && !assignval.span.from_expansion()
+        && switch_to_eager_eval(cx, assignval)
+        && span_is_local(assignval.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(recv), clone_trait, &[])
+    {
+        sugg(cx, body, expr, recv_path.span, assignval.span);
+    }
+}
+
+fn sugg<'tcx>(
+    cx: &LateContext<'tcx>,
+    body: &'tcx Expr<'_>,
+    expr: &'tcx Expr<'_>,
+    slice_span: rustc_span::Span,
+    assignval_span: rustc_span::Span,
+) {
+    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,
+    );
+}
diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs
index c5e75af2303..cdc8c18c3b7 100644
--- a/clippy_lints/src/loops/mod.rs
+++ b/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, pat, arg, body, expr, &self.msrv);
             needless_range_loop::check(cx, pat, arg, body, expr);
             explicit_counter_loop::check(cx, pat, arg, body, expr, label);
         }