Skip to content

Commit 2e96c74

Browse files
borsbors
committed
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...
2 parents 52deee2 + 758d0e8 commit 2e96c74

File tree

6 files changed

+798
-1
lines changed

6 files changed

+798
-1
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5150,6 +5150,7 @@ Released 2018-09-13
51505150
[`inefficient_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inefficient_to_string
51515151
[`infallible_destructuring_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#infallible_destructuring_match
51525152
[`infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_iter
5153+
[`infinite_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_loop
51535154
[`inherent_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string
51545155
[`inherent_to_string_shadow_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string_shadow_display
51555156
[`init_numbered_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#init_numbered_fields

‎clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
266266
crate::loops::EXPLICIT_INTO_ITER_LOOP_INFO,
267267
crate::loops::EXPLICIT_ITER_LOOP_INFO,
268268
crate::loops::FOR_KV_MAP_INFO,
269+
crate::loops::INFINITE_LOOP_INFO,
269270
crate::loops::ITER_NEXT_LOOP_INFO,
270271
crate::loops::MANUAL_FIND_INFO,
271272
crate::loops::MANUAL_FLATTEN_INFO,

‎clippy_lints/src/loops/infinite_loop.rs

+125
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::{fn_def_id, is_lint_allowed};
3+
use hir::intravisit::{walk_expr, Visitor};
4+
use hir::{Expr, ExprKind, FnRetTy, FnSig, Node};
5+
use rustc_ast::Label;
6+
use rustc_errors::Applicability;
7+
use rustc_hir as hir;
8+
use rustc_lint::LateContext;
9+
10+
use super::INFINITE_LOOP;
11+
12+
pub(super) fn check<'tcx>(
13+
cx: &LateContext<'tcx>,
14+
expr: &Expr<'_>,
15+
loop_block: &'tcx hir::Block<'_>,
16+
label: Option<Label>,
17+
) {
18+
if is_lint_allowed(cx, INFINITE_LOOP, expr.hir_id) {
19+
return;
20+
}
21+
22+
// Skip check if this loop is not in a function/method/closure. (In some weird case)
23+
let Some(parent_fn_ret) = get_parent_fn_ret_ty(cx, expr) else {
24+
return;
25+
};
26+
// Or, its parent function is already returning `Never`
27+
if matches!(
28+
parent_fn_ret,
29+
FnRetTy::Return(hir::Ty {
30+
kind: hir::TyKind::Never,
31+
..
32+
})
33+
) {
34+
return;
35+
}
36+
37+
let mut loop_visitor = LoopVisitor {
38+
cx,
39+
label,
40+
is_finite: false,
41+
loop_depth: 0,
42+
};
43+
loop_visitor.visit_block(loop_block);
44+
45+
let is_finite_loop = loop_visitor.is_finite;
46+
47+
if !is_finite_loop {
48+
span_lint_and_then(cx, INFINITE_LOOP, expr.span, "infinite loop detected", |diag| {
49+
if let FnRetTy::DefaultReturn(ret_span) = parent_fn_ret {
50+
diag.span_suggestion(
51+
ret_span,
52+
"if this is intentional, consider specifing `!` as function return",
53+
" -> !",
54+
Applicability::MaybeIncorrect,
55+
);
56+
} else {
57+
diag.help("if this is not intended, try adding a `break` or `return` condition in the loop");
58+
}
59+
});
60+
}
61+
}
62+
63+
fn get_parent_fn_ret_ty<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> Option<FnRetTy<'tcx>> {
64+
for (_, parent_node) in cx.tcx.hir().parent_iter(expr.hir_id) {
65+
match parent_node {
66+
Node::Item(hir::Item {
67+
kind: hir::ItemKind::Fn(FnSig { decl, .. }, _, _),
68+
..
69+
})
70+
| Node::TraitItem(hir::TraitItem {
71+
kind: hir::TraitItemKind::Fn(FnSig { decl, .. }, _),
72+
..
73+
})
74+
| Node::ImplItem(hir::ImplItem {
75+
kind: hir::ImplItemKind::Fn(FnSig { decl, .. }, _),
76+
..
77+
})
78+
| Node::Expr(Expr {
79+
kind: ExprKind::Closure(hir::Closure { fn_decl: decl, .. }),
80+
..
81+
}) => return Some(decl.output),
82+
_ => (),
83+
}
84+
}
85+
None
86+
}
87+
88+
struct LoopVisitor<'hir, 'tcx> {
89+
cx: &'hir LateContext<'tcx>,
90+
label: Option<Label>,
91+
loop_depth: usize,
92+
is_finite: bool,
93+
}
94+
95+
impl<'hir> Visitor<'hir> for LoopVisitor<'hir, '_> {
96+
fn visit_expr(&mut self, ex: &'hir Expr<'_>) {
97+
match &ex.kind {
98+
ExprKind::Break(hir::Destination { label, .. }, ..) => {
99+
// Assuming breaks the loop when `loop_depth` is 0,
100+
// as it could only means this `break` breaks current loop or any of its upper loop.
101+
// Or, the depth is not zero but the label is matched.
102+
if self.loop_depth == 0 || (label.is_some() && *label == self.label) {
103+
self.is_finite = true;
104+
}
105+
},
106+
ExprKind::Ret(..) => self.is_finite = true,
107+
ExprKind::Loop(..) => {
108+
self.loop_depth += 1;
109+
walk_expr(self, ex);
110+
self.loop_depth = self.loop_depth.saturating_sub(1);
111+
},
112+
_ => {
113+
// Calls to a function that never return
114+
if let Some(did) = fn_def_id(self.cx, ex) {
115+
let fn_ret_ty = self.cx.tcx.fn_sig(did).skip_binder().output().skip_binder();
116+
if fn_ret_ty.is_never() {
117+
self.is_finite = true;
118+
return;
119+
}
120+
}
121+
walk_expr(self, ex);
122+
},
123+
}
124+
}
125+
}

‎clippy_lints/src/loops/mod.rs

+46-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ mod explicit_counter_loop;
33
mod explicit_into_iter_loop;
44
mod explicit_iter_loop;
55
mod for_kv_map;
6+
mod infinite_loop;
67
mod iter_next_loop;
78
mod manual_find;
89
mod manual_flatten;
@@ -635,6 +636,48 @@ declare_clippy_lint! {
635636
"checking for emptiness of a `Vec` in the loop condition and popping an element in the body"
636637
}
637638

639+
declare_clippy_lint! {
640+
/// ### What it does
641+
/// Checks for infinite loops in a function where the return type is not `!`
642+
/// and lint accordingly.
643+
///
644+
/// ### Why is this bad?
645+
/// A loop should be gently exited somewhere, or at least mark its parent function as
646+
/// never return (`!`).
647+
///
648+
/// ### Example
649+
/// ```no_run,ignore
650+
/// fn run_forever() {
651+
/// loop {
652+
/// // do something
653+
/// }
654+
/// }
655+
/// ```
656+
/// If infinite loops are as intended:
657+
/// ```no_run,ignore
658+
/// fn run_forever() -> ! {
659+
/// loop {
660+
/// // do something
661+
/// }
662+
/// }
663+
/// ```
664+
/// Otherwise add a `break` or `return` condition:
665+
/// ```no_run,ignore
666+
/// fn run_forever() {
667+
/// loop {
668+
/// // do something
669+
/// if condition {
670+
/// break;
671+
/// }
672+
/// }
673+
/// }
674+
/// ```
675+
#[clippy::version = "1.75.0"]
676+
pub INFINITE_LOOP,
677+
restriction,
678+
"possibly unintended infinite loop"
679+
}
680+
638681
pub struct Loops {
639682
msrv: Msrv,
640683
enforce_iter_loop_reborrow: bool,
@@ -669,6 +712,7 @@ impl_lint_pass!(Loops => [
669712
MANUAL_FIND,
670713
MANUAL_WHILE_LET_SOME,
671714
UNUSED_ENUMERATE_INDEX,
715+
INFINITE_LOOP,
672716
]);
673717

674718
impl<'tcx> LateLintPass<'tcx> for Loops {
@@ -707,10 +751,11 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
707751
// check for `loop { if let {} else break }` that could be `while let`
708752
// (also matches an explicit "match" instead of "if let")
709753
// (even if the "match" or "if let" is used for declaration)
710-
if let ExprKind::Loop(block, _, LoopSource::Loop, _) = expr.kind {
754+
if let ExprKind::Loop(block, label, LoopSource::Loop, _) = expr.kind {
711755
// also check for empty `loop {}` statements, skipping those in #[panic_handler]
712756
empty_loop::check(cx, expr, block);
713757
while_let_loop::check(cx, expr, block);
758+
infinite_loop::check(cx, expr, block, label);
714759
}
715760

716761
while_let_on_iterator::check(cx, expr);

0 commit comments

Comments
 (0)