mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

stop warning never-returning calls

Browse Source 
and add more test cases
This commit is contained in:
J-ZhengLi 2023-11-21 16:18:18 +08:00
parent 2d9fc6dfc8
commit 3e9a6d142e
4 changed files with 306 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

113
clippy_lints/src/loops/infinite_loops.rs
View File

@ -1,7 +1,7 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_lint_allowed;
use clippy_utils::{fn_def_id, is_lint_allowed};
use hir::intravisit::{walk_expr, Visitor};
use hir::{Block, Destination, Expr, ExprKind, FnRetTy, Ty, TyKind};
use hir::{Expr, ExprKind, FnRetTy, FnSig, Node};
use rustc_ast::Label;
use rustc_errors::Applicability;
use rustc_hir as hir;
@ -9,44 +9,56 @@
use super::INFINITE_LOOPS;
pub(super) fn check(
cx: &LateContext<'_>,
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &Expr<'_>,
loop_block: &Block<'_>,
loop_block: &'tcx hir::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,
..
})
)
{
if is_lint_allowed(cx, INFINITE_LOOPS, 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;
}
// 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 mut direct_visitor = LoopVisitor {
cx,
label,
is_finite: false,
enter_nested_loop: false,
};
direct_visitor.visit_block(loop_block);
let is_finite_loop = direct_br_or_ret_finder.found || {
let mut inner_br_or_ret_finder = BreakOrRetFinder {
let is_finite_loop = direct_visitor.is_finite || {
let mut inner_loop_visitor = LoopVisitor {
cx,
label,
is_finite: false,
enter_nested_loop: true,
..Default::default()
};
inner_br_or_ret_finder.visit_block(loop_block);
inner_br_or_ret_finder.found
inner_loop_visitor.visit_block(loop_block);
inner_loop_visitor.is_finite
};
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 {
if let FnRetTy::DefaultReturn(ret_span) = parent_fn_ret {
diag.span_suggestion(
ret_span,
"if this is intentional, consider specifing `!` as function return",
@ -56,37 +68,72 @@ pub(super) fn check(
} else {
diag.span_help(
expr.span,
"if this is not intended, add a `break` or `return` condition in this loop",
"if this is not intended, try adding a `break` or `return` condition in this loop",
);
}
});
}
}
#[derive(Default)]
struct BreakOrRetFinder {
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>,
found: bool,
is_finite: bool,
enter_nested_loop: bool,
}
impl<'hir> Visitor<'hir> for BreakOrRetFinder {
impl<'hir> Visitor<'hir> for LoopVisitor<'hir, '_> {
fn visit_expr(&mut self, ex: &'hir Expr<'_>) {
match &ex.kind {
ExprKind::Break(Destination { label, .. }, ..) => {
ExprKind::Break(hir::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;
self.is_finite = true;
}
} else {
self.found = true;
self.is_finite = true;
}
},
ExprKind::Ret(..) => self.found = true,
ExprKind::Ret(..) => self.is_finite = true,
ExprKind::Loop(..) if !self.enter_nested_loop => (),
_ => walk_expr(self, ex),
_ => {
// 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);
},
}
}
}

34
clippy_lints/src/loops/mod.rs
View File

@ -23,7 +23,7 @@
use clippy_config::msrvs::Msrv;
use clippy_utils::higher;
use rustc_hir::{self as hir, Expr, ExprKind, LoopSource, Pat};
use rustc_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;
@ -678,22 +678,20 @@
"possibly unintended infinite loops"
}
pub struct Loops<'tcx> {
pub struct Loops {
msrv: Msrv,
enforce_iter_loop_reborrow: bool,
parent_fn_ret_ty: Option<hir::FnRetTy<'tcx>>,
}
impl<'tcx> Loops<'tcx> {
impl Loops {
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,
@ -717,7 +715,7 @@ pub fn new(msrv: Msrv, enforce_iter_loop_reborrow: bool) -> Self {
INFINITE_LOOPS,
]);
impl<'tcx> LateLintPass<'tcx> for Loops<'tcx> {
impl<'tcx> LateLintPass<'tcx> for Loops {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
let for_loop = higher::ForLoop::hir(expr);
if let Some(higher::ForLoop {
@ -757,9 +755,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
// 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);
}
infinite_loops::check(cx, expr, block, label);
}
while_let_on_iterator::check(cx, expr);
@ -771,25 +767,11 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
}
}
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<'tcx> Loops<'tcx> {
fn check_for_loop(
impl Loops {
fn check_for_loop<'tcx>(
&self,
cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,

101
tests/ui/infinite_loops.rs
View File

@ -11,6 +11,31 @@ fn no_break() {
}
}
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();
@ -256,4 +281,80 @@ fn ret_in_macro(opt: Option<u8>) {
}
}
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 main() {}

126
tests/ui/infinite_loops.stderr
View File

@ -15,7 +15,73 @@ LL | fn no_break() -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:21:5
--> $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 this loop
--> $DIR/infinite_loops.rs:33:5
|
LL | / loop {
LL | |
LL | | do_something();
LL | | }
| |_____^
error: infinite loop detected
--> $DIR/infinite_loops.rs:46:5
|
LL | / loop {
LL | | fn inner_fn() -> ! {
@ -31,7 +97,7 @@ LL | fn no_break_never_ret_noise() -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:64:5
--> $DIR/infinite_loops.rs:89:5
|
LL | / loop {
LL | |
@ -48,7 +114,7 @@ LL | fn break_inner_but_not_outer_1(cond: bool) -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:75:5
--> $DIR/infinite_loops.rs:100:5
|
LL | / loop {
LL | |
@ -65,7 +131,7 @@ LL | fn break_inner_but_not_outer_2(cond: bool) -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:89:9
--> $DIR/infinite_loops.rs:114:9
|
LL | / loop {
LL | |
@ -79,7 +145,7 @@ LL | fn break_outer_but_not_inner() -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:112:9
--> $DIR/infinite_loops.rs:137:9
|
LL | / loop {
LL | |
@ -96,7 +162,7 @@ LL | fn break_wrong_loop(cond: bool) -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:152:5
--> $DIR/infinite_loops.rs:177:5
|
LL | / loop {
LL | |
@ -113,7 +179,7 @@ LL | fn match_like() -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:193:5
--> $DIR/infinite_loops.rs:218:5
|
LL | / loop {
LL | |
@ -127,7 +193,7 @@ LL | fn match_like() -> ! {
| ++++
error: infinite loop detected
--> $DIR/infinite_loops.rs:198:5
--> $DIR/infinite_loops.rs:223:5
|
LL | / loop {
LL | |
@ -143,5 +209,47 @@ help: if this is intentional, consider specifing `!` as function return
LL | fn match_like() -> ! {
| ++++
error: aborting due to 9 previous errors
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: aborting due to 16 previous errors