From 544f71f48d0b73db7d5815519dc6fd6a4ce87390 Mon Sep 17 00:00:00 2001 From: lapla-cogito Date: Fri, 20 Dec 2024 22:29:06 +0900 Subject: [PATCH] add `manual_repeat_n` lint --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/methods/manual_repeat_n.rs | 43 +++++++++++++++++++++ clippy_lints/src/methods/mod.rs | 26 +++++++++++++ clippy_utils/src/lib.rs | 1 + tests/ui/from_iter_instead_of_collect.fixed | 2 +- tests/ui/from_iter_instead_of_collect.rs | 2 +- tests/ui/manual_repeat_n.fixed | 30 ++++++++++++++ tests/ui/manual_repeat_n.rs | 30 ++++++++++++++ tests/ui/manual_repeat_n.stderr | 35 +++++++++++++++++ tests/ui/manual_str_repeat.fixed | 2 +- tests/ui/manual_str_repeat.rs | 2 +- tests/ui/slow_vector_initialization.fixed | 3 +- tests/ui/slow_vector_initialization.rs | 3 +- tests/ui/slow_vector_initialization.stderr | 26 ++++++------- 15 files changed, 188 insertions(+), 19 deletions(-) create mode 100644 clippy_lints/src/methods/manual_repeat_n.rs create mode 100644 tests/ui/manual_repeat_n.fixed create mode 100644 tests/ui/manual_repeat_n.rs create mode 100644 tests/ui/manual_repeat_n.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index ceb08eeac3ff..cbac1a929df4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5768,6 +5768,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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 5e6cfd94283c..cc0c9ec127c4 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -420,6 +420,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, diff --git a/clippy_lints/src/methods/manual_repeat_n.rs b/clippy_lints/src/methods/manual_repeat_n.rs new file mode 100644 index 000000000000..6e09bf132aa1 --- /dev/null +++ b/clippy_lints/src/methods/manual_repeat_n.rs @@ -0,0 +1,43 @@ +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, std_or_core}; +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() + && 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) + && !expr_use_ctxt(cx, expr).is_ty_unified + && let Some(std_or_core) = std_or_core(cx) + { + 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_or_core}::iter::repeat_n({}, {})", + snippet_with_context(cx, repeat_arg.span, expr.span.ctxt(), "..", &mut app).0, + snippet(cx, take_arg.span, "..") + ), + app, + ); + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index d7e9f65bfa3a..b416d7d5814d 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -58,6 +58,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; @@ -4339,6 +4340,29 @@ declare_clippy_lint! { "using `NonZero::new_unchecked()` in a `const` context" } +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 because the original value can be reused for the last `.next()` call rather than always cloning. + /// + /// ### Example + /// ```no_run + /// let _ = std::iter::repeat(10).take(3); + /// ``` + /// Use instead: + /// ```no_run + /// let _ = std::iter::repeat_n(10, 3); + /// ``` + #[clippy::version = "1.86.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, @@ -4506,6 +4530,7 @@ impl_lint_pass!(Methods => [ UNNECESSARY_MAP_OR, DOUBLE_ENDED_ITERATOR_LAST, USELESS_NONZERO_NEW_UNCHECKED, + MANUAL_REPEAT_N, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -5176,6 +5201,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, diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index a0de9a76a0ef..676f387a7f01 100644 --- 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::manual_repeat_n, rustc::diagnostic_outside_of_impl, rustc::untranslatable_diagnostic )] diff --git a/tests/ui/from_iter_instead_of_collect.fixed b/tests/ui/from_iter_instead_of_collect.fixed index c250162dfb8c..67da45a348f9 100644 --- a/tests/ui/from_iter_instead_of_collect.fixed +++ b/tests/ui/from_iter_instead_of_collect.fixed @@ -1,6 +1,6 @@ #![warn(clippy::from_iter_instead_of_collect)] #![allow(unused_imports)] -#![allow(clippy::useless_vec)] +#![allow(clippy::useless_vec, clippy::manual_repeat_n)] use std::collections::{BTreeMap, BTreeSet, HashMap, VecDeque}; diff --git a/tests/ui/from_iter_instead_of_collect.rs b/tests/ui/from_iter_instead_of_collect.rs index 8adbb841c8ba..423a7454bed7 100644 --- a/tests/ui/from_iter_instead_of_collect.rs +++ b/tests/ui/from_iter_instead_of_collect.rs @@ -1,6 +1,6 @@ #![warn(clippy::from_iter_instead_of_collect)] #![allow(unused_imports)] -#![allow(clippy::useless_vec)] +#![allow(clippy::useless_vec, clippy::manual_repeat_n)] use std::collections::{BTreeMap, BTreeSet, HashMap, VecDeque}; diff --git a/tests/ui/manual_repeat_n.fixed b/tests/ui/manual_repeat_n.fixed new file mode 100644 index 000000000000..4235b02a89e3 --- /dev/null +++ b/tests/ui/manual_repeat_n.fixed @@ -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> { + 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!(), + }; +} diff --git a/tests/ui/manual_repeat_n.rs b/tests/ui/manual_repeat_n.rs new file mode 100644 index 000000000000..dbf9ac6a14af --- /dev/null +++ b/tests/ui/manual_repeat_n.rs @@ -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> { + 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!(), + }; +} diff --git a/tests/ui/manual_repeat_n.stderr b/tests/ui/manual_repeat_n.stderr new file mode 100644 index 000000000000..87395b3f8bf1 --- /dev/null +++ b/tests/ui/manual_repeat_n.stderr @@ -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 + diff --git a/tests/ui/manual_str_repeat.fixed b/tests/ui/manual_str_repeat.fixed index 5f2f1bd9916d..da6f36f53b0d 100644 --- 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::manual_repeat_n)] #![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 index 3e3c7f4db4a2..686ed4fee7d1 100644 --- 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, clippy::manual_repeat_n)] #![warn(clippy::manual_str_repeat)] use std::borrow::Cow; diff --git a/tests/ui/slow_vector_initialization.fixed b/tests/ui/slow_vector_initialization.fixed index 8c16bb307ca0..a8570366e646 100644 --- a/tests/ui/slow_vector_initialization.fixed +++ b/tests/ui/slow_vector_initialization.fixed @@ -1,4 +1,5 @@ -#![allow(clippy::useless_vec)] +#![allow(clippy::useless_vec, clippy::manual_repeat_n)] + use std::iter::repeat; fn main() { resize_vector(); diff --git a/tests/ui/slow_vector_initialization.rs b/tests/ui/slow_vector_initialization.rs index 6831dad70b43..4b30fad409e3 100644 --- a/tests/ui/slow_vector_initialization.rs +++ b/tests/ui/slow_vector_initialization.rs @@ -1,4 +1,5 @@ -#![allow(clippy::useless_vec)] +#![allow(clippy::useless_vec, clippy::manual_repeat_n)] + use std::iter::repeat; fn main() { resize_vector(); diff --git a/tests/ui/slow_vector_initialization.stderr b/tests/ui/slow_vector_initialization.stderr index 7f4b9f7b67a4..4a25cafcddf2 100644 --- 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:13:20 + --> tests/ui/slow_vector_initialization.rs:14:20 | LL | let mut vec1 = Vec::with_capacity(len); | ____________________^ @@ -11,7 +11,7 @@ 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:19:20 + --> tests/ui/slow_vector_initialization.rs:20:20 | LL | let mut vec2 = Vec::with_capacity(len - 10); | ____________________^ @@ -20,7 +20,7 @@ LL | | vec2.extend(repeat(0).take(len - 10)); | |_________________________________________^ help: consider replacing this with: `vec![0; len - 10]` error: slow zero-filling initialization - --> tests/ui/slow_vector_initialization.rs:27:20 + --> tests/ui/slow_vector_initialization.rs:28:20 | LL | let mut vec4 = Vec::with_capacity(len); | ____________________^ @@ -29,7 +29,7 @@ LL | | vec4.extend(repeat(0).take(vec4.capacity())); | |________________________________________________^ help: consider replacing this with: `vec![0; len]` error: slow zero-filling initialization - --> tests/ui/slow_vector_initialization.rs:38:27 + --> tests/ui/slow_vector_initialization.rs:39:27 | LL | let mut resized_vec = Vec::with_capacity(30); | ___________________________^ @@ -38,7 +38,7 @@ LL | | resized_vec.resize(30, 0); | |_____________________________^ help: consider replacing this with: `vec![0; 30]` error: slow zero-filling initialization - --> tests/ui/slow_vector_initialization.rs:42:26 + --> tests/ui/slow_vector_initialization.rs:43:26 | LL | let mut extend_vec = Vec::with_capacity(30); | __________________________^ @@ -47,7 +47,7 @@ LL | | extend_vec.extend(repeat(0).take(30)); | |_________________________________________^ help: consider replacing this with: `vec![0; 30]` error: slow zero-filling initialization - --> tests/ui/slow_vector_initialization.rs:50:20 + --> tests/ui/slow_vector_initialization.rs:51:20 | LL | let mut vec1 = Vec::with_capacity(len); | ____________________^ @@ -56,7 +56,7 @@ LL | | vec1.resize(len, 0); | |_______________________^ help: consider replacing this with: `vec![0; len]` error: slow zero-filling initialization - --> tests/ui/slow_vector_initialization.rs:59:20 + --> tests/ui/slow_vector_initialization.rs:60:20 | LL | let mut vec3 = Vec::with_capacity(len - 10); | ____________________^ @@ -65,7 +65,7 @@ LL | | vec3.resize(len - 10, 0); | |____________________________^ help: consider replacing this with: `vec![0; len - 10]` error: slow zero-filling initialization - --> tests/ui/slow_vector_initialization.rs:63:20 + --> tests/ui/slow_vector_initialization.rs:64:20 | LL | let mut vec4 = Vec::with_capacity(len); | ____________________^ @@ -74,7 +74,7 @@ LL | | vec4.resize(vec4.capacity(), 0); | |___________________________________^ help: consider replacing this with: `vec![0; len]` error: slow zero-filling initialization - --> tests/ui/slow_vector_initialization.rs:68:12 + --> tests/ui/slow_vector_initialization.rs:69:12 | LL | vec1 = Vec::with_capacity(10); | ____________^ @@ -83,7 +83,7 @@ LL | | vec1.resize(10, 0); | |______________________^ help: consider replacing this with: `vec![0; 10]` error: slow zero-filling initialization - --> tests/ui/slow_vector_initialization.rs:76:20 + --> tests/ui/slow_vector_initialization.rs:77:20 | LL | let mut vec1 = Vec::new(); | ____________________^ @@ -92,7 +92,7 @@ LL | | vec1.resize(len, 0); | |_______________________^ help: consider replacing this with: `vec![0; len]` error: slow zero-filling initialization - --> tests/ui/slow_vector_initialization.rs:81:20 + --> tests/ui/slow_vector_initialization.rs:82:20 | LL | let mut vec3 = Vec::new(); | ____________________^ @@ -101,7 +101,7 @@ LL | | vec3.resize(len - 10, 0); | |____________________________^ help: consider replacing this with: `vec![0; len - 10]` error: slow zero-filling initialization - --> tests/ui/slow_vector_initialization.rs:86:12 + --> tests/ui/slow_vector_initialization.rs:87:12 | LL | vec1 = Vec::new(); | ____________^ @@ -110,7 +110,7 @@ LL | | vec1.resize(10, 0); | |______________________^ help: consider replacing this with: `vec![0; 10]` error: slow zero-filling initialization - --> tests/ui/slow_vector_initialization.rs:90:12 + --> tests/ui/slow_vector_initialization.rs:91:12 | LL | vec1 = vec![]; | ____________^