about summary refs log tree commit diff
diff options
context:
space:
mode:
authorFridtjof Stoldt <xFrednet@gmail.com>2025-01-22 14:41:13 +0000
committerGitHub <noreply@github.com>2025-01-22 14:41:13 +0000
commit67e6bf33fee137e8419863342bac0eecaae0f221 (patch)
tree8bf5d480c745ceacc372e58f815d4ee56632b476
parent88d83b6e55ff3bf0b50f01d674c54b83acd21dc6 (diff)
parentded9354dcf76aaa2cd2164a8b556f61c2f188d79 (diff)
downloadrust-67e6bf33fee137e8419863342bac0eecaae0f221.tar.gz
rust-67e6bf33fee137e8419863342bac0eecaae0f221.zip
Suggest using `Vec::extend()` in `same_item_push` (#13987)
Using `Vec::extend(std::iter::repeat_n(item, N))` allows to use the more
natural number of elements to add `N`, as is probably done in the
original loop, instead of computing the difference between the existing
number of elements and the wanted one.

Before MSRV 1.82, the older suggestion to use `Vec::resize()` is still
issued.

Inspired by #6156 (which predates `repeat_n()`).

changelog: [`same_item_push`]: recommend using `Vec::extend()` to extend
a vector
-rw-r--r--book/src/lint_configuration.md1
-rw-r--r--clippy_config/src/conf.rs1
-rw-r--r--clippy_lints/src/loops/mod.rs2
-rw-r--r--clippy_lints/src/loops/same_item_push.rs34
-rw-r--r--tests/ui/same_item_push.rs20
-rw-r--r--tests/ui/same_item_push.stderr36
6 files changed, 66 insertions, 28 deletions
diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md
index 181e794e6e4..a3e10088db2 100644
--- a/book/src/lint_configuration.md
+++ b/book/src/lint_configuration.md
@@ -767,6 +767,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
 * [`ptr_as_ptr`](https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr)
 * [`redundant_field_names`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names)
 * [`redundant_static_lifetimes`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes)
+* [`same_item_push`](https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push)
 * [`seek_from_current`](https://rust-lang.github.io/rust-clippy/master/index.html#seek_from_current)
 * [`seek_rewind`](https://rust-lang.github.io/rust-clippy/master/index.html#seek_rewind)
 * [`transmute_ptr_to_ref`](https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ref)
diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs
index c616589c56e..9bf6c675f00 100644
--- a/clippy_config/src/conf.rs
+++ b/clippy_config/src/conf.rs
@@ -636,6 +636,7 @@ define_Conf! {
         ptr_as_ptr,
         redundant_field_names,
         redundant_static_lifetimes,
+        same_item_push,
         seek_from_current,
         seek_rewind,
         transmute_ptr_to_ref,
diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs
index f3ca4a4a571..c5e75af2303 100644
--- a/clippy_lints/src/loops/mod.rs
+++ b/clippy_lints/src/loops/mod.rs
@@ -830,7 +830,7 @@ impl Loops {
         for_kv_map::check(cx, pat, arg, body);
         mut_range_bound::check(cx, arg, body);
         single_element_loop::check(cx, pat, arg, body, expr);
-        same_item_push::check(cx, pat, arg, body, expr);
+        same_item_push::check(cx, pat, arg, body, expr, &self.msrv);
         manual_flatten::check(cx, pat, arg, body, span);
         manual_find::check(cx, pat, arg, body, span, expr);
         unused_enumerate_index::check(cx, pat, arg, body);
diff --git a/clippy_lints/src/loops/same_item_push.rs b/clippy_lints/src/loops/same_item_push.rs
index 951ebc9caef..c27e930c99a 100644
--- a/clippy_lints/src/loops/same_item_push.rs
+++ b/clippy_lints/src/loops/same_item_push.rs
@@ -1,8 +1,9 @@
 use super::SAME_ITEM_PUSH;
-use clippy_utils::diagnostics::span_lint_and_help;
-use clippy_utils::path_to_local;
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::msrvs::Msrv;
 use clippy_utils::source::snippet_with_context;
 use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
+use clippy_utils::{msrvs, path_to_local, std_or_core};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
@@ -19,19 +20,30 @@ pub(super) fn check<'tcx>(
     _: &'tcx Expr<'_>,
     body: &'tcx Expr<'_>,
     _: &'tcx Expr<'_>,
+    msrv: &Msrv,
 ) {
-    fn emit_lint(cx: &LateContext<'_>, vec: &Expr<'_>, pushed_item: &Expr<'_>, ctxt: SyntaxContext) {
+    fn emit_lint(cx: &LateContext<'_>, vec: &Expr<'_>, pushed_item: &Expr<'_>, ctxt: SyntaxContext, msrv: &Msrv) {
         let mut app = Applicability::Unspecified;
         let vec_str = snippet_with_context(cx, vec.span, ctxt, "", &mut app).0;
         let item_str = snippet_with_context(cx, pushed_item.span, ctxt, "", &mut app).0;
 
-        span_lint_and_help(
+        let secondary_help = if msrv.meets(msrvs::REPEAT_N)
+            && let Some(std_or_core) = std_or_core(cx)
+        {
+            format!("or `{vec_str}.extend({std_or_core}::iter::repeat_n({item_str}, SIZE))`")
+        } else {
+            format!("or `{vec_str}.resize(NEW_SIZE, {item_str})`")
+        };
+
+        span_lint_and_then(
             cx,
             SAME_ITEM_PUSH,
             vec.span,
-            "it looks like the same item is being pushed into this Vec",
-            None,
-            format!("consider using vec![{item_str};SIZE] or {vec_str}.resize(NEW_SIZE, {item_str})"),
+            "it looks like the same item is being pushed into this `Vec`",
+            |diag| {
+                diag.help(format!("consider using `vec![{item_str};SIZE]`"))
+                    .help(secondary_help);
+            },
         );
     }
 
@@ -67,11 +79,11 @@ pub(super) fn check<'tcx>(
                         {
                             match init.kind {
                                 // immutable bindings that are initialized with literal
-                                ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item, ctxt),
+                                ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item, ctxt, msrv),
                                 // immutable bindings that are initialized with constant
                                 ExprKind::Path(ref path) => {
                                     if let Res::Def(DefKind::Const, ..) = cx.qpath_res(path, init.hir_id) {
-                                        emit_lint(cx, vec, pushed_item, ctxt);
+                                        emit_lint(cx, vec, pushed_item, ctxt, msrv);
                                     }
                                 },
                                 _ => {},
@@ -79,11 +91,11 @@ pub(super) fn check<'tcx>(
                         }
                     },
                     // constant
-                    Res::Def(DefKind::Const, ..) => emit_lint(cx, vec, pushed_item, ctxt),
+                    Res::Def(DefKind::Const, ..) => emit_lint(cx, vec, pushed_item, ctxt, msrv),
                     _ => {},
                 }
             },
-            ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item, ctxt),
+            ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item, ctxt, msrv),
             _ => {},
         }
     }
diff --git a/tests/ui/same_item_push.rs b/tests/ui/same_item_push.rs
index df9c2817f50..87fd59ad317 100644
--- a/tests/ui/same_item_push.rs
+++ b/tests/ui/same_item_push.rs
@@ -21,33 +21,43 @@ fn main() {
     let item = 2;
     for _ in 5..=20 {
         vec.push(item);
-        //~^ ERROR: it looks like the same item is being pushed into this Vec
+        //~^ ERROR: it looks like the same item is being pushed into this `Vec`
     }
 
     let mut vec: Vec<u8> = Vec::new();
     for _ in 0..15 {
         let item = 2;
         vec.push(item);
-        //~^ ERROR: it looks like the same item is being pushed into this Vec
+        //~^ ERROR: it looks like the same item is being pushed into this `Vec`
     }
 
     let mut vec: Vec<u8> = Vec::new();
     for _ in 0..15 {
         vec.push(13);
-        //~^ ERROR: it looks like the same item is being pushed into this Vec
+        //~^ ERROR: it looks like the same item is being pushed into this `Vec`
     }
 
     let mut vec = Vec::new();
     for _ in 0..20 {
         vec.push(VALUE);
-        //~^ ERROR: it looks like the same item is being pushed into this Vec
+        //~^ ERROR: it looks like the same item is being pushed into this `Vec`
     }
 
     let mut vec = Vec::new();
     let item = VALUE;
     for _ in 0..20 {
         vec.push(item);
-        //~^ ERROR: it looks like the same item is being pushed into this Vec
+        //~^ ERROR: it looks like the same item is being pushed into this `Vec`
+    }
+
+    #[clippy::msrv = "1.81"]
+    fn older_msrv() {
+        let mut vec = Vec::new();
+        let item = VALUE;
+        for _ in 0..20 {
+            vec.push(item);
+            //~^ ERROR: it looks like the same item is being pushed into this `Vec`
+        }
     }
 
     // ** non-linted cases **
diff --git a/tests/ui/same_item_push.stderr b/tests/ui/same_item_push.stderr
index eb296ed4ce4..e3fa4f9cbce 100644
--- a/tests/ui/same_item_push.stderr
+++ b/tests/ui/same_item_push.stderr
@@ -1,44 +1,58 @@
-error: it looks like the same item is being pushed into this Vec
+error: it looks like the same item is being pushed into this `Vec`
   --> tests/ui/same_item_push.rs:23:9
    |
 LL |         vec.push(item);
    |         ^^^
    |
-   = help: consider using vec![item;SIZE] or vec.resize(NEW_SIZE, item)
+   = help: consider using `vec![item;SIZE]`
+   = help: or `vec.extend(std::iter::repeat_n(item, SIZE))`
    = note: `-D clippy::same-item-push` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::same_item_push)]`
 
-error: it looks like the same item is being pushed into this Vec
+error: it looks like the same item is being pushed into this `Vec`
   --> tests/ui/same_item_push.rs:30:9
    |
 LL |         vec.push(item);
    |         ^^^
    |
-   = help: consider using vec![item;SIZE] or vec.resize(NEW_SIZE, item)
+   = help: consider using `vec![item;SIZE]`
+   = help: or `vec.extend(std::iter::repeat_n(item, SIZE))`
 
-error: it looks like the same item is being pushed into this Vec
+error: it looks like the same item is being pushed into this `Vec`
   --> tests/ui/same_item_push.rs:36:9
    |
 LL |         vec.push(13);
    |         ^^^
    |
-   = help: consider using vec![13;SIZE] or vec.resize(NEW_SIZE, 13)
+   = help: consider using `vec![13;SIZE]`
+   = help: or `vec.extend(std::iter::repeat_n(13, SIZE))`
 
-error: it looks like the same item is being pushed into this Vec
+error: it looks like the same item is being pushed into this `Vec`
   --> tests/ui/same_item_push.rs:42:9
    |
 LL |         vec.push(VALUE);
    |         ^^^
    |
-   = help: consider using vec![VALUE;SIZE] or vec.resize(NEW_SIZE, VALUE)
+   = help: consider using `vec![VALUE;SIZE]`
+   = help: or `vec.extend(std::iter::repeat_n(VALUE, SIZE))`
 
-error: it looks like the same item is being pushed into this Vec
+error: it looks like the same item is being pushed into this `Vec`
   --> tests/ui/same_item_push.rs:49:9
    |
 LL |         vec.push(item);
    |         ^^^
    |
-   = help: consider using vec![item;SIZE] or vec.resize(NEW_SIZE, item)
+   = help: consider using `vec![item;SIZE]`
+   = help: or `vec.extend(std::iter::repeat_n(item, SIZE))`
 
-error: aborting due to 5 previous errors
+error: it looks like the same item is being pushed into this `Vec`
+  --> tests/ui/same_item_push.rs:58:13
+   |
+LL |             vec.push(item);
+   |             ^^^
+   |
+   = help: consider using `vec![item;SIZE]`
+   = help: or `vec.resize(NEW_SIZE, item)`
+
+error: aborting due to 6 previous errors