Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
42 changes: 42 additions & 0 deletions clippy_lints/src/loops/for_unbounded_range.rs
Original file line number Diff line number Diff line change
@@ -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,
);
});
}
}
32 changes: 19 additions & 13 deletions clippy_lints/src/loops/infinite_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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, '_> {
Expand Down
32 changes: 32 additions & 0 deletions clippy_lints/src/loops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 `!`
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<'_>) {
Expand Down
35 changes: 35 additions & 0 deletions tests/ui/for_unbounded_range.fixed
Original file line number Diff line number Diff line change
@@ -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);
}
}
35 changes: 35 additions & 0 deletions tests/ui/for_unbounded_range.rs
Original file line number Diff line number Diff line change
@@ -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);
}
}
32 changes: 32 additions & 0 deletions tests/ui/for_unbounded_range.stderr
Original file line number Diff line number Diff line change
@@ -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

1 change: 1 addition & 0 deletions tests/ui/manual_memcpy/without_loop_counters.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand Down
1 change: 1 addition & 0 deletions tests/ui/manual_memcpy/without_loop_counters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand Down
12 changes: 6 additions & 6 deletions tests/ui/manual_memcpy/without_loop_counters.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ LL | | }
| |_____^ help: try replacing the loop by: `dst[..0].copy_from_slice(&src[..0]);`

error: it looks like you're manually copying between slices
--> tests/ui/manual_memcpy/without_loop_counters.rs:162:5
--> tests/ui/manual_memcpy/without_loop_counters.rs:163:5
|
LL | / for i in 0..4 {
LL | |
Expand All @@ -139,7 +139,7 @@ LL | | }
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src[..4]);`

error: it looks like you're manually copying between slices
--> tests/ui/manual_memcpy/without_loop_counters.rs:169:5
--> tests/ui/manual_memcpy/without_loop_counters.rs:170:5
|
LL | / for i in 0..5 {
LL | |
Expand All @@ -149,7 +149,7 @@ LL | | }
| |_____^ help: try replacing the loop by: `dst[..5].copy_from_slice(&src);`

error: it looks like you're manually copying between slices
--> tests/ui/manual_memcpy/without_loop_counters.rs:176:5
--> tests/ui/manual_memcpy/without_loop_counters.rs:177:5
|
LL | / for i in 0..5 {
LL | |
Expand All @@ -159,7 +159,7 @@ LL | | }
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src);`

error: it looks like you're manually copying between slices
--> tests/ui/manual_memcpy/without_loop_counters.rs:224:5
--> tests/ui/manual_memcpy/without_loop_counters.rs:225:5
|
LL | / for i in 0..5 {
LL | |
Expand All @@ -169,7 +169,7 @@ LL | | }
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src[0]);`

error: it looks like you're manually copying between slices
--> tests/ui/manual_memcpy/without_loop_counters.rs:231:5
--> tests/ui/manual_memcpy/without_loop_counters.rs:232:5
|
LL | / for i in 0..5 {
LL | |
Expand All @@ -179,7 +179,7 @@ LL | | }
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src[0][1]);`

error: it looks like you're manually copying between slices
--> tests/ui/manual_memcpy/without_loop_counters.rs:240:5
--> tests/ui/manual_memcpy/without_loop_counters.rs:241:5
|
LL | / for i in 0..src.len() {
LL | |
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/ty_fn_sig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Regression test

pub fn retry<F: Fn()>(f: F) {
for _i in 0.. {
for _i in 0..=i32::MAX {
f();
}
}
Expand Down