From 447edf92b4dfc96263665972c59b706c966a8833 Mon Sep 17 00:00:00 2001 From: Christoph Beberweil Date: Thu, 23 Nov 2023 23:07:36 +0100 Subject: [PATCH 1/5] suggest alternatives to iterate an array of ranges Co-authored-by: ThinkerDreamer <74881094+ThinkerDreamer@users.noreply.github.com> --- clippy_lints/src/loops/single_element_loop.rs | 33 ++++++--- tests/ui/single_element_loop.fixed | 12 ++-- tests/ui/single_element_loop.stderr | 72 +++++-------------- 3 files changed, 43 insertions(+), 74 deletions(-) diff --git a/clippy_lints/src/loops/single_element_loop.rs b/clippy_lints/src/loops/single_element_loop.rs index d860b297a0268..7164bf06b8dd9 100644 --- a/clippy_lints/src/loops/single_element_loop.rs +++ b/clippy_lints/src/loops/single_element_loop.rs @@ -1,6 +1,6 @@ use super::SINGLE_ELEMENT_LOOP; use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::{indent_of, snippet_with_applicability}; +use clippy_utils::source::{indent_of, snippet, snippet_with_applicability}; use clippy_utils::visitors::contains_break_or_continue; use rustc_ast::util::parser::PREC_PREFIX; use rustc_ast::Mutability; @@ -87,14 +87,27 @@ pub(super) fn check<'tcx>( arg_snip = format!("({arg_snip})").into(); } - span_lint_and_sugg( - cx, - SINGLE_ELEMENT_LOOP, - expr.span, - "for loop over a single element", - "try", - format!("{{\n{indent}let {pat_snip} = {prefix}{arg_snip};{block_str}}}"), - applicability, - ); + if clippy_utils::higher::Range::hir(arg_expression).is_some() { + let sugg = snippet(cx, arg_expression.span, ".."); + span_lint_and_sugg( + cx, + SINGLE_ELEMENT_LOOP, + arg.span, + "for loop over a single range inside an array, rather than iterating over the elements in the range directly", + "did you mean to iterate over the range instead?", + sugg.to_string(), + applicability, + ); + } else { + span_lint_and_sugg( + cx, + SINGLE_ELEMENT_LOOP, + expr.span, + "for loop over a single element", + "try", + format!("{{\n{indent}let {pat_snip} = {prefix}{arg_snip};{block_str}}}"), + applicability, + ); + } } } diff --git a/tests/ui/single_element_loop.fixed b/tests/ui/single_element_loop.fixed index a82eb6afcf5f3..4e59c76319863 100644 --- a/tests/ui/single_element_loop.fixed +++ b/tests/ui/single_element_loop.fixed @@ -15,23 +15,19 @@ fn main() { dbg!(item); } - { - let item = &(0..5); + for item in 0..5 { dbg!(item); } - { - let item = &mut (0..5); + for item in 0..5 { dbg!(item); } - { - let item = 0..5; + for item in 0..5 { dbg!(item); } - { - let item = 0..5; + for item in 0..5 { dbg!(item); } diff --git a/tests/ui/single_element_loop.stderr b/tests/ui/single_element_loop.stderr index 603dd7406e4e6..3937bd9574356 100644 --- a/tests/ui/single_element_loop.stderr +++ b/tests/ui/single_element_loop.stderr @@ -32,69 +32,29 @@ LL + dbg!(item); LL + } | -error: for loop over a single element - --> $DIR/single_element_loop.rs:16:5 - | -LL | / for item in &[0..5] { -LL | | dbg!(item); -LL | | } - | |_____^ - | -help: try - | -LL ~ { -LL + let item = &(0..5); -LL + dbg!(item); -LL + } +error: for loop over a single range inside an array, rather than iterating over the elements in the range directly + --> $DIR/single_element_loop.rs:16:17 | +LL | for item in &[0..5] { + | ^^^^^^^ help: did you mean to iterate over the range instead?: `0..5` -error: for loop over a single element - --> $DIR/single_element_loop.rs:20:5 - | -LL | / for item in [0..5].iter_mut() { -LL | | dbg!(item); -LL | | } - | |_____^ - | -help: try - | -LL ~ { -LL + let item = &mut (0..5); -LL + dbg!(item); -LL + } +error: for loop over a single range inside an array, rather than iterating over the elements in the range directly + --> $DIR/single_element_loop.rs:20:17 | +LL | for item in [0..5].iter_mut() { + | ^^^^^^^^^^^^^^^^^ help: did you mean to iterate over the range instead?: `0..5` -error: for loop over a single element - --> $DIR/single_element_loop.rs:24:5 - | -LL | / for item in [0..5] { -LL | | dbg!(item); -LL | | } - | |_____^ - | -help: try - | -LL ~ { -LL + let item = 0..5; -LL + dbg!(item); -LL + } +error: for loop over a single range inside an array, rather than iterating over the elements in the range directly + --> $DIR/single_element_loop.rs:24:17 | +LL | for item in [0..5] { + | ^^^^^^ help: did you mean to iterate over the range instead?: `0..5` -error: for loop over a single element - --> $DIR/single_element_loop.rs:28:5 - | -LL | / for item in [0..5].into_iter() { -LL | | dbg!(item); -LL | | } - | |_____^ - | -help: try - | -LL ~ { -LL + let item = 0..5; -LL + dbg!(item); -LL + } +error: for loop over a single range inside an array, rather than iterating over the elements in the range directly + --> $DIR/single_element_loop.rs:28:17 | +LL | for item in [0..5].into_iter() { + | ^^^^^^^^^^^^^^^^^^ help: did you mean to iterate over the range instead?: `0..5` error: for loop over a single element --> $DIR/single_element_loop.rs:47:5 From c58d692e1ff39b0bdffacb907510663f26bbd7e1 Mon Sep 17 00:00:00 2001 From: Christoph Beberweil Date: Fri, 24 Nov 2023 10:30:19 +0100 Subject: [PATCH 2/5] fix: 7125 update lint applicability to Unspecified --- clippy_lints/src/loops/single_element_loop.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/loops/single_element_loop.rs b/clippy_lints/src/loops/single_element_loop.rs index 7164bf06b8dd9..5740da528c255 100644 --- a/clippy_lints/src/loops/single_element_loop.rs +++ b/clippy_lints/src/loops/single_element_loop.rs @@ -96,7 +96,7 @@ pub(super) fn check<'tcx>( "for loop over a single range inside an array, rather than iterating over the elements in the range directly", "did you mean to iterate over the range instead?", sugg.to_string(), - applicability, + Applicability::Unspecified, ); } else { span_lint_and_sugg( From 2512341fe45f62c5e1e5aea0d70a0430b73e639b Mon Sep 17 00:00:00 2001 From: Christoph Beberweil Date: Fri, 24 Nov 2023 10:38:45 +0100 Subject: [PATCH 3/5] feat: 7125 shorten lint text --- clippy_lints/src/loops/single_element_loop.rs | 4 +++- tests/ui/single_element_loop.stderr | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/loops/single_element_loop.rs b/clippy_lints/src/loops/single_element_loop.rs index 5740da528c255..b3db88bd0987d 100644 --- a/clippy_lints/src/loops/single_element_loop.rs +++ b/clippy_lints/src/loops/single_element_loop.rs @@ -88,12 +88,14 @@ pub(super) fn check<'tcx>( } if clippy_utils::higher::Range::hir(arg_expression).is_some() { + let range_expr = snippet(cx, arg_expression.span, "?").to_string(); + let sugg = snippet(cx, arg_expression.span, ".."); span_lint_and_sugg( cx, SINGLE_ELEMENT_LOOP, arg.span, - "for loop over a single range inside an array, rather than iterating over the elements in the range directly", + format!("This loops only once with {pat_snip} being {range_expr}").as_str(), "did you mean to iterate over the range instead?", sugg.to_string(), Applicability::Unspecified, diff --git a/tests/ui/single_element_loop.stderr b/tests/ui/single_element_loop.stderr index 3937bd9574356..6d2a89a706a0d 100644 --- a/tests/ui/single_element_loop.stderr +++ b/tests/ui/single_element_loop.stderr @@ -32,25 +32,25 @@ LL + dbg!(item); LL + } | -error: for loop over a single range inside an array, rather than iterating over the elements in the range directly +error: This loops only once with item being 0..5 --> $DIR/single_element_loop.rs:16:17 | LL | for item in &[0..5] { | ^^^^^^^ help: did you mean to iterate over the range instead?: `0..5` -error: for loop over a single range inside an array, rather than iterating over the elements in the range directly +error: This loops only once with item being 0..5 --> $DIR/single_element_loop.rs:20:17 | LL | for item in [0..5].iter_mut() { | ^^^^^^^^^^^^^^^^^ help: did you mean to iterate over the range instead?: `0..5` -error: for loop over a single range inside an array, rather than iterating over the elements in the range directly +error: This loops only once with item being 0..5 --> $DIR/single_element_loop.rs:24:17 | LL | for item in [0..5] { | ^^^^^^ help: did you mean to iterate over the range instead?: `0..5` -error: for loop over a single range inside an array, rather than iterating over the elements in the range directly +error: This loops only once with item being 0..5 --> $DIR/single_element_loop.rs:28:17 | LL | for item in [0..5].into_iter() { From bce869f0c05ffe2ce341e79e2f4cf9c0a06078c1 Mon Sep 17 00:00:00 2001 From: Christoph Beberweil Date: Fri, 24 Nov 2023 17:29:03 +0100 Subject: [PATCH 4/5] fix: 7125 lint message should start with a small letter --- clippy_lints/src/loops/single_element_loop.rs | 2 +- tests/ui/single_element_loop.stderr | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/loops/single_element_loop.rs b/clippy_lints/src/loops/single_element_loop.rs index b3db88bd0987d..b5786231a1abc 100644 --- a/clippy_lints/src/loops/single_element_loop.rs +++ b/clippy_lints/src/loops/single_element_loop.rs @@ -95,7 +95,7 @@ pub(super) fn check<'tcx>( cx, SINGLE_ELEMENT_LOOP, arg.span, - format!("This loops only once with {pat_snip} being {range_expr}").as_str(), + format!("this loops only once with {pat_snip} being {range_expr}").as_str(), "did you mean to iterate over the range instead?", sugg.to_string(), Applicability::Unspecified, diff --git a/tests/ui/single_element_loop.stderr b/tests/ui/single_element_loop.stderr index 6d2a89a706a0d..c45f36b1d1c85 100644 --- a/tests/ui/single_element_loop.stderr +++ b/tests/ui/single_element_loop.stderr @@ -32,25 +32,25 @@ LL + dbg!(item); LL + } | -error: This loops only once with item being 0..5 +error: this loops only once with item being 0..5 --> $DIR/single_element_loop.rs:16:17 | LL | for item in &[0..5] { | ^^^^^^^ help: did you mean to iterate over the range instead?: `0..5` -error: This loops only once with item being 0..5 +error: this loops only once with item being 0..5 --> $DIR/single_element_loop.rs:20:17 | LL | for item in [0..5].iter_mut() { | ^^^^^^^^^^^^^^^^^ help: did you mean to iterate over the range instead?: `0..5` -error: This loops only once with item being 0..5 +error: this loops only once with item being 0..5 --> $DIR/single_element_loop.rs:24:17 | LL | for item in [0..5] { | ^^^^^^ help: did you mean to iterate over the range instead?: `0..5` -error: This loops only once with item being 0..5 +error: this loops only once with item being 0..5 --> $DIR/single_element_loop.rs:28:17 | LL | for item in [0..5].into_iter() { From f9c6335a0f1a2a0fb17a67d0aa72eae63bbd8a3f Mon Sep 17 00:00:00 2001 From: Christoph Beberweil Date: Fri, 24 Nov 2023 17:47:31 +0100 Subject: [PATCH 5/5] feat: 7125 code snippets are wrapped in backticks --- clippy_lints/src/loops/single_element_loop.rs | 2 +- tests/ui/single_element_loop.stderr | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/loops/single_element_loop.rs b/clippy_lints/src/loops/single_element_loop.rs index b5786231a1abc..4773a1454b7a0 100644 --- a/clippy_lints/src/loops/single_element_loop.rs +++ b/clippy_lints/src/loops/single_element_loop.rs @@ -95,7 +95,7 @@ pub(super) fn check<'tcx>( cx, SINGLE_ELEMENT_LOOP, arg.span, - format!("this loops only once with {pat_snip} being {range_expr}").as_str(), + format!("this loops only once with `{pat_snip}` being `{range_expr}`").as_str(), "did you mean to iterate over the range instead?", sugg.to_string(), Applicability::Unspecified, diff --git a/tests/ui/single_element_loop.stderr b/tests/ui/single_element_loop.stderr index c45f36b1d1c85..952d704143a5b 100644 --- a/tests/ui/single_element_loop.stderr +++ b/tests/ui/single_element_loop.stderr @@ -32,25 +32,25 @@ LL + dbg!(item); LL + } | -error: this loops only once with item being 0..5 +error: this loops only once with `item` being `0..5` --> $DIR/single_element_loop.rs:16:17 | LL | for item in &[0..5] { | ^^^^^^^ help: did you mean to iterate over the range instead?: `0..5` -error: this loops only once with item being 0..5 +error: this loops only once with `item` being `0..5` --> $DIR/single_element_loop.rs:20:17 | LL | for item in [0..5].iter_mut() { | ^^^^^^^^^^^^^^^^^ help: did you mean to iterate over the range instead?: `0..5` -error: this loops only once with item being 0..5 +error: this loops only once with `item` being `0..5` --> $DIR/single_element_loop.rs:24:17 | LL | for item in [0..5] { | ^^^^^^ help: did you mean to iterate over the range instead?: `0..5` -error: this loops only once with item being 0..5 +error: this loops only once with `item` being `0..5` --> $DIR/single_element_loop.rs:28:17 | LL | for item in [0..5].into_iter() {