add a lint for redundant repeat().take()

rust-lang · Dec 20, 2024 · 33a5b21 · 33a5b21
1 parent 0f9cc8d
commit 33a5b21
Show file tree

Hide file tree

Showing 13 changed files with 147 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5954,6 +5954,7 @@ Released 2018-09-13
 [`redundant_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern
 [`redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching
 [`redundant_pub_crate`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pub_crate
+[`redundant_repeat_take`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_repeat_take
 [`redundant_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing
 [`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
 [`redundant_type_annotations`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_type_annotations

diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
@@ -456,6 +456,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::methods::READONLY_WRITE_LOCK_INFO,
     crate::methods::READ_LINE_WITHOUT_TRIM_INFO,
     crate::methods::REDUNDANT_AS_STR_INFO,
+    crate::methods::REDUNDANT_REPEAT_TAKE_INFO,
     crate::methods::REPEAT_ONCE_INFO,
     crate::methods::RESULT_FILTER_MAP_INFO,
     crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO,

diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
@@ -91,6 +91,7 @@ mod range_zip_with_len;
 mod read_line_without_trim;
 mod readonly_write_lock;
 mod redundant_as_str;
+mod redundant_repeat_take;
 mod repeat_once;
 mod result_map_or_else_none;
 mod search_is_some;
@@ -4284,6 +4285,29 @@ declare_clippy_lint! {
     "map of a trivial closure (not dependent on parameter) over a range"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Checks for `repeat().take()` that can be replaced with `repeat_n()`.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Using `repeat_n()` is more concise and clearer.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let _ = std::iter::repeat(10).take(3);
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let _ = std::iter::repeat_n(10, 3);
+    /// ```
+    #[clippy::version = "1.85.0"]
+    pub REDUNDANT_REPEAT_TAKE,
+    style,
+    "detect `repeat().take()` that can be replaced with `repeat_n()`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -4449,6 +4473,7 @@ impl_lint_pass!(Methods => [
     MAP_ALL_ANY_IDENTITY,
     MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
     UNNECESSARY_MAP_OR,
+    REDUNDANT_REPEAT_TAKE,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -5117,6 +5142,7 @@ impl Methods {
                 ("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg),
                 ("take", [arg]) => {
                     iter_out_of_bounds::check_take(cx, expr, recv, arg);
+                    redundant_repeat_take::check(cx, expr, recv, arg);
                     if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) {
                         iter_overeager_cloned::check(
                             cx,

diff --git a/clippy_lints/src/methods/redundant_repeat_take.rs b/clippy_lints/src/methods/redundant_repeat_take.rs
@@ -0,0 +1,32 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, QPath};
+use rustc_lint::LateContext;
+
+use super::REDUNDANT_REPEAT_TAKE;
+
+pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, repeat_expr: &Expr<'_>, take_arg: &Expr<'_>) {
+    if !expr.span.from_expansion()
+        && let ExprKind::Call(inner, repeat_args) = repeat_expr.kind
+        && repeat_args.len() == 1
+        && let ExprKind::Path(ref qpath) = inner.kind
+        && let QPath::Resolved(_, path) = qpath
+        && path.segments.len() == 1
+        && path.segments[0].ident.name.as_str() == "repeat"
+    {
+        span_lint_and_sugg(
+            cx,
+            REDUNDANT_REPEAT_TAKE,
+            expr.span,
+            "redundant `repeat().take()`",
+            "consider using `repeat_n()` instead",
+            format!(
+                "std::iter::repeat_n({}, {})",
+                snippet(cx, repeat_args[0].span, ""),
+                snippet(cx, take_arg.span, "")
+            ),
+            Applicability::MachineApplicable,
+        );
+    }
+}
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
@@ -15,6 +15,7 @@
     clippy::missing_errors_doc,
     clippy::missing_panics_doc,
     clippy::must_use_candidate,
+    clippy::redundant_repeat_take,
     rustc::diagnostic_outside_of_impl,
     rustc::untranslatable_diagnostic
 )]

diff --git a/tests/ui/manual_str_repeat.fixed b/tests/ui/manual_str_repeat.fixed
@@ -1,4 +1,4 @@
-#![allow(non_local_definitions)]
+#![allow(non_local_definitions, clippy::redundant_repeat_take)]
 #![warn(clippy::manual_str_repeat)]
 
 use std::borrow::Cow;

diff --git a/tests/ui/manual_str_repeat.rs b/tests/ui/manual_str_repeat.rs
@@ -1,4 +1,4 @@
-#![allow(non_local_definitions)]
+#![allow(non_local_definitions, redundant_repeat_take)]
 #![warn(clippy::manual_str_repeat)]
 
 use std::borrow::Cow;

diff --git a/tests/ui/manual_str_repeat.stderr b/tests/ui/manual_str_repeat.stderr
@@ -1,3 +1,28 @@
+error: lint name `redundant_repeat_take` is deprecated and may not have an effect in the future
+  --> tests/ui/manual_str_repeat.rs:1:33
+   |
+LL | #![allow(non_local_definitions, redundant_repeat_take)]
+   |                                 ^^^^^^^^^^^^^^^^^^^^^ help: change it to: `clippy::redundant_repeat_take`
+   |
+   = note: `-D renamed-and-removed-lints` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(renamed_and_removed_lints)]`
+
+error: lint name `redundant_repeat_take` is deprecated and may not have an effect in the future
+  --> tests/ui/manual_str_repeat.rs:1:33
+   |
+LL | #![allow(non_local_definitions, redundant_repeat_take)]
+   |                                 ^^^^^^^^^^^^^^^^^^^^^ help: change it to: `clippy::redundant_repeat_take`
+   |
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error: lint name `redundant_repeat_take` is deprecated and may not have an effect in the future
+  --> tests/ui/manual_str_repeat.rs:1:33
+   |
+LL | #![allow(non_local_definitions, redundant_repeat_take)]
+   |                                 ^^^^^^^^^^^^^^^^^^^^^ help: change it to: `clippy::redundant_repeat_take`
+   |
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
 error: manual implementation of `str::repeat` using iterators
   --> tests/ui/manual_str_repeat.rs:8:21
    |
@@ -61,5 +86,5 @@ error: manual implementation of `str::repeat` using iterators
 LL |     let _: String = std::iter::repeat("test").take(10).collect();
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `"test".repeat(10)`
 
-error: aborting due to 10 previous errors
+error: aborting due to 13 previous errors
 
diff --git a/tests/ui/redundant_repeat_take.fixed b/tests/ui/redundant_repeat_take.fixed
@@ -0,0 +1,13 @@
+#![warn(clippy::redundant_repeat_take)]
+
+use std::iter::repeat;
+
+fn main() {
+    let _ = std::iter::repeat_n(10, 3);
+
+    let _ = std::iter::repeat_n(String::from("foo"), 4);
+
+    for value in std::iter::repeat(5).take(3) {}
+
+    let _: Vec<_> = std::iter::repeat(String::from("bar")).take(10).collect();
+}
diff --git a/tests/ui/redundant_repeat_take.rs b/tests/ui/redundant_repeat_take.rs
@@ -0,0 +1,13 @@
+#![warn(clippy::redundant_repeat_take)]
+
+use std::iter::repeat;
+
+fn main() {
+    let _ = repeat(10).take(3);
+
+    let _ = repeat(String::from("foo")).take(4);
+
+    for value in std::iter::repeat(5).take(3) {}
+
+    let _: Vec<_> = std::iter::repeat(String::from("bar")).take(10).collect();
+}
diff --git a/tests/ui/redundant_repeat_take.stderr b/tests/ui/redundant_repeat_take.stderr
@@ -0,0 +1,17 @@
+error: redundant `repeat().take()`
+  --> tests/ui/redundant_repeat_take.rs:6:13
+   |
+LL |     let _ = repeat(10).take(3);
+   |             ^^^^^^^^^^^^^^^^^^ help: consider using `repeat_n()` instead: `std::iter::repeat_n(10, 3)`
+   |
+   = note: `-D clippy::redundant-repeat-take` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::redundant_repeat_take)]`
+
+error: redundant `repeat().take()`
+  --> tests/ui/redundant_repeat_take.rs:8:13
+   |
+LL |     let _ = repeat(String::from("foo")).take(4);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `repeat_n()` instead: `std::iter::repeat_n(String::from("foo"), 4)`
+
+error: aborting due to 2 previous errors
+
diff --git a/tests/ui/slow_vector_initialization.rs b/tests/ui/slow_vector_initialization.rs
@@ -1,3 +1,5 @@
+#![allow(clippy::redundant_repeat_take)]
+
 //@no-rustfix
 use std::iter::repeat;
 fn main() {

diff --git a/tests/ui/slow_vector_initialization.stderr b/tests/ui/slow_vector_initialization.stderr
@@ -1,5 +1,5 @@
 error: slow zero-filling initialization
-  --> tests/ui/slow_vector_initialization.rs:14:5
+  --> tests/ui/slow_vector_initialization.rs:16:5
    |
 LL |     let mut vec1 = Vec::with_capacity(len);
    |                    ----------------------- help: consider replacing this with: `vec![0; len]`
@@ -10,95 +10,95 @@ LL |     vec1.extend(repeat(0).take(len));
    = help: to override `-D warnings` add `#[allow(clippy::slow_vector_initialization)]`
 
 error: slow zero-filling initialization
-  --> tests/ui/slow_vector_initialization.rs:20:5
+  --> tests/ui/slow_vector_initialization.rs:22:5
    |
 LL |     let mut vec2 = Vec::with_capacity(len - 10);
    |                    ---------------------------- help: consider replacing this with: `vec![0; len - 10]`
 LL |     vec2.extend(repeat(0).take(len - 10));
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: slow zero-filling initialization
-  --> tests/ui/slow_vector_initialization.rs:28:5
+  --> tests/ui/slow_vector_initialization.rs:30:5
    |
 LL |     let mut vec4 = Vec::with_capacity(len);
    |                    ----------------------- help: consider replacing this with: `vec![0; len]`
 LL |     vec4.extend(repeat(0).take(vec4.capacity()));
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: slow zero-filling initialization
-  --> tests/ui/slow_vector_initialization.rs:39:5
+  --> tests/ui/slow_vector_initialization.rs:41:5
    |
 LL |     let mut resized_vec = Vec::with_capacity(30);
    |                           ---------------------- help: consider replacing this with: `vec![0; 30]`
 LL |     resized_vec.resize(30, 0);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: slow zero-filling initialization
-  --> tests/ui/slow_vector_initialization.rs:43:5
+  --> tests/ui/slow_vector_initialization.rs:45:5
    |
 LL |     let mut extend_vec = Vec::with_capacity(30);
    |                          ---------------------- help: consider replacing this with: `vec![0; 30]`
 LL |     extend_vec.extend(repeat(0).take(30));
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: slow zero-filling initialization
-  --> tests/ui/slow_vector_initialization.rs:51:5
+  --> tests/ui/slow_vector_initialization.rs:53:5
    |
 LL |     let mut vec1 = Vec::with_capacity(len);
    |                    ----------------------- help: consider replacing this with: `vec![0; len]`
 LL |     vec1.resize(len, 0);
    |     ^^^^^^^^^^^^^^^^^^^
 
 error: slow zero-filling initialization
-  --> tests/ui/slow_vector_initialization.rs:60:5
+  --> tests/ui/slow_vector_initialization.rs:62:5
    |
 LL |     let mut vec3 = Vec::with_capacity(len - 10);
    |                    ---------------------------- help: consider replacing this with: `vec![0; len - 10]`
 LL |     vec3.resize(len - 10, 0);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: slow zero-filling initialization
-  --> tests/ui/slow_vector_initialization.rs:64:5
+  --> tests/ui/slow_vector_initialization.rs:66:5
    |
 LL |     let mut vec4 = Vec::with_capacity(len);
    |                    ----------------------- help: consider replacing this with: `vec![0; len]`
 LL |     vec4.resize(vec4.capacity(), 0);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: slow zero-filling initialization
-  --> tests/ui/slow_vector_initialization.rs:69:5
+  --> tests/ui/slow_vector_initialization.rs:71:5
    |
 LL |     vec1 = Vec::with_capacity(10);
    |            ---------------------- help: consider replacing this with: `vec![0; 10]`
 LL |     vec1.resize(10, 0);
    |     ^^^^^^^^^^^^^^^^^^
 
 error: slow zero-filling initialization
-  --> tests/ui/slow_vector_initialization.rs:77:5
+  --> tests/ui/slow_vector_initialization.rs:79:5
    |
 LL |     let mut vec1 = Vec::new();
    |                    ---------- help: consider replacing this with: `vec![0; len]`
 LL |     vec1.resize(len, 0);
    |     ^^^^^^^^^^^^^^^^^^^
 
 error: slow zero-filling initialization
-  --> tests/ui/slow_vector_initialization.rs:82:5
+  --> tests/ui/slow_vector_initialization.rs:84:5
    |
 LL |     let mut vec3 = Vec::new();
    |                    ---------- help: consider replacing this with: `vec![0; len - 10]`
 LL |     vec3.resize(len - 10, 0);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: slow zero-filling initialization
-  --> tests/ui/slow_vector_initialization.rs:87:5
+  --> tests/ui/slow_vector_initialization.rs:89:5
    |
 LL |     vec1 = Vec::new();
    |            ---------- help: consider replacing this with: `vec![0; 10]`
 LL |     vec1.resize(10, 0);
    |     ^^^^^^^^^^^^^^^^^^
 
 error: slow zero-filling initialization
-  --> tests/ui/slow_vector_initialization.rs:91:5
+  --> tests/ui/slow_vector_initialization.rs:93:5
    |
 LL |     vec1 = vec![];
    |            ------ help: consider replacing this with: `vec![0; 10]`