add new lint: for_unbounded_range (#16257)

*[View all
comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust-clippy/pull/16257)*

This adds a Clippy lint for the possible confusing behaviour pointed out
in rust-lang/rust#150084 and
rust-lang/rust#111514, and discussed in
rust-lang/libs-team#304.

While no consensus have been reached in the API Change Proposal linked
above I thought that it would still make sense to add a Clippy lint for
the time being.

Open questions:
- Which group should it be in?
- Should it give warnings for `AsciiChar`, `Ipv4Addr` and `Ipv6Addr`?
(Currently these will always panic when running over the max value)

changelog: new lint: [`for_unbounded_range`]

Author: samueltardieu

CHANGELOG.md

@@ -6840,6 +6840,7 @@ Released 2018-09-13
 [`for_loop_over_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loop_over_option
 [`for_loop_over_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loop_over_result
 [`for_loops_over_fallibles`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loops_over_fallibles
+[`for_unbounded_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_unbounded_range
 [`forget_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_copy
 [`forget_non_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_non_drop
 [`forget_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_ref

clippy_lints/src/declared_lints.rs

@@ -276,6 +276,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::loops::EXPLICIT_INTO_ITER_LOOP_INFO,
     crate::loops::EXPLICIT_ITER_LOOP_INFO,
     crate::loops::FOR_KV_MAP_INFO,
+    crate::loops::FOR_UNBOUNDED_RANGE_INFO,
     crate::loops::INFINITE_LOOP_INFO,
     crate::loops::ITER_NEXT_LOOP_INFO,
     crate::loops::MANUAL_FIND_INFO,

clippy_lints/src/loops/for_unbounded_range.rs

@@ -0,0 +1,42 @@
+use super::FOR_UNBOUNDED_RANGE;
+use super::infinite_loop::LoopVisitor;
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::higher;
+use rustc_ast::Label;
+use rustc_hir::Expr;
+use rustc_hir::intravisit::Visitor as _;
+use rustc_lint::LateContext;
+use rustc_span::Span;
+
+pub fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    label: Option<Label>,
+    arg: &'tcx Expr<'tcx>,
+    span: Span,
+    body: &'tcx Expr<'tcx>,
+) {
+    if let Some(range) = higher::Range::hir(cx, arg)
+        && let Some(range_start) = range.start
+        && let None = range.end
+        && let ty = cx.typeck_results().expr_ty_adjusted(range_start)
+        && (ty.is_integral() || ty.is_char())
+    {
+        let mut loop_visitor = LoopVisitor::new(cx, label);
+
+        loop_visitor.visit_expr(body);
+
+        if loop_visitor.is_finite {
+            // The loop is likely finite.
+            return;
+        }
+
+        span_lint_and_then(cx, FOR_UNBOUNDED_RANGE, span, "for loop on unbounded range", |diag| {
+            diag.span_suggestion_verbose(
+                arg.span.shrink_to_hi(),
+                "for loops over unbounded ranges will wrap around and may panic, consider adding an inclusive high bound",
+                format!("={ty}::MAX"),
+                rustc_errors::Applicability::MaybeIncorrect,
+            );
+        });
+    }
+}

clippy_lints/src/loops/infinite_loop.rs

@@ -39,13 +39,7 @@ pub(super) fn check<'tcx>(
         return;
     }
 
-    let mut loop_visitor = LoopVisitor {
-        cx,
-        label,
-        inner_labels: label.into_iter().collect(),
-        loop_depth: 0,
-        is_finite: false,
-    };
+    let mut loop_visitor = LoopVisitor::new(cx, label);
     loop_visitor.visit_block(loop_block);
 
     let is_finite_loop = loop_visitor.is_finite;
@@ -146,12 +140,24 @@ fn get_parent_fn_ret_ty<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> Option
     None
 }
 
-struct LoopVisitor<'hir, 'tcx> {
-    cx: &'hir LateContext<'tcx>,
-    label: Option<Label>,
-    inner_labels: Vec<Label>,
-    loop_depth: usize,
-    is_finite: bool,
+pub(super) struct LoopVisitor<'hir, 'tcx> {
+    pub cx: &'hir LateContext<'tcx>,
+    pub label: Option<Label>,
+    pub inner_labels: Vec<Label>,
+    pub loop_depth: usize,
+    pub is_finite: bool,
+}
+
+impl<'hir, 'tcx> LoopVisitor<'hir, 'tcx> {
+    pub fn new(cx: &'hir LateContext<'tcx>, label: Option<Label>) -> Self {
+        LoopVisitor {
+            cx,
+            label,
+            inner_labels: label.into_iter().collect(),
+            loop_depth: 0,
+            is_finite: false,
+        }
+    }
 }
 
 impl<'hir> Visitor<'hir> for LoopVisitor<'hir, '_> {

clippy_lints/src/loops/mod.rs

@@ -4,6 +4,7 @@ mod explicit_counter_loop;
 mod explicit_into_iter_loop;
 mod explicit_iter_loop;
 mod for_kv_map;
+mod for_unbounded_range;
 mod infinite_loop;
 mod iter_next_loop;
 mod manual_find;
@@ -236,6 +237,35 @@ declare_clippy_lint! {
     "looping on a map using `iter` when `keys` or `values` would do"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for unbounded for loops over char or integers.
+    ///
+    /// ### Why is this bad?
+    /// Using a unbounded range over char and integers will unexpectedly not handle overflows so it will lead to panics
+    /// or infinite loops.
+    ///
+    /// Instead there should be a max value set, usually the `MAX` constant for a given type such as `'\0'..char::MAX`
+    /// or `250..u8::MAX`.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// for i in 250u8.. {
+    ///   println!("{i}");
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// for i in 250u8..=u8::MAX {
+    ///   println!("{i}");
+    /// }
+    /// ```
+    #[clippy::version = "1.98.0"]
+    pub FOR_UNBOUNDED_RANGE,
+    suspicious,
+    "using a for loop over unbounded range such as `0..`"
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for infinite loops in a function where the return type is not `!`
@@ -792,6 +822,7 @@ impl_lint_pass!(Loops => [
     EXPLICIT_INTO_ITER_LOOP,
     EXPLICIT_ITER_LOOP,
     FOR_KV_MAP,
+    FOR_UNBOUNDED_RANGE,
     INFINITE_LOOP,
     ITER_NEXT_LOOP,
     MANUAL_FIND,
@@ -951,6 +982,7 @@ impl Loops {
         manual_find::check(cx, pat, arg, body, span, expr);
         unused_enumerate_index::check(cx, arg, pat, None, body);
         char_indices_as_byte_indices::check(cx, pat, arg, body);
+        for_unbounded_range::check(cx, label, arg, span, body);
     }
 
     fn check_for_loop_arg(&self, cx: &LateContext<'_>, _: &Pat<'_>, arg: &Expr<'_>) {

tests/ui/for_unbounded_range.fixed

@@ -0,0 +1,35 @@
+#![warn(clippy::for_unbounded_range)]
+
+fn do_something<T: Copy>(_t: T) {}
+
+fn main() {
+    for i in 0_u8..=u8::MAX {
+        do_something(i);
+    }
+
+    for i in 0_u8.. {
+        if i > 2 {
+            break;
+        }
+        do_something(i);
+    }
+
+    'outer: for i in 0_u8..10 {
+        for i in 0_u8.. {
+            if i > 2 {
+                break 'outer;
+            }
+            do_something(i);
+        }
+    }
+
+    for i in 0_u8..=u8::MAX {
+        //~^ for_unbounded_range
+        do_something(i);
+    }
+
+    for i in '\0'..=char::MAX {
+        //~^ for_unbounded_range
+        do_something(i);
+    }
+}

tests/ui/for_unbounded_range.rs

@@ -0,0 +1,35 @@
+#![warn(clippy::for_unbounded_range)]
+
+fn do_something<T: Copy>(_t: T) {}
+
+fn main() {
+    for i in 0_u8..=u8::MAX {
+        do_something(i);
+    }
+
+    for i in 0_u8.. {
+        if i > 2 {
+            break;
+        }
+        do_something(i);
+    }
+
+    'outer: for i in 0_u8..10 {
+        for i in 0_u8.. {
+            if i > 2 {
+                break 'outer;
+            }
+            do_something(i);
+        }
+    }
+
+    for i in 0_u8.. {
+        //~^ for_unbounded_range
+        do_something(i);
+    }
+
+    for i in '\0'.. {
+        //~^ for_unbounded_range
+        do_something(i);
+    }
+}

tests/ui/for_unbounded_range.stderr

@@ -0,0 +1,32 @@
+error: for loop on unbounded range
+  --> tests/ui/for_unbounded_range.rs:26:5
+   |
+LL | /     for i in 0_u8.. {
+LL | |
+LL | |         do_something(i);
+LL | |     }
+   | |_____^
+   |
+   = note: `-D clippy::for-unbounded-range` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::for_unbounded_range)]`
+help: for loops over unbounded ranges will wrap around and may panic, consider adding an inclusive high bound
+   |
+LL |     for i in 0_u8..=u8::MAX {
+   |                    ++++++++
+
+error: for loop on unbounded range
+  --> tests/ui/for_unbounded_range.rs:31:5
+   |
+LL | /     for i in '\0'.. {
+LL | |
+LL | |         do_something(i);
+LL | |     }
+   | |_____^
+   |
+help: for loops over unbounded ranges will wrap around and may panic, consider adding an inclusive high bound
+   |
+LL |     for i in '\0'..=char::MAX {
+   |                    ++++++++++
+
+error: aborting due to 2 previous errors
+

tests/ui/manual_memcpy/without_loop_counters.fixed

@@ -93,6 +93,7 @@ pub fn manual_copy(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) {
     dst[..0].copy_from_slice(&src[..0]);
 
     // `RangeTo` `for` loop - don't trigger lint
+    #[expect(clippy::for_unbounded_range)]
     for i in 0.. {
         dst[i] = src[i];
     }

tests/ui/manual_memcpy/without_loop_counters.rs

@@ -141,6 +141,7 @@ pub fn manual_copy(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) {
     }
 
     // `RangeTo` `for` loop - don't trigger lint
+    #[expect(clippy::for_unbounded_range)]
     for i in 0.. {
         dst[i] = src[i];
     }