diff options
| author | J-ZhengLi <lizheng135@huawei.com> | 2023-11-17 18:10:50 +0800 |
|---|---|---|
| committer | J-ZhengLi <lizheng135@huawei.com> | 2023-11-17 18:10:50 +0800 |
| commit | 2d9fc6dfc8160f47f13da9be3fb5fc48ef62880f (patch) | |
| tree | 3d57fc75f22459f2afdef2e2f4fc8ff65793ba90 /clippy_lints | |
| parent | 406d953820f48947216d6f3a31bef690d5798dca (diff) | |
| download | rust-2d9fc6dfc8160f47f13da9be3fb5fc48ef62880f.tar.gz rust-2d9fc6dfc8160f47f13da9be3fb5fc48ef62880f.zip | |
implement unoptimized code logic for [`infinite_loops`]
Diffstat (limited to 'clippy_lints')
| -rw-r--r-- | clippy_lints/src/declared_lints.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/loops/infinite_loops.rs | 92 | ||||
| -rw-r--r-- | clippy_lints/src/loops/mod.rs | 79 |
3 files changed, 164 insertions, 8 deletions
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 85854a0dfb7..7520b57891e 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -263,6 +263,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::loops::EXPLICIT_INTO_ITER_LOOP_INFO, crate::loops::EXPLICIT_ITER_LOOP_INFO, crate::loops::FOR_KV_MAP_INFO, + crate::loops::INFINITE_LOOPS_INFO, crate::loops::ITER_NEXT_LOOP_INFO, crate::loops::MANUAL_FIND_INFO, crate::loops::MANUAL_FLATTEN_INFO, diff --git a/clippy_lints/src/loops/infinite_loops.rs b/clippy_lints/src/loops/infinite_loops.rs new file mode 100644 index 00000000000..b97cd8bd006 --- /dev/null +++ b/clippy_lints/src/loops/infinite_loops.rs @@ -0,0 +1,92 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::is_lint_allowed; +use hir::intravisit::{walk_expr, Visitor}; +use hir::{Block, Destination, Expr, ExprKind, FnRetTy, Ty, TyKind}; +use rustc_ast::Label; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_lint::LateContext; + +use super::INFINITE_LOOPS; + +pub(super) fn check( + cx: &LateContext<'_>, + expr: &Expr<'_>, + loop_block: &Block<'_>, + label: Option<Label>, + parent_fn_ret_ty: FnRetTy<'_>, +) { + if is_lint_allowed(cx, INFINITE_LOOPS, expr.hir_id) + || matches!( + parent_fn_ret_ty, + FnRetTy::Return(Ty { + kind: TyKind::Never, + .. + }) + ) + { + return; + } + + // First, find any `break` or `return` without entering any inner loop, + // then, find `return` or labeled `break` which breaks this loop with entering inner loop, + // otherwise this loop is a infinite loop. + let mut direct_br_or_ret_finder = BreakOrRetFinder::default(); + direct_br_or_ret_finder.visit_block(loop_block); + + let is_finite_loop = direct_br_or_ret_finder.found || { + let mut inner_br_or_ret_finder = BreakOrRetFinder { + label, + enter_nested_loop: true, + ..Default::default() + }; + inner_br_or_ret_finder.visit_block(loop_block); + inner_br_or_ret_finder.found + }; + + if !is_finite_loop { + span_lint_and_then(cx, INFINITE_LOOPS, expr.span, "infinite loop detected", |diag| { + if let FnRetTy::DefaultReturn(ret_span) = parent_fn_ret_ty { + diag.span_suggestion( + ret_span, + "if this is intentional, consider specifing `!` as function return", + " -> !", + Applicability::MaybeIncorrect, + ); + } else { + diag.span_help( + expr.span, + "if this is not intended, add a `break` or `return` condition in this loop", + ); + } + }); + } +} + +#[derive(Default)] +struct BreakOrRetFinder { + label: Option<Label>, + found: bool, + enter_nested_loop: bool, +} + +impl<'hir> Visitor<'hir> for BreakOrRetFinder { + fn visit_expr(&mut self, ex: &'hir Expr<'_>) { + match &ex.kind { + ExprKind::Break(Destination { label, .. }, ..) => { + // When entering nested loop, only by breaking this loop's label + // would be considered as exiting this loop. + if self.enter_nested_loop { + if label.is_some() && *label == self.label { + self.found = true; + } + } else { + self.found = true; + } + }, + ExprKind::Ret(..) => self.found = true, + ExprKind::Loop(..) if !self.enter_nested_loop => (), + _ => walk_expr(self, ex), + } + } +} diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs index b99e0fd814f..c69d0b68cf4 100644 --- a/clippy_lints/src/loops/mod.rs +++ b/clippy_lints/src/loops/mod.rs @@ -3,6 +3,7 @@ mod explicit_counter_loop; mod explicit_into_iter_loop; mod explicit_iter_loop; mod for_kv_map; +mod infinite_loops; mod iter_next_loop; mod manual_find; mod manual_flatten; @@ -22,7 +23,7 @@ mod while_let_on_iterator; use clippy_config::msrvs::Msrv; use clippy_utils::higher; -use rustc_hir::{Expr, ExprKind, LoopSource, Pat}; +use rustc_hir::{self as hir, Expr, ExprKind, LoopSource, Pat}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Span; @@ -635,20 +636,64 @@ declare_clippy_lint! { "checking for emptiness of a `Vec` in the loop condition and popping an element in the body" } -pub struct Loops { +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 lease 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_LOOPS, + restriction, + "possibly unintended infinite loops" +} + +pub struct Loops<'tcx> { msrv: Msrv, enforce_iter_loop_reborrow: bool, + parent_fn_ret_ty: Option<hir::FnRetTy<'tcx>>, } -impl Loops { +impl<'tcx> Loops<'tcx> { pub fn new(msrv: Msrv, enforce_iter_loop_reborrow: bool) -> Self { Self { msrv, enforce_iter_loop_reborrow, + parent_fn_ret_ty: None, } } } -impl_lint_pass!(Loops => [ +impl_lint_pass!(Loops<'_> => [ MANUAL_MEMCPY, MANUAL_FLATTEN, NEEDLESS_RANGE_LOOP, @@ -669,9 +714,10 @@ impl_lint_pass!(Loops => [ MANUAL_FIND, MANUAL_WHILE_LET_SOME, UNUSED_ENUMERATE_INDEX, + INFINITE_LOOPS, ]); -impl<'tcx> LateLintPass<'tcx> for Loops { +impl<'tcx> LateLintPass<'tcx> for Loops<'tcx> { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { let for_loop = higher::ForLoop::hir(expr); if let Some(higher::ForLoop { @@ -707,10 +753,13 @@ impl<'tcx> LateLintPass<'tcx> for Loops { // check for `loop { if let {} else break }` that could be `while let` // (also matches an explicit "match" instead of "if let") // (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] empty_loop::check(cx, expr, block); while_let_loop::check(cx, expr, block); + if let Some(parent_fn_ret_ty) = self.parent_fn_ret_ty { + infinite_loops::check(cx, expr, block, label, parent_fn_ret_ty); + } } while_let_on_iterator::check(cx, expr); @@ -722,11 +771,25 @@ impl<'tcx> LateLintPass<'tcx> for Loops { } } + fn check_fn( + &mut self, + _: &LateContext<'tcx>, + kind: hir::intravisit::FnKind<'tcx>, + decl: &'tcx hir::FnDecl<'tcx>, + _: &'tcx hir::Body<'tcx>, + _: Span, + _: rustc_span::def_id::LocalDefId, + ) { + if let hir::intravisit::FnKind::ItemFn(..) = kind { + self.parent_fn_ret_ty = Some(decl.output); + } + } + extract_msrv_attr!(LateContext); } -impl Loops { - fn check_for_loop<'tcx>( +impl<'tcx> Loops<'tcx> { + fn check_for_loop( &self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, |
