Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add a new lint for repeat().take() that can be replaced with repeat_n() #13858

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
2 changes: 1 addition & 1 deletion clippy_lints/src/doc/lazy_continuation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub(super) fn check(
diag.span_suggestion_verbose(
span.shrink_to_hi(),
"indent this line",
std::iter::repeat(" ").take(indent).join(""),
std::iter::repeat_n(" ", indent).join(""),
Applicability::MaybeIncorrect,
);
diag.help("if this is supposed to be its own paragraph, add a blank line");
Expand Down
42 changes: 42 additions & 0 deletions clippy_lints/src/methods/manual_repeat_n.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
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::{expr_use_ctxt, 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<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'tcx>,
repeat_expr: &Expr<'_>,
take_arg: &Expr<'_>,
msrv: &Msrv,
) {
if msrv.meets(msrvs::REPEAT_N)
&& !expr.span.from_expansion()
&& !expr_use_ctxt(cx, expr).is_ty_unified
&& 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. Also, `repeat_n()` is sometimes faster than `repeat().take()` when the type of the element is non-trivial to clone.
///
/// ### 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
4 changes: 2 additions & 2 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ use core::mem;
use core::ops::ControlFlow;
use std::collections::hash_map::Entry;
use std::hash::BuildHasherDefault;
use std::iter::{once, repeat};
use std::iter::{once, repeat_n};
use std::sync::{Mutex, MutexGuard, OnceLock};

use itertools::Itertools;
Expand Down Expand Up @@ -3414,7 +3414,7 @@ fn maybe_get_relative_path(from: &DefPath, to: &DefPath, max_super: usize) -> St
}))
.join("::")
} else {
repeat(String::from("super")).take(go_up_by).chain(path).join("::")
repeat_n(String::from("super"), go_up_by).chain(path).join("::")
}
}

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

30 changes: 30 additions & 0 deletions tests/ui/manual_repeat_n.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#![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);
}

mod foo_lib {
pub fn iter() -> std::iter::Take<std::iter::Repeat<&'static [u8]>> {
todo!()
}
}

fn foo() {
let _ = match 1 {
1 => foo_lib::iter(),
// Shouldn't lint because `external_lib::iter` doesn't return `std::iter::RepeatN`.
2 => std::iter::repeat([1, 2].as_slice()).take(2),
_ => todo!(),
};
}
30 changes: 30 additions & 0 deletions tests/ui/manual_repeat_n.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#![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);
}

mod foo_lib {
pub fn iter() -> std::iter::Take<std::iter::Repeat<&'static [u8]>> {
todo!()
}
}

fn foo() {
let _ = match 1 {
1 => foo_lib::iter(),
// Shouldn't lint because `external_lib::iter` doesn't return `std::iter::RepeatN`.
2 => std::iter::repeat([1, 2].as_slice()).take(2),
_ => todo!(),
};
}
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
Loading
Loading