Auto merge of #11829 - J-ZhengLi:issue11438, r=matthiaskrgr

new lint to detect infinite loop

closes: #11438

changelog: add new lint to detect infinite loop

~*I'll change the lint name*~. Should I name it  `infinite_loop` or `infinite_loops` is fine? Ahhhh, English is hard...
This commit is contained in:
bors 2023-12-12 17:53:51 +00:00
commit 2e96c74dce
6 changed files with 798 additions and 1 deletions

View File

@ -5150,6 +5150,7 @@ Released 2018-09-13
[`inefficient_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inefficient_to_string [`inefficient_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inefficient_to_string
[`infallible_destructuring_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#infallible_destructuring_match [`infallible_destructuring_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#infallible_destructuring_match
[`infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_iter [`infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_iter
[`infinite_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_loop
[`inherent_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string [`inherent_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string
[`inherent_to_string_shadow_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string_shadow_display [`inherent_to_string_shadow_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string_shadow_display
[`init_numbered_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#init_numbered_fields [`init_numbered_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#init_numbered_fields

View File

@ -266,6 +266,7 @@
crate::loops::EXPLICIT_INTO_ITER_LOOP_INFO, crate::loops::EXPLICIT_INTO_ITER_LOOP_INFO,
crate::loops::EXPLICIT_ITER_LOOP_INFO, crate::loops::EXPLICIT_ITER_LOOP_INFO,
crate::loops::FOR_KV_MAP_INFO, crate::loops::FOR_KV_MAP_INFO,
crate::loops::INFINITE_LOOP_INFO,
crate::loops::ITER_NEXT_LOOP_INFO, crate::loops::ITER_NEXT_LOOP_INFO,
crate::loops::MANUAL_FIND_INFO, crate::loops::MANUAL_FIND_INFO,
crate::loops::MANUAL_FLATTEN_INFO, crate::loops::MANUAL_FLATTEN_INFO,

View File

@ -0,0 +1,125 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::{fn_def_id, is_lint_allowed};
use hir::intravisit::{walk_expr, Visitor};
use hir::{Expr, ExprKind, FnRetTy, FnSig, Node};
use rustc_ast::Label;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use super::INFINITE_LOOP;
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &Expr<'_>,
loop_block: &'tcx hir::Block<'_>,
label: Option<Label>,
) {
if is_lint_allowed(cx, INFINITE_LOOP, expr.hir_id) {
return;
}
// Skip check if this loop is not in a function/method/closure. (In some weird case)
let Some(parent_fn_ret) = get_parent_fn_ret_ty(cx, expr) else {
return;
};
// Or, its parent function is already returning `Never`
if matches!(
parent_fn_ret,
FnRetTy::Return(hir::Ty {
kind: hir::TyKind::Never,
..
})
) {
return;
}
let mut loop_visitor = LoopVisitor {
cx,
label,
is_finite: false,
loop_depth: 0,
};
loop_visitor.visit_block(loop_block);
let is_finite_loop = loop_visitor.is_finite;
if !is_finite_loop {
span_lint_and_then(cx, INFINITE_LOOP, expr.span, "infinite loop detected", |diag| {
if let FnRetTy::DefaultReturn(ret_span) = parent_fn_ret {
diag.span_suggestion(
ret_span,
"if this is intentional, consider specifing `!` as function return",
" -> !",
Applicability::MaybeIncorrect,
);
} else {
diag.help("if this is not intended, try adding a `break` or `return` condition in the loop");
}
});
}
}
fn get_parent_fn_ret_ty<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> Option<FnRetTy<'tcx>> {
for (_, parent_node) in cx.tcx.hir().parent_iter(expr.hir_id) {
match parent_node {
Node::Item(hir::Item {
kind: hir::ItemKind::Fn(FnSig { decl, .. }, _, _),
..
})
| Node::TraitItem(hir::TraitItem {
kind: hir::TraitItemKind::Fn(FnSig { decl, .. }, _),
..
})
| Node::ImplItem(hir::ImplItem {
kind: hir::ImplItemKind::Fn(FnSig { decl, .. }, _),
..
})
| Node::Expr(Expr {
kind: ExprKind::Closure(hir::Closure { fn_decl: decl, .. }),
..
}) => return Some(decl.output),
_ => (),
}
}
None
}
struct LoopVisitor<'hir, 'tcx> {
cx: &'hir LateContext<'tcx>,
label: Option<Label>,
loop_depth: usize,
is_finite: bool,
}
impl<'hir> Visitor<'hir> for LoopVisitor<'hir, '_> {
fn visit_expr(&mut self, ex: &'hir Expr<'_>) {
match &ex.kind {
ExprKind::Break(hir::Destination { label, .. }, ..) => {
// Assuming breaks the loop when `loop_depth` is 0,
// as it could only means this `break` breaks current loop or any of its upper loop.
// Or, the depth is not zero but the label is matched.
if self.loop_depth == 0 || (label.is_some() && *label == self.label) {
self.is_finite = true;
}
},
ExprKind::Ret(..) => self.is_finite = true,
ExprKind::Loop(..) => {
self.loop_depth += 1;
walk_expr(self, ex);
self.loop_depth = self.loop_depth.saturating_sub(1);
},
_ => {
// Calls to a function that never return
if let Some(did) = fn_def_id(self.cx, ex) {
let fn_ret_ty = self.cx.tcx.fn_sig(did).skip_binder().output().skip_binder();
if fn_ret_ty.is_never() {
self.is_finite = true;
return;
}
}
walk_expr(self, ex);
},
}
}
}

View File

@ -3,6 +3,7 @@
mod explicit_into_iter_loop; mod explicit_into_iter_loop;
mod explicit_iter_loop; mod explicit_iter_loop;
mod for_kv_map; mod for_kv_map;
mod infinite_loop;
mod iter_next_loop; mod iter_next_loop;
mod manual_find; mod manual_find;
mod manual_flatten; mod manual_flatten;
@ -635,6 +636,48 @@
"checking for emptiness of a `Vec` in the loop condition and popping an element in the body" "checking for emptiness of a `Vec` in the loop condition and popping an element in the body"
} }
declare_clippy_lint! {
/// ### What it does
/// Checks for infinite loops in a function where the return type is not `!`
/// and lint accordingly.
///
/// ### Why is this bad?
/// A loop should be gently exited somewhere, or at least mark its parent function as
/// never return (`!`).
///
/// ### Example
/// ```no_run,ignore
/// fn run_forever() {
/// loop {
/// // do something
/// }
/// }
/// ```
/// If infinite loops are as intended:
/// ```no_run,ignore
/// fn run_forever() -> ! {
/// loop {
/// // do something
/// }
/// }
/// ```
/// Otherwise add a `break` or `return` condition:
/// ```no_run,ignore
/// fn run_forever() {
/// loop {
/// // do something
/// if condition {
/// break;
/// }
/// }
/// }
/// ```
#[clippy::version = "1.75.0"]
pub INFINITE_LOOP,
restriction,
"possibly unintended infinite loop"
}
pub struct Loops { pub struct Loops {
msrv: Msrv, msrv: Msrv,
enforce_iter_loop_reborrow: bool, enforce_iter_loop_reborrow: bool,
@ -669,6 +712,7 @@ pub fn new(msrv: Msrv, enforce_iter_loop_reborrow: bool) -> Self {
MANUAL_FIND, MANUAL_FIND,
MANUAL_WHILE_LET_SOME, MANUAL_WHILE_LET_SOME,
UNUSED_ENUMERATE_INDEX, UNUSED_ENUMERATE_INDEX,
INFINITE_LOOP,
]); ]);
impl<'tcx> LateLintPass<'tcx> for Loops { impl<'tcx> LateLintPass<'tcx> for Loops {
@ -707,10 +751,11 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
// check for `loop { if let {} else break }` that could be `while let` // check for `loop { if let {} else break }` that could be `while let`
// (also matches an explicit "match" instead of "if let") // (also matches an explicit "match" instead of "if let")
// (even if the "match" or "if let" is used for declaration) // (even if the "match" or "if let" is used for declaration)
if let ExprKind::Loop(block, _, LoopSource::Loop, _) = expr.kind { if let ExprKind::Loop(block, label, LoopSource::Loop, _) = expr.kind {
// also check for empty `loop {}` statements, skipping those in #[panic_handler] // also check for empty `loop {}` statements, skipping those in #[panic_handler]
empty_loop::check(cx, expr, block); empty_loop::check(cx, expr, block);
while_let_loop::check(cx, expr, block); while_let_loop::check(cx, expr, block);
infinite_loop::check(cx, expr, block, label);
} }
while_let_on_iterator::check(cx, expr); while_let_on_iterator::check(cx, expr);

366
tests/ui/infinite_loops.rs Normal file
View File

@ -0,0 +1,366 @@
//@no-rustfix
#![allow(clippy::never_loop)]
#![warn(clippy::infinite_loop)]
fn do_something() {}
fn no_break() {
loop {
//~^ ERROR: infinite loop detected
do_something();
}
}
fn all_inf() {
loop {
//~^ ERROR: infinite loop detected
loop {
//~^ ERROR: infinite loop detected
loop {
//~^ ERROR: infinite loop detected
do_something();
}
}
do_something();
}
}
fn no_break_return_some_ty() -> Option<u8> {
loop {
do_something();
return None;
}
loop {
//~^ ERROR: infinite loop detected
do_something();
}
}
fn no_break_never_ret() -> ! {
loop {
do_something();
}
}
fn no_break_never_ret_noise() {
loop {
fn inner_fn() -> ! {
std::process::exit(0);
}
do_something();
}
}
fn has_direct_break_1() {
loop {
do_something();
break;
}
}
fn has_direct_break_2() {
'outer: loop {
do_something();
break 'outer;
}
}
fn has_indirect_break_1(cond: bool) {
'outer: loop {
loop {
if cond {
break 'outer;
}
}
}
}
fn has_indirect_break_2(stop_num: i32) {
'outer: loop {
for x in 0..5 {
if x == stop_num {
break 'outer;
}
}
}
}
fn break_inner_but_not_outer_1(cond: bool) {
loop {
//~^ ERROR: infinite loop detected
loop {
if cond {
break;
}
}
}
}
fn break_inner_but_not_outer_2(cond: bool) {
loop {
//~^ ERROR: infinite loop detected
'inner: loop {
loop {
if cond {
break 'inner;
}
}
}
}
}
fn break_outer_but_not_inner() {
loop {
loop {
//~^ ERROR: infinite loop detected
do_something();
}
break;
}
}
fn can_break_both_inner_and_outer(cond: bool) {
'outer: loop {
loop {
if cond {
break 'outer;
} else {
break;
}
}
}
}
fn break_wrong_loop(cond: bool) {
// 'inner has statement to break 'outer loop, but it was breaked early by a labeled child loop
'outer: loop {
loop {
//~^ ERROR: infinite loop detected
'inner: loop {
loop {
loop {
break 'inner;
}
break 'outer;
}
}
}
}
}
fn has_direct_return(cond: bool) {
loop {
if cond {
return;
}
}
}
fn ret_in_inner(cond: bool) {
loop {
loop {
if cond {
return;
}
}
}
}
enum Foo {
A,
B,
C,
}
fn match_like() {
let opt: Option<u8> = Some(1);
loop {
//~^ ERROR: infinite loop detected
match opt {
Some(v) => {
println!("{v}");
},
None => {
do_something();
},
}
}
loop {
match opt {
Some(v) => {
println!("{v}");
},
None => {
do_something();
break;
},
}
}
let result: Result<u8, u16> = Ok(1);
loop {
let _val = match result {
Ok(1) => 1 + 1,
Ok(v) => v / 2,
Err(_) => return,
};
}
loop {
let Ok(_val) = result else { return };
}
loop {
let Ok(_val) = result.map(|v| 10) else { break };
}
loop {
//~^ ERROR: infinite loop detected
let _x = matches!(result, Ok(v) if v != 0).then_some(0);
}
loop {
//~^ ERROR: infinite loop detected
// This `return` does not return the function, so it doesn't count
let _x = matches!(result, Ok(v) if v != 0).then(|| {
if true {
return;
}
do_something();
});
}
let mut val = 0;
let mut fooc = Foo::C;
loop {
val = match fooc {
Foo::A => 0,
Foo::B => {
fooc = Foo::C;
1
},
Foo::C => break,
};
}
loop {
val = match fooc {
Foo::A => 0,
Foo::B => 1,
Foo::C => {
break;
},
};
}
}
macro_rules! set_or_ret {
($opt:expr, $a:expr) => {{
match $opt {
Some(val) => $a = val,
None => return,
}
}};
}
fn ret_in_macro(opt: Option<u8>) {
let opt: Option<u8> = Some(1);
let mut a: u8 = 0;
loop {
set_or_ret!(opt, a);
}
let res: Result<bool, u8> = Ok(true);
loop {
match res {
Ok(true) => set_or_ret!(opt, a),
_ => do_something(),
}
}
}
fn panic_like_macros_1() {
loop {
do_something();
panic!();
}
}
fn panic_like_macros_2() {
let mut x = 0;
loop {
do_something();
if true {
todo!();
}
}
loop {
do_something();
x += 1;
assert_eq!(x, 0);
}
loop {
do_something();
assert!(x % 2 == 0);
}
loop {
do_something();
match Some(1) {
Some(n) => println!("{n}"),
None => unreachable!("It won't happen"),
}
}
}
fn exit_directly(cond: bool) {
loop {
if cond {
std::process::exit(0);
}
}
}
trait MyTrait {
fn problematic_trait_method() {
loop {
//~^ ERROR: infinite loop detected
do_something();
}
}
fn could_be_problematic();
}
impl MyTrait for String {
fn could_be_problematic() {
loop {
//~^ ERROR: infinite loop detected
do_something();
}
}
}
fn inf_loop_in_closure() {
let _loop_forever = || {
loop {
//~^ ERROR: infinite loop detected
do_something();
}
};
let _somehow_ok = || -> ! {
loop {
do_something();
}
};
}
fn inf_loop_in_res() -> Result<(), i32> {
Ok(loop {
do_something()
})
}
fn main() {}

View File

@ -0,0 +1,259 @@
error: infinite loop detected
--> $DIR/infinite_loops.rs:8:5
|
LL | / loop {
LL | |
LL | | do_something();
LL | | }
| |_____^
|
= note: `-D clippy::infinite-loop` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::infinite_loop)]`
help: if this is intentional, consider specifing `!` as function return
|
LL | fn no_break() -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:15:5
|
LL | / loop {
LL | |
LL | | loop {
LL | |
... |
LL | | do_something();
LL | | }
| |_____^
|
help: if this is intentional, consider specifing `!` as function return
|
LL | fn all_inf() -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:17:9
|
LL | / loop {
LL | |
LL | | loop {
LL | |
LL | | do_something();
LL | | }
LL | | }
| |_________^
|
help: if this is intentional, consider specifing `!` as function return
|
LL | fn all_inf() -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:19:13
|
LL | / loop {
LL | |
LL | | do_something();
LL | | }
| |_____________^
|
help: if this is intentional, consider specifing `!` as function return
|
LL | fn all_inf() -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:33:5
|
LL | / loop {
LL | |
LL | | do_something();
LL | | }
| |_____^
|
= help: if this is not intended, try adding a `break` or `return` condition in the loop
error: infinite loop detected
--> $DIR/infinite_loops.rs:46:5
|
LL | / loop {
LL | | fn inner_fn() -> ! {
LL | | std::process::exit(0);
LL | | }
LL | | do_something();
LL | | }
| |_____^
|
help: if this is intentional, consider specifing `!` as function return
|
LL | fn no_break_never_ret_noise() -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:89:5
|
LL | / loop {
LL | |
LL | | loop {
LL | | if cond {
... |
LL | | }
LL | | }
| |_____^
|
help: if this is intentional, consider specifing `!` as function return
|
LL | fn break_inner_but_not_outer_1(cond: bool) -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:100:5
|
LL | / loop {
LL | |
LL | | 'inner: loop {
LL | | loop {
... |
LL | | }
LL | | }
| |_____^
|
help: if this is intentional, consider specifing `!` as function return
|
LL | fn break_inner_but_not_outer_2(cond: bool) -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:114:9
|
LL | / loop {
LL | |
LL | | do_something();
LL | | }
| |_________^
|
help: if this is intentional, consider specifing `!` as function return
|
LL | fn break_outer_but_not_inner() -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:137:9
|
LL | / loop {
LL | |
LL | | 'inner: loop {
LL | | loop {
... |
LL | | }
LL | | }
| |_________^
|
help: if this is intentional, consider specifing `!` as function return
|
LL | fn break_wrong_loop(cond: bool) -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:177:5
|
LL | / loop {
LL | |
LL | | match opt {
LL | | Some(v) => {
... |
LL | | }
LL | | }
| |_____^
|
help: if this is intentional, consider specifing `!` as function return
|
LL | fn match_like() -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:218:5
|
LL | / loop {
LL | |
LL | | let _x = matches!(result, Ok(v) if v != 0).then_some(0);
LL | | }
| |_____^
|
help: if this is intentional, consider specifing `!` as function return
|
LL | fn match_like() -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:223:5
|
LL | / loop {
LL | |
LL | | // This `return` does not return the function, so it doesn't count
LL | | let _x = matches!(result, Ok(v) if v != 0).then(|| {
... |
LL | | });
LL | | }
| |_____^
|
help: if this is intentional, consider specifing `!` as function return
|
LL | fn match_like() -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:328:9
|
LL | / loop {
LL | |
LL | | do_something();
LL | | }
| |_________^
|
help: if this is intentional, consider specifing `!` as function return
|
LL | fn problematic_trait_method() -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:338:9
|
LL | / loop {
LL | |
LL | | do_something();
LL | | }
| |_________^
|
help: if this is intentional, consider specifing `!` as function return
|
LL | fn could_be_problematic() -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:347:9
|
LL | / loop {
LL | |
LL | | do_something();
LL | | }
| |_________^
|
help: if this is intentional, consider specifing `!` as function return
|
LL | let _loop_forever = || -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:361:8
|
LL | Ok(loop {
| ________^
LL | | do_something()
LL | | })
| |_____^
|
= help: if this is not intended, try adding a `break` or `return` condition in the loop
error: aborting due to 17 previous errors