From 6702c7a7a6b47b3aed254e6b2f1575de234511c3 Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Mon, 12 Jun 2023 11:09:52 -0500 Subject: [PATCH 1/2] Add lint [`single_range_in_vec_init`] --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/single_range_in_vec_init.rs | 153 +++++++++++++++++++ tests/ui/single_element_loop.fixed | 2 + tests/ui/single_element_loop.rs | 2 + tests/ui/single_element_loop.stderr | 14 +- tests/ui/single_range_in_vec_init.rs | 58 +++++++ tests/ui/single_range_in_vec_init.stderr | 145 ++++++++++++++++++ tests/ui/while_let_on_iterator.fixed | 1 + tests/ui/while_let_on_iterator.rs | 1 + tests/ui/while_let_on_iterator.stderr | 52 +++---- 12 files changed, 399 insertions(+), 33 deletions(-) create mode 100644 clippy_lints/src/single_range_in_vec_init.rs create mode 100644 tests/ui/single_range_in_vec_init.rs create mode 100644 tests/ui/single_range_in_vec_init.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 85fddc970473..238c2584221a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5164,6 +5164,7 @@ Released 2018-09-13 [`single_element_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_element_loop [`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match [`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else +[`single_range_in_vec_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_range_in_vec_init [`size_of_in_element_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_in_element_count [`size_of_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_ref [`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 523faa302dc7..ec127857ab26 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -569,6 +569,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::significant_drop_tightening::SIGNIFICANT_DROP_TIGHTENING_INFO, crate::single_char_lifetime_names::SINGLE_CHAR_LIFETIME_NAMES_INFO, crate::single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS_INFO, + crate::single_range_in_vec_init::SINGLE_RANGE_IN_VEC_INIT_INFO, crate::size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT_INFO, crate::size_of_ref::SIZE_OF_REF_INFO, crate::slow_vector_initialization::SLOW_VECTOR_INITIALIZATION_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 243d1fd8606a..cf6499b96161 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -288,6 +288,7 @@ mod shadow; mod significant_drop_tightening; mod single_char_lifetime_names; mod single_component_path_imports; +mod single_range_in_vec_init; mod size_of_in_element_count; mod size_of_ref; mod slow_vector_initialization; @@ -1045,6 +1046,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: }); let stack_size_threshold = conf.stack_size_threshold; store.register_late_pass(move |_| Box::new(large_stack_frames::LargeStackFrames::new(stack_size_threshold))); + store.register_late_pass(|_| Box::new(single_range_in_vec_init::SingleRangeInVecInit)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/single_range_in_vec_init.rs b/clippy_lints/src/single_range_in_vec_init.rs new file mode 100644 index 000000000000..2d3f18a62c01 --- /dev/null +++ b/clippy_lints/src/single_range_in_vec_init.rs @@ -0,0 +1,153 @@ +use clippy_utils::{ + diagnostics::span_lint_and_then, + get_trait_def_id, + higher::VecArgs, + macros::root_macro_call_first_node, + source::{snippet_opt, snippet_with_applicability}, + ty::implements_trait, +}; +use rustc_ast::{LitIntType, LitKind, UintTy}; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, LangItem, QPath}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use std::fmt::{self, Display, Formatter}; + +declare_clippy_lint! { + /// ### What it does + /// Checks for `Vec` or array initializations that contain only one range. + /// + /// ### Why is this bad? + /// This is almost always incorrect, as it will result in a `Vec` that has only element. Almost + /// always, the programmer intended for it to include all elements in the range or for the end + /// of the range to be the length instead. + /// + /// ### Example + /// ```rust + /// let x = [0..200]; + /// ``` + /// Use instead: + /// ```rust + /// // If it was intended to include every element in the range... + /// let x = (0..200).collect::>(); + /// // ...Or if 200 was meant to be the len + /// let x = [0; 200]; + /// ``` + #[clippy::version = "1.72.0"] + pub SINGLE_RANGE_IN_VEC_INIT, + suspicious, + "checks for initialization of `Vec` or arrays which consist of a single range" +} +declare_lint_pass!(SingleRangeInVecInit => [SINGLE_RANGE_IN_VEC_INIT]); + +enum SuggestedType { + Vec, + Array, +} + +impl SuggestedType { + fn starts_with(&self) -> &'static str { + if matches!(self, SuggestedType::Vec) { + "vec!" + } else { + "[" + } + } + + fn ends_with(&self) -> &'static str { + if matches!(self, SuggestedType::Vec) { "" } else { "]" } + } +} + +impl Display for SuggestedType { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + if matches!(&self, SuggestedType::Vec) { + write!(f, "a `Vec`") + } else { + write!(f, "an array") + } + } +} + +impl LateLintPass<'_> for SingleRangeInVecInit { + fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { + // inner_expr: `vec![0..200]` or `[0..200]` + // ^^^^^^ ^^^^^^^ + // span: `vec![0..200]` or `[0..200]` + // ^^^^^^^^^^^^ ^^^^^^^^ + // kind: What to print, an array or a `Vec` + let (inner_expr, span, kind) = if let ExprKind::Array([inner_expr]) = expr.kind + && !expr.span.from_expansion() + { + (inner_expr, expr.span, SuggestedType::Array) + } else if let Some(macro_call) = root_macro_call_first_node(cx, expr) + && let Some(VecArgs::Vec([expr])) = VecArgs::hir(cx, expr) + { + (expr, macro_call.span, SuggestedType::Vec) + } else { + return; + }; + + let ExprKind::Struct(QPath::LangItem(lang_item, ..), [start, end], None) = inner_expr.kind else { + return; + }; + + if matches!(lang_item, LangItem::Range) + && let ty = cx.typeck_results().expr_ty(start.expr) + && let Some(snippet) = snippet_opt(cx, span) + // `is_from_proc_macro` will skip any `vec![]`. Let's not! + && snippet.starts_with(kind.starts_with()) + && snippet.ends_with(kind.ends_with()) + { + let mut app = Applicability::MaybeIncorrect; + let start_snippet = snippet_with_applicability(cx, start.span, "...", &mut app); + let end_snippet = snippet_with_applicability(cx, end.span, "...", &mut app); + + let should_emit_every_value = if let Some(step_def_id) = get_trait_def_id(cx, &["core", "iter", "Step"]) + && implements_trait(cx, ty, step_def_id, &[]) + { + true + } else { + false + }; + let should_emit_of_len = if let Some(copy_def_id) = cx.tcx.lang_items().copy_trait() + && implements_trait(cx, ty, copy_def_id, &[]) + && let ExprKind::Lit(lit_kind) = end.expr.kind + && let LitKind::Int(.., suffix_type) = lit_kind.node + && let LitIntType::Unsigned(UintTy::Usize) | LitIntType::Unsuffixed = suffix_type + { + true + } else { + false + }; + + if should_emit_every_value || should_emit_of_len { + span_lint_and_then( + cx, + SINGLE_RANGE_IN_VEC_INIT, + span, + &format!("{kind} of `Range` that is only one element"), + |diag| { + if should_emit_every_value { + diag.span_suggestion( + span, + "if you wanted a `Vec` that contains every value in the range, try", + format!("({start_snippet}..{end_snippet}).collect::>()"), + app, + ); + } + + if should_emit_of_len { + diag.span_suggestion( + inner_expr.span, + format!("if you wanted {kind} of len {end_snippet}, try"), + format!("{start_snippet}; {end_snippet}"), + app, + ); + } + }, + ); + } + } + } +} diff --git a/tests/ui/single_element_loop.fixed b/tests/ui/single_element_loop.fixed index 1697a0cf29b8..598f259415da 100644 --- a/tests/ui/single_element_loop.fixed +++ b/tests/ui/single_element_loop.fixed @@ -1,6 +1,8 @@ //@run-rustfix // Tests from for_loop.rs that don't have suggestions +#![allow(clippy::single_range_in_vec_init)] + #[warn(clippy::single_element_loop)] fn main() { let item1 = 2; diff --git a/tests/ui/single_element_loop.rs b/tests/ui/single_element_loop.rs index 860424f42ddf..3fc461735a49 100644 --- a/tests/ui/single_element_loop.rs +++ b/tests/ui/single_element_loop.rs @@ -1,6 +1,8 @@ //@run-rustfix // Tests from for_loop.rs that don't have suggestions +#![allow(clippy::single_range_in_vec_init)] + #[warn(clippy::single_element_loop)] fn main() { let item1 = 2; diff --git a/tests/ui/single_element_loop.stderr b/tests/ui/single_element_loop.stderr index 14437a59745e..c40c6198945a 100644 --- a/tests/ui/single_element_loop.stderr +++ b/tests/ui/single_element_loop.stderr @@ -1,5 +1,5 @@ error: for loop over a single element - --> $DIR/single_element_loop.rs:7:5 + --> $DIR/single_element_loop.rs:9:5 | LL | / for item in &[item1] { LL | | dbg!(item); @@ -16,7 +16,7 @@ LL + } | error: for loop over a single element - --> $DIR/single_element_loop.rs:11:5 + --> $DIR/single_element_loop.rs:13:5 | LL | / for item in [item1].iter() { LL | | dbg!(item); @@ -32,7 +32,7 @@ LL + } | error: for loop over a single element - --> $DIR/single_element_loop.rs:15:5 + --> $DIR/single_element_loop.rs:17:5 | LL | / for item in &[0..5] { LL | | dbg!(item); @@ -48,7 +48,7 @@ LL + } | error: for loop over a single element - --> $DIR/single_element_loop.rs:19:5 + --> $DIR/single_element_loop.rs:21:5 | LL | / for item in [0..5].iter_mut() { LL | | dbg!(item); @@ -64,7 +64,7 @@ LL + } | error: for loop over a single element - --> $DIR/single_element_loop.rs:23:5 + --> $DIR/single_element_loop.rs:25:5 | LL | / for item in [0..5] { LL | | dbg!(item); @@ -80,7 +80,7 @@ LL + } | error: for loop over a single element - --> $DIR/single_element_loop.rs:27:5 + --> $DIR/single_element_loop.rs:29:5 | LL | / for item in [0..5].into_iter() { LL | | dbg!(item); @@ -96,7 +96,7 @@ LL + } | error: for loop over a single element - --> $DIR/single_element_loop.rs:46:5 + --> $DIR/single_element_loop.rs:48:5 | LL | / for _ in [42] { LL | | let _f = |n: u32| { diff --git a/tests/ui/single_range_in_vec_init.rs b/tests/ui/single_range_in_vec_init.rs new file mode 100644 index 000000000000..abf784e0f0ed --- /dev/null +++ b/tests/ui/single_range_in_vec_init.rs @@ -0,0 +1,58 @@ +//@aux-build:proc_macros.rs +#![allow(clippy::no_effect, clippy::useless_vec, unused)] +#![warn(clippy::single_range_in_vec_init)] +#![feature(generic_arg_infer)] + +#[macro_use] +extern crate proc_macros; + +macro_rules! a { + () => { + vec![0..200]; + }; +} + +fn awa(start: T, end: T) { + [start..end]; +} + +fn awa_vec(start: T, end: T) { + vec![start..end]; +} + +fn main() { + // Lint + [0..200]; + vec![0..200]; + [0u8..200]; + [0usize..200]; + [0..200usize]; + vec![0u8..200]; + vec![0usize..200]; + vec![0..200usize]; + // Only suggest collect + [0..200isize]; + vec![0..200isize]; + // Do not lint + [0..200, 0..100]; + vec![0..200, 0..100]; + [0.0..200.0]; + vec![0.0..200.0]; + // `Copy` is not implemented for `Range`, so this doesn't matter + // [0..200; 2]; + // [vec!0..200; 2]; + + // Unfortunately skips any macros + a!(); + + // Skip external macros and procedural macros + external! { + [0..200]; + vec![0..200]; + } + with_span! { + span + [0..200]; + vec![0..200]; + } +} diff --git a/tests/ui/single_range_in_vec_init.stderr b/tests/ui/single_range_in_vec_init.stderr new file mode 100644 index 000000000000..fc03b2205e2e --- /dev/null +++ b/tests/ui/single_range_in_vec_init.stderr @@ -0,0 +1,145 @@ +error: an array of `Range` that is only one element + --> $DIR/single_range_in_vec_init.rs:25:5 + | +LL | [0..200]; + | ^^^^^^^^ + | + = note: `-D clippy::single-range-in-vec-init` implied by `-D warnings` +help: if you wanted a `Vec` that contains every value in the range, try + | +LL | (0..200).collect::>(); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: if you wanted an array of len 200, try + | +LL | [0; 200]; + | ~~~~~~ + +error: a `Vec` of `Range` that is only one element + --> $DIR/single_range_in_vec_init.rs:26:5 + | +LL | vec![0..200]; + | ^^^^^^^^^^^^ + | +help: if you wanted a `Vec` that contains every value in the range, try + | +LL | (0..200).collect::>(); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: if you wanted a `Vec` of len 200, try + | +LL | vec![0; 200]; + | ~~~~~~ + +error: an array of `Range` that is only one element + --> $DIR/single_range_in_vec_init.rs:27:5 + | +LL | [0u8..200]; + | ^^^^^^^^^^ + | +help: if you wanted a `Vec` that contains every value in the range, try + | +LL | (0u8..200).collect::>(); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: if you wanted an array of len 200, try + | +LL | [0u8; 200]; + | ~~~~~~~~ + +error: an array of `Range` that is only one element + --> $DIR/single_range_in_vec_init.rs:28:5 + | +LL | [0usize..200]; + | ^^^^^^^^^^^^^ + | +help: if you wanted a `Vec` that contains every value in the range, try + | +LL | (0usize..200).collect::>(); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: if you wanted an array of len 200, try + | +LL | [0usize; 200]; + | ~~~~~~~~~~~ + +error: an array of `Range` that is only one element + --> $DIR/single_range_in_vec_init.rs:29:5 + | +LL | [0..200usize]; + | ^^^^^^^^^^^^^ + | +help: if you wanted a `Vec` that contains every value in the range, try + | +LL | (0..200usize).collect::>(); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: if you wanted an array of len 200usize, try + | +LL | [0; 200usize]; + | ~~~~~~~~~~~ + +error: a `Vec` of `Range` that is only one element + --> $DIR/single_range_in_vec_init.rs:30:5 + | +LL | vec![0u8..200]; + | ^^^^^^^^^^^^^^ + | +help: if you wanted a `Vec` that contains every value in the range, try + | +LL | (0u8..200).collect::>(); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: if you wanted a `Vec` of len 200, try + | +LL | vec![0u8; 200]; + | ~~~~~~~~ + +error: a `Vec` of `Range` that is only one element + --> $DIR/single_range_in_vec_init.rs:31:5 + | +LL | vec![0usize..200]; + | ^^^^^^^^^^^^^^^^^ + | +help: if you wanted a `Vec` that contains every value in the range, try + | +LL | (0usize..200).collect::>(); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: if you wanted a `Vec` of len 200, try + | +LL | vec![0usize; 200]; + | ~~~~~~~~~~~ + +error: a `Vec` of `Range` that is only one element + --> $DIR/single_range_in_vec_init.rs:32:5 + | +LL | vec![0..200usize]; + | ^^^^^^^^^^^^^^^^^ + | +help: if you wanted a `Vec` that contains every value in the range, try + | +LL | (0..200usize).collect::>(); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: if you wanted a `Vec` of len 200usize, try + | +LL | vec![0; 200usize]; + | ~~~~~~~~~~~ + +error: an array of `Range` that is only one element + --> $DIR/single_range_in_vec_init.rs:34:5 + | +LL | [0..200isize]; + | ^^^^^^^^^^^^^ + | +help: if you wanted a `Vec` that contains every value in the range, try + | +LL | (0..200isize).collect::>(); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: a `Vec` of `Range` that is only one element + --> $DIR/single_range_in_vec_init.rs:35:5 + | +LL | vec![0..200isize]; + | ^^^^^^^^^^^^^^^^^ + | +help: if you wanted a `Vec` that contains every value in the range, try + | +LL | (0..200isize).collect::>(); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: aborting due to 10 previous errors + diff --git a/tests/ui/while_let_on_iterator.fixed b/tests/ui/while_let_on_iterator.fixed index 11f0de07c62f..41a380ab8f6a 100644 --- a/tests/ui/while_let_on_iterator.fixed +++ b/tests/ui/while_let_on_iterator.fixed @@ -6,6 +6,7 @@ clippy::manual_find, clippy::never_loop, clippy::redundant_closure_call, + clippy::single_range_in_vec_init, clippy::uninlined_format_args, clippy::useless_vec )] diff --git a/tests/ui/while_let_on_iterator.rs b/tests/ui/while_let_on_iterator.rs index e54575dd3b43..4c6433880b63 100644 --- a/tests/ui/while_let_on_iterator.rs +++ b/tests/ui/while_let_on_iterator.rs @@ -6,6 +6,7 @@ clippy::manual_find, clippy::never_loop, clippy::redundant_closure_call, + clippy::single_range_in_vec_init, clippy::uninlined_format_args, clippy::useless_vec )] diff --git a/tests/ui/while_let_on_iterator.stderr b/tests/ui/while_let_on_iterator.stderr index f8a66f2ad3e9..3236765e1db0 100644 --- a/tests/ui/while_let_on_iterator.stderr +++ b/tests/ui/while_let_on_iterator.stderr @@ -1,5 +1,5 @@ error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:15:5 + --> $DIR/while_let_on_iterator.rs:16:5 | LL | while let Option::Some(x) = iter.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in iter` @@ -7,151 +7,151 @@ LL | while let Option::Some(x) = iter.next() { = note: `-D clippy::while-let-on-iterator` implied by `-D warnings` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:20:5 + --> $DIR/while_let_on_iterator.rs:21:5 | LL | while let Some(x) = iter.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in iter` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:25:5 + --> $DIR/while_let_on_iterator.rs:26:5 | LL | while let Some(_) = iter.next() {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in iter` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:101:9 + --> $DIR/while_let_on_iterator.rs:102:9 | LL | while let Some([..]) = it.next() {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for [..] in it` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:108:9 + --> $DIR/while_let_on_iterator.rs:109:9 | LL | while let Some([_x]) = it.next() {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for [_x] in it` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:121:9 + --> $DIR/while_let_on_iterator.rs:122:9 | LL | while let Some(x @ [_]) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x @ [_] in it` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:141:9 + --> $DIR/while_let_on_iterator.rs:142:9 | LL | while let Some(_) = y.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in y` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:198:9 + --> $DIR/while_let_on_iterator.rs:199:9 | LL | while let Some(m) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in it.by_ref()` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:209:5 + --> $DIR/while_let_on_iterator.rs:210:5 | LL | while let Some(n) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for n in it` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:211:9 + --> $DIR/while_let_on_iterator.rs:212:9 | LL | while let Some(m) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in it` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:220:9 + --> $DIR/while_let_on_iterator.rs:221:9 | LL | while let Some(m) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in it` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:229:9 + --> $DIR/while_let_on_iterator.rs:230:9 | LL | while let Some(m) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in it.by_ref()` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:246:9 + --> $DIR/while_let_on_iterator.rs:247:9 | LL | while let Some(m) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in it.by_ref()` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:261:13 + --> $DIR/while_let_on_iterator.rs:262:13 | LL | while let Some(i) = self.0.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for i in self.0.by_ref()` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:293:13 + --> $DIR/while_let_on_iterator.rs:294:13 | LL | while let Some(i) = self.0.0.0.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for i in self.0.0.0.by_ref()` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:322:5 + --> $DIR/while_let_on_iterator.rs:323:5 | LL | while let Some(n) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for n in it.by_ref()` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:334:9 + --> $DIR/while_let_on_iterator.rs:335:9 | LL | while let Some(x) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it.by_ref()` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:348:5 + --> $DIR/while_let_on_iterator.rs:349:5 | LL | while let Some(x) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it.by_ref()` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:359:5 + --> $DIR/while_let_on_iterator.rs:360:5 | LL | while let Some(x) = it.0.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it.0.by_ref()` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:394:5 + --> $DIR/while_let_on_iterator.rs:395:5 | LL | while let Some(x) = s.x.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in s.x.by_ref()` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:401:5 + --> $DIR/while_let_on_iterator.rs:402:5 | LL | while let Some(x) = x[0].next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in x[0].by_ref()` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:409:9 + --> $DIR/while_let_on_iterator.rs:410:9 | LL | while let Some(x) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:419:9 + --> $DIR/while_let_on_iterator.rs:420:9 | LL | while let Some(x) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:429:9 + --> $DIR/while_let_on_iterator.rs:430:9 | LL | while let Some(x) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it.by_ref()` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:439:9 + --> $DIR/while_let_on_iterator.rs:440:9 | LL | while let Some(x) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:449:5 + --> $DIR/while_let_on_iterator.rs:450:5 | LL | while let Some(..) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in it` From 830d307d0a4b0bb584bf3552fcbf0e2dc2575aec Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Mon, 12 Jun 2023 12:55:31 -0500 Subject: [PATCH 2/2] refactor a bit --- clippy_lints/src/single_range_in_vec_init.rs | 38 +++++++++----------- tests/ui/single_range_in_vec_init.stderr | 20 +++++------ 2 files changed, 26 insertions(+), 32 deletions(-) diff --git a/clippy_lints/src/single_range_in_vec_init.rs b/clippy_lints/src/single_range_in_vec_init.rs index 2d3f18a62c01..dfe8be7a6a61 100644 --- a/clippy_lints/src/single_range_in_vec_init.rs +++ b/clippy_lints/src/single_range_in_vec_init.rs @@ -1,10 +1,6 @@ use clippy_utils::{ - diagnostics::span_lint_and_then, - get_trait_def_id, - higher::VecArgs, - macros::root_macro_call_first_node, - source::{snippet_opt, snippet_with_applicability}, - ty::implements_trait, + diagnostics::span_lint_and_then, get_trait_def_id, higher::VecArgs, macros::root_macro_call_first_node, + source::snippet_opt, ty::implements_trait, }; use rustc_ast::{LitIntType, LitKind, UintTy}; use rustc_errors::Applicability; @@ -18,9 +14,9 @@ declare_clippy_lint! { /// Checks for `Vec` or array initializations that contain only one range. /// /// ### Why is this bad? - /// This is almost always incorrect, as it will result in a `Vec` that has only element. Almost - /// always, the programmer intended for it to include all elements in the range or for the end - /// of the range to be the length instead. + /// This is almost always incorrect, as it will result in a `Vec` that has only one element. + /// Almost always, the programmer intended for it to include all elements in the range or for + /// the end of the range to be the length instead. /// /// ### Example /// ```rust @@ -75,8 +71,8 @@ impl LateLintPass<'_> for SingleRangeInVecInit { // ^^^^^^ ^^^^^^^ // span: `vec![0..200]` or `[0..200]` // ^^^^^^^^^^^^ ^^^^^^^^ - // kind: What to print, an array or a `Vec` - let (inner_expr, span, kind) = if let ExprKind::Array([inner_expr]) = expr.kind + // suggested_type: What to print, "an array" or "a `Vec`" + let (inner_expr, span, suggested_type) = if let ExprKind::Array([inner_expr]) = expr.kind && !expr.span.from_expansion() { (inner_expr, expr.span, SuggestedType::Array) @@ -96,13 +92,11 @@ impl LateLintPass<'_> for SingleRangeInVecInit { && let ty = cx.typeck_results().expr_ty(start.expr) && let Some(snippet) = snippet_opt(cx, span) // `is_from_proc_macro` will skip any `vec![]`. Let's not! - && snippet.starts_with(kind.starts_with()) - && snippet.ends_with(kind.ends_with()) + && snippet.starts_with(suggested_type.starts_with()) + && snippet.ends_with(suggested_type.ends_with()) + && let Some(start_snippet) = snippet_opt(cx, start.span) + && let Some(end_snippet) = snippet_opt(cx, end.span) { - let mut app = Applicability::MaybeIncorrect; - let start_snippet = snippet_with_applicability(cx, start.span, "...", &mut app); - let end_snippet = snippet_with_applicability(cx, end.span, "...", &mut app); - let should_emit_every_value = if let Some(step_def_id) = get_trait_def_id(cx, &["core", "iter", "Step"]) && implements_trait(cx, ty, step_def_id, &[]) { @@ -126,23 +120,23 @@ impl LateLintPass<'_> for SingleRangeInVecInit { cx, SINGLE_RANGE_IN_VEC_INIT, span, - &format!("{kind} of `Range` that is only one element"), + &format!("{suggested_type} of `Range` that is only one element"), |diag| { if should_emit_every_value { diag.span_suggestion( span, - "if you wanted a `Vec` that contains every value in the range, try", + "if you wanted a `Vec` that contains the entire range, try", format!("({start_snippet}..{end_snippet}).collect::>()"), - app, + Applicability::MaybeIncorrect, ); } if should_emit_of_len { diag.span_suggestion( inner_expr.span, - format!("if you wanted {kind} of len {end_snippet}, try"), + format!("if you wanted {suggested_type} of len {end_snippet}, try"), format!("{start_snippet}; {end_snippet}"), - app, + Applicability::MaybeIncorrect, ); } }, diff --git a/tests/ui/single_range_in_vec_init.stderr b/tests/ui/single_range_in_vec_init.stderr index fc03b2205e2e..3e3d521f4a50 100644 --- a/tests/ui/single_range_in_vec_init.stderr +++ b/tests/ui/single_range_in_vec_init.stderr @@ -5,7 +5,7 @@ LL | [0..200]; | ^^^^^^^^ | = note: `-D clippy::single-range-in-vec-init` implied by `-D warnings` -help: if you wanted a `Vec` that contains every value in the range, try +help: if you wanted a `Vec` that contains the entire range, try | LL | (0..200).collect::>(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -20,7 +20,7 @@ error: a `Vec` of `Range` that is only one element LL | vec![0..200]; | ^^^^^^^^^^^^ | -help: if you wanted a `Vec` that contains every value in the range, try +help: if you wanted a `Vec` that contains the entire range, try | LL | (0..200).collect::>(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -35,7 +35,7 @@ error: an array of `Range` that is only one element LL | [0u8..200]; | ^^^^^^^^^^ | -help: if you wanted a `Vec` that contains every value in the range, try +help: if you wanted a `Vec` that contains the entire range, try | LL | (0u8..200).collect::>(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -50,7 +50,7 @@ error: an array of `Range` that is only one element LL | [0usize..200]; | ^^^^^^^^^^^^^ | -help: if you wanted a `Vec` that contains every value in the range, try +help: if you wanted a `Vec` that contains the entire range, try | LL | (0usize..200).collect::>(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -65,7 +65,7 @@ error: an array of `Range` that is only one element LL | [0..200usize]; | ^^^^^^^^^^^^^ | -help: if you wanted a `Vec` that contains every value in the range, try +help: if you wanted a `Vec` that contains the entire range, try | LL | (0..200usize).collect::>(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -80,7 +80,7 @@ error: a `Vec` of `Range` that is only one element LL | vec![0u8..200]; | ^^^^^^^^^^^^^^ | -help: if you wanted a `Vec` that contains every value in the range, try +help: if you wanted a `Vec` that contains the entire range, try | LL | (0u8..200).collect::>(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -95,7 +95,7 @@ error: a `Vec` of `Range` that is only one element LL | vec![0usize..200]; | ^^^^^^^^^^^^^^^^^ | -help: if you wanted a `Vec` that contains every value in the range, try +help: if you wanted a `Vec` that contains the entire range, try | LL | (0usize..200).collect::>(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -110,7 +110,7 @@ error: a `Vec` of `Range` that is only one element LL | vec![0..200usize]; | ^^^^^^^^^^^^^^^^^ | -help: if you wanted a `Vec` that contains every value in the range, try +help: if you wanted a `Vec` that contains the entire range, try | LL | (0..200usize).collect::>(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -125,7 +125,7 @@ error: an array of `Range` that is only one element LL | [0..200isize]; | ^^^^^^^^^^^^^ | -help: if you wanted a `Vec` that contains every value in the range, try +help: if you wanted a `Vec` that contains the entire range, try | LL | (0..200isize).collect::>(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -136,7 +136,7 @@ error: a `Vec` of `Range` that is only one element LL | vec![0..200isize]; | ^^^^^^^^^^^^^^^^^ | -help: if you wanted a `Vec` that contains every value in the range, try +help: if you wanted a `Vec` that contains the entire range, try | LL | (0..200isize).collect::>(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~