Skip to content

Commit

Permalink
add a lint for repeat().take()
Browse files Browse the repository at this point in the history
  • Loading branch information
lapla-cogito committed Dec 20, 2024
1 parent 0f9cc8d commit 8e422ea
Show file tree
Hide file tree
Showing 15 changed files with 159 additions and 19 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5725,6 +5725,7 @@ Released 2018-09-13
[`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
[`manual_range_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_patterns
[`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid
[`manual_repeat_n`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_repeat_n
[`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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::methods::MANUAL_IS_VARIANT_AND_INFO,
crate::methods::MANUAL_NEXT_BACK_INFO,
crate::methods::MANUAL_OK_OR_INFO,
crate::methods::MANUAL_REPEAT_N_INFO,
crate::methods::MANUAL_SATURATING_ARITHMETIC_INFO,
crate::methods::MANUAL_SPLIT_ONCE_INFO,
crate::methods::MANUAL_STR_REPEAT_INFO,
Expand Down
35 changes: 35 additions & 0 deletions clippy_lints/src/methods/manual_repeat_n.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::source::{snippet, snippet_with_context};
use clippy_utils::{fn_def_id, is_trait_method};
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_span::sym;

use super::MANUAL_REPEAT_N;

pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, repeat_expr: &Expr<'_>, take_arg: &Expr<'_>, msrv: &Msrv) {
if msrv.meets(msrvs::REPEAT_N)
&& !expr.span.from_expansion()
&& is_trait_method(cx, expr, sym::Iterator)
&& let ExprKind::Call(_, [repeat_arg]) = repeat_expr.kind
&& let Some(def_id) = fn_def_id(cx, repeat_expr)
&& cx.tcx.is_diagnostic_item(sym::iter_repeat, def_id)
{
let mut app = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
MANUAL_REPEAT_N,
expr.span,
"this `.repeat().take()` can be written more concisely",
"consider using `repeat_n()` instead",
format!(
"std::iter::repeat_n({}, {})",
snippet_with_context(cx, repeat_arg.span, expr.span.ctxt(), "..", &mut app).0,
snippet(cx, take_arg.span, "")
),
Applicability::MachineApplicable,
);
}
}
26 changes: 26 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ mod manual_inspect;
mod manual_is_variant_and;
mod manual_next_back;
mod manual_ok_or;
mod manual_repeat_n;
mod manual_saturating_arithmetic;
mod manual_str_repeat;
mod manual_try_fold;
Expand Down Expand Up @@ -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 MANUAL_REPEAT_N,
style,
"detect `repeat().take()` that can be replaced with `repeat_n()`"
}

pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Msrv,
Expand Down Expand Up @@ -4449,6 +4473,7 @@ impl_lint_pass!(Methods => [
MAP_ALL_ANY_IDENTITY,
MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
UNNECESSARY_MAP_OR,
MANUAL_REPEAT_N,
]);

/// Extracts a method call name, args, and `Span` of the method name.
Expand Down Expand Up @@ -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);
manual_repeat_n::check(cx, expr, recv, arg, &self.msrv);
if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) {
iter_overeager_cloned::check(
cx,
Expand Down
1 change: 1 addition & 0 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
clippy::missing_errors_doc,
clippy::missing_panics_doc,
clippy::must_use_candidate,
clippy::manual_repeat_n,
rustc::diagnostic_outside_of_impl,
rustc::untranslatable_diagnostic
)]
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/from_iter_instead_of_collect.fixed
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![warn(clippy::from_iter_instead_of_collect)]
#![warn(clippy::from_iter_instead_of_collect, clippy::manual_repeat_n)]
#![allow(unused_imports)]
#![allow(clippy::useless_vec)]

Expand All @@ -19,7 +19,7 @@ impl<'a> FromIterator<&'a bool> for Foo {
}

fn main() {
let iter_expr = std::iter::repeat(5).take(5);
let iter_expr = std::iter::repeat_n(5, 5);
let _ = iter_expr.collect::<Vec<_>>();

let _ = vec![5, 5, 5, 5].iter().enumerate().collect::<HashMap<usize, &i8>>();
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/from_iter_instead_of_collect.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![warn(clippy::from_iter_instead_of_collect)]
#![warn(clippy::from_iter_instead_of_collect, clippy::manual_repeat_n)]
#![allow(unused_imports)]
#![allow(clippy::useless_vec)]

Expand Down
11 changes: 10 additions & 1 deletion tests/ui/from_iter_instead_of_collect.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@ LL | <Self as FromIterator<bool>>::from_iter(iter.into_iter().copied())
= note: `-D clippy::from-iter-instead-of-collect` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::from_iter_instead_of_collect)]`

error: this `.repeat().take()` can be written more concisely
--> tests/ui/from_iter_instead_of_collect.rs:22:21
|
LL | let iter_expr = std::iter::repeat(5).take(5);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `repeat_n()` instead: `std::iter::repeat_n(5, 5)`
|
= note: `-D clippy::manual-repeat-n` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::manual_repeat_n)]`

error: usage of `FromIterator::from_iter`
--> tests/ui/from_iter_instead_of_collect.rs:23:13
|
Expand Down Expand Up @@ -91,5 +100,5 @@ error: usage of `FromIterator::from_iter`
LL | for _i in Vec::<&i32>::from_iter([1, 2, 3].iter()) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `[1, 2, 3].iter().collect::<Vec<&i32>>()`

error: aborting due to 15 previous errors
error: aborting due to 16 previous errors

15 changes: 15 additions & 0 deletions tests/ui/manual_repeat_n.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#![warn(clippy::manual_repeat_n)]

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_n(5, 3) {}

let _: Vec<_> = std::iter::repeat_n(String::from("bar"), 10).collect();

let _ = std::iter::repeat_n(vec![1, 2], 2);
}
15 changes: 15 additions & 0 deletions tests/ui/manual_repeat_n.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#![warn(clippy::manual_repeat_n)]

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();

let _ = repeat(vec![1, 2]).take(2);
}
35 changes: 35 additions & 0 deletions tests/ui/manual_repeat_n.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
error: this `.repeat().take()` can be written more concisely
--> tests/ui/manual_repeat_n.rs:6:13
|
LL | let _ = repeat(10).take(3);
| ^^^^^^^^^^^^^^^^^^ help: consider using `repeat_n()` instead: `std::iter::repeat_n(10, 3)`
|
= note: `-D clippy::manual-repeat-n` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::manual_repeat_n)]`

error: this `.repeat().take()` can be written more concisely
--> tests/ui/manual_repeat_n.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: this `.repeat().take()` can be written more concisely
--> tests/ui/manual_repeat_n.rs:10:18
|
LL | for value in std::iter::repeat(5).take(3) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `repeat_n()` instead: `std::iter::repeat_n(5, 3)`

error: this `.repeat().take()` can be written more concisely
--> tests/ui/manual_repeat_n.rs:12:21
|
LL | let _: Vec<_> = std::iter::repeat(String::from("bar")).take(10).collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `repeat_n()` instead: `std::iter::repeat_n(String::from("bar"), 10)`

error: this `.repeat().take()` can be written more concisely
--> tests/ui/manual_repeat_n.rs:14:13
|
LL | let _ = repeat(vec![1, 2]).take(2);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `repeat_n()` instead: `std::iter::repeat_n(vec![1, 2], 2)`

error: aborting due to 5 previous errors

2 changes: 1 addition & 1 deletion tests/ui/manual_str_repeat.fixed
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![allow(non_local_definitions)]
#![allow(non_local_definitions, clippy::manual_repeat_n)]
#![warn(clippy::manual_str_repeat)]

use std::borrow::Cow;
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/manual_str_repeat.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![allow(non_local_definitions)]
#![allow(non_local_definitions, clippy::manual_repeat_n)]
#![warn(clippy::manual_str_repeat)]

use std::borrow::Cow;
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/slow_vector_initialization.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(clippy::manual_repeat_n)]

//@no-rustfix
use std::iter::repeat;
fn main() {
Expand Down
26 changes: 13 additions & 13 deletions tests/ui/slow_vector_initialization.stderr
Original file line number Diff line number Diff line change
@@ -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]`
Expand All @@ -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]`
Expand Down

0 comments on commit 8e422ea

Please sign in to comment.