Auto merge of rust-lang#11862 - christophbeberweil:7125-single-elemen…

…t-loop-over-range, r=llogiq suggest alternatives to iterate an array of ranges works towards rust-lang#7125 changelog: [`single_element_loop`]: suggest better syntax when iterating over an array of a single range `@thinkerdreamer` and myself worked on this issue during a workshop by `@llogiq` at the RustLab 2023 conference. It is our first contribution to clippy. When iterating over an array of only one element, _which is a range_, our change suggests to replace the array with the contained range itself. Additionally, a hint is printed stating that the user probably intended to iterate over the range and not the array. If the single element in the array is not a range, the previous suggestion in the form of `let {pat_snip} = {prefix}{arg_snip};{block_str}`is used. This change lints the array with the single range directly, so any prefixes or suffixes are covered as well.
rust-lang-ci · Nov 24, 2023 · 6cfbe57 · 6cfbe57
2 parents 3e7a63b + f9c6335
commit 6cfbe57
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 74 deletions.
diff --git 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,29 @@ 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 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,
+                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,
+            );
+        } 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
@@ -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
@@ -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: 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 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: 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 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: 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 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: this loops only once with `item` being `0..5`
+  --> $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