Skip to content

Commit fc78fde

Browse files
J-ZhengLiJ-ZhengLi
committed
[infinite_loop]: fix suggestion with unit ty return;
add more test cases with async clousures;
1 parent 3e95fa8 commit fc78fde

File tree

3 files changed

+160
-46
lines changed

3 files changed

+160
-46
lines changed

‎clippy_lints/src/loops/infinite_loop.rs

Lines changed: 49 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::ops::ControlFlow;
2+
13
use clippy_utils::diagnostics::span_lint_and_then;
24
use clippy_utils::{fn_def_id, is_from_proc_macro, is_lint_allowed};
35
use hir::intravisit::{walk_expr, Visitor};
@@ -42,10 +44,22 @@ pub(super) fn check<'tcx>(
4244
if !is_finite_loop {
4345
span_lint_and_then(cx, INFINITE_LOOP, expr.span, "infinite loop detected", |diag| {
4446
if let Some(span) = parent_fn_ret.sugg_span() {
47+
// Check if the type span is after a `->` or not.
48+
let suggestion = if cx
49+
.tcx
50+
.sess
51+
.source_map()
52+
.span_extend_to_prev_str(span, "->", false, true)
53+
.is_none()
54+
{
55+
" -> !"
56+
} else {
57+
"!"
58+
};
4559
diag.span_suggestion(
4660
span,
4761
"if this is intentional, consider specifying `!` as function return",
48-
" -> !",
62+
suggestion,
4963
Applicability::MaybeIncorrect,
5064
);
5165
} else {
@@ -141,35 +155,39 @@ impl<'hir> RetTy<'hir> {
141155
/// given `ty` is not an `OpaqueDef`.
142156
fn inner_<'tcx>(cx: &LateContext<'tcx>, ty: &Ty<'tcx>) -> Option<Ty<'tcx>> {
143157
/// Visitor to find the type binding.
144-
struct BindingVisitor<'tcx> {
145-
res: Option<Ty<'tcx>>,
146-
}
147-
impl<'tcx> Visitor<'tcx> for BindingVisitor<'tcx> {
148-
fn visit_assoc_type_binding(&mut self, type_binding: &'tcx hir::TypeBinding<'tcx>) {
149-
if self.res.is_some() {
150-
return;
151-
}
152-
if let hir::TypeBindingKind::Equality {
158+
struct BindingVisitor;
159+
160+
impl<'tcx> Visitor<'tcx> for BindingVisitor {
161+
type Result = ControlFlow<Ty<'tcx>>;
162+
163+
fn visit_assoc_item_constraint(
164+
&mut self,
165+
constraint: &'tcx hir::AssocItemConstraint<'tcx>,
166+
) -> Self::Result {
167+
if let hir::AssocItemConstraintKind::Equality {
153168
term: hir::Term::Ty(ty),
154-
} = type_binding.kind
169+
} = constraint.kind
155170
{
156-
self.res = Some(*ty);
171+
ControlFlow::Break(*ty)
172+
} else {
173+
ControlFlow::Continue(())
157174
}
158175
}
159176
}
160177

161-
let TyKind::OpaqueDef(item_id, ..) = ty.kind else {
162-
return None;
163-
};
164-
let opaque_ty_item = cx.tcx.hir().item(item_id);
165-
166-
// Sinces the `item_id` is from a `TyKind::OpaqueDef`,
167-
// therefore the `Item` related to it should always be `OpaqueTy`.
168-
assert!(matches!(opaque_ty_item.kind, ItemKind::OpaqueTy(_)));
169-
170-
let mut vis = BindingVisitor { res: None };
171-
vis.visit_item(opaque_ty_item);
172-
vis.res
178+
if let TyKind::OpaqueDef(item_id, ..) = ty.kind
179+
&& let opaque_ty_item = cx.tcx.hir().item(item_id)
180+
&& let ItemKind::OpaqueTy(_) = opaque_ty_item.kind
181+
{
182+
let mut vis = BindingVisitor;
183+
// Use `vis.break_value()` once it's stablized.
184+
match vis.visit_item(opaque_ty_item) {
185+
ControlFlow::Break(res) => Some(res),
186+
ControlFlow::Continue(()) => None,
187+
}
188+
} else {
189+
None
190+
}
173191
}
174192

175193
match fn_ret_ty {
@@ -186,7 +204,12 @@ impl<'hir> RetTy<'hir> {
186204
}
187205
}
188206
fn is_never(&self) -> bool {
189-
let Self::Return(ty) = self else { return false };
190-
matches!(ty.kind, TyKind::Never)
207+
matches!(
208+
self,
209+
RetTy::Return(Ty {
210+
kind: TyKind::Never,
211+
..
212+
})
213+
)
191214
}
192215
}

‎tests/ui/infinite_loops.rs

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
//@no-rustfix
22
//@aux-build:proc_macros.rs
33

4-
#![allow(clippy::never_loop)]
4+
#![allow(clippy::never_loop, clippy::unused_unit)]
55
#![warn(clippy::infinite_loop)]
6+
#![feature(async_closure)]
67

78
extern crate proc_macros;
89
use proc_macros::{external, with_span};
@@ -390,6 +391,20 @@ fn span_inside_fn() {
390391
}
391392
}
392393

394+
fn return_unit() -> () {
395+
loop {
396+
//~^ ERROR: infinite loop detected
397+
do_nothing();
398+
}
399+
400+
let async_return_unit = || -> () {
401+
loop {
402+
//~^ ERROR: infinite loop detected
403+
do_nothing();
404+
}
405+
};
406+
}
407+
393408
// loop in async functions
394409
mod issue_12338 {
395410
use super::do_something;
@@ -405,6 +420,26 @@ mod issue_12338 {
405420
do_something();
406421
}
407422
}
423+
// Just to make sure checks for async block works as intended.
424+
fn async_closure() {
425+
let _loop_forever = async || {
426+
loop {
427+
//~^ ERROR: infinite loop detected
428+
do_something();
429+
}
430+
};
431+
let _somehow_ok = async || -> ! {
432+
loop {
433+
do_something();
434+
}
435+
};
436+
let _ret_unit = async || -> () {
437+
loop {
438+
//~^ ERROR: infinite loop detected
439+
do_something();
440+
}
441+
};
442+
}
408443
}
409444

410445
fn main() {}

‎tests/ui/infinite_loops.stderr

Lines changed: 75 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: infinite loop detected
2-
--> tests/ui/infinite_loops.rs:13:5
2+
--> tests/ui/infinite_loops.rs:14:5
33
|
44
LL | / loop {
55
LL | |
@@ -15,7 +15,7 @@ LL | fn no_break() -> ! {
1515
| ++++
1616

1717
error: infinite loop detected
18-
--> tests/ui/infinite_loops.rs:20:5
18+
--> tests/ui/infinite_loops.rs:21:5
1919
|
2020
LL | / loop {
2121
LL | |
@@ -32,7 +32,7 @@ LL | fn all_inf() -> ! {
3232
| ++++
3333

3434
error: infinite loop detected
35-
--> tests/ui/infinite_loops.rs:22:9
35+
--> tests/ui/infinite_loops.rs:23:9
3636
|
3737
LL | / loop {
3838
LL | |
@@ -49,7 +49,7 @@ LL | fn all_inf() -> ! {
4949
| ++++
5050

5151
error: infinite loop detected
52-
--> tests/ui/infinite_loops.rs:24:13
52+
--> tests/ui/infinite_loops.rs:25:13
5353
|
5454
LL | / loop {
5555
LL | |
@@ -63,7 +63,7 @@ LL | fn all_inf() -> ! {
6363
| ++++
6464

6565
error: infinite loop detected
66-
--> tests/ui/infinite_loops.rs:38:5
66+
--> tests/ui/infinite_loops.rs:39:5
6767
|
6868
LL | / loop {
6969
LL | |
@@ -74,7 +74,7 @@ LL | | }
7474
= help: if this is not intended, try adding a `break` or `return` condition in the loop
7575

7676
error: infinite loop detected
77-
--> tests/ui/infinite_loops.rs:51:5
77+
--> tests/ui/infinite_loops.rs:52:5
7878
|
7979
LL | / loop {
8080
LL | | fn inner_fn() -> ! {
@@ -90,7 +90,7 @@ LL | fn no_break_never_ret_noise() -> ! {
9090
| ++++
9191

9292
error: infinite loop detected
93-
--> tests/ui/infinite_loops.rs:94:5
93+
--> tests/ui/infinite_loops.rs:95:5
9494
|
9595
LL | / loop {
9696
LL | |
@@ -107,7 +107,7 @@ LL | fn break_inner_but_not_outer_1(cond: bool) -> ! {
107107
| ++++
108108

109109
error: infinite loop detected
110-
--> tests/ui/infinite_loops.rs:105:5
110+
--> tests/ui/infinite_loops.rs:106:5
111111
|
112112
LL | / loop {
113113
LL | |
@@ -124,7 +124,7 @@ LL | fn break_inner_but_not_outer_2(cond: bool) -> ! {
124124
| ++++
125125

126126
error: infinite loop detected
127-
--> tests/ui/infinite_loops.rs:119:9
127+
--> tests/ui/infinite_loops.rs:120:9
128128
|
129129
LL | / loop {
130130
LL | |
@@ -138,7 +138,7 @@ LL | fn break_outer_but_not_inner() -> ! {
138138
| ++++
139139

140140
error: infinite loop detected
141-
--> tests/ui/infinite_loops.rs:142:9
141+
--> tests/ui/infinite_loops.rs:143:9
142142
|
143143
LL | / loop {
144144
LL | |
@@ -155,7 +155,7 @@ LL | fn break_wrong_loop(cond: bool) -> ! {
155155
| ++++
156156

157157
error: infinite loop detected
158-
--> tests/ui/infinite_loops.rs:182:5
158+
--> tests/ui/infinite_loops.rs:183:5
159159
|
160160
LL | / loop {
161161
LL | |
@@ -172,7 +172,7 @@ LL | fn match_like() -> ! {
172172
| ++++
173173

174174
error: infinite loop detected
175-
--> tests/ui/infinite_loops.rs:223:5
175+
--> tests/ui/infinite_loops.rs:224:5
176176
|
177177
LL | / loop {
178178
LL | |
@@ -186,7 +186,7 @@ LL | fn match_like() -> ! {
186186
| ++++
187187

188188
error: infinite loop detected
189-
--> tests/ui/infinite_loops.rs:228:5
189+
--> tests/ui/infinite_loops.rs:229:5
190190
|
191191
LL | / loop {
192192
LL | |
@@ -203,7 +203,7 @@ LL | fn match_like() -> ! {
203203
| ++++
204204

205205
error: infinite loop detected
206-
--> tests/ui/infinite_loops.rs:333:9
206+
--> tests/ui/infinite_loops.rs:334:9
207207
|
208208
LL | / loop {
209209
LL | |
@@ -217,7 +217,7 @@ LL | fn problematic_trait_method() -> ! {
217217
| ++++
218218

219219
error: infinite loop detected
220-
--> tests/ui/infinite_loops.rs:343:9
220+
--> tests/ui/infinite_loops.rs:344:9
221221
|
222222
LL | / loop {
223223
LL | |
@@ -231,7 +231,7 @@ LL | fn could_be_problematic() -> ! {
231231
| ++++
232232

233233
error: infinite loop detected
234-
--> tests/ui/infinite_loops.rs:352:9
234+
--> tests/ui/infinite_loops.rs:353:9
235235
|
236236
LL | / loop {
237237
LL | |
@@ -245,7 +245,7 @@ LL | let _loop_forever = || -> ! {
245245
| ++++
246246

247247
error: infinite loop detected
248-
--> tests/ui/infinite_loops.rs:366:8
248+
--> tests/ui/infinite_loops.rs:367:8
249249
|
250250
LL | Ok(loop {
251251
| ________^
@@ -256,7 +256,35 @@ LL | | })
256256
= help: if this is not intended, try adding a `break` or `return` condition in the loop
257257

258258
error: infinite loop detected
259-
--> tests/ui/infinite_loops.rs:403:9
259+
--> tests/ui/infinite_loops.rs:395:5
260+
|
261+
LL | / loop {
262+
LL | |
263+
LL | | do_nothing();
264+
LL | | }
265+
| |_____^
266+
|
267+
help: if this is intentional, consider specifying `!` as function return
268+
|
269+
LL | fn return_unit() -> ! {
270+
| ~
271+
272+
error: infinite loop detected
273+
--> tests/ui/infinite_loops.rs:401:9
274+
|
275+
LL | / loop {
276+
LL | |
277+
LL | | do_nothing();
278+
LL | | }
279+
| |_________^
280+
|
281+
help: if this is intentional, consider specifying `!` as function return
282+
|
283+
LL | let async_return_unit = || -> ! {
284+
| ~
285+
286+
error: infinite loop detected
287+
--> tests/ui/infinite_loops.rs:418:9
260288
|
261289
LL | / loop {
262290
LL | |
@@ -269,5 +297,33 @@ help: if this is intentional, consider specifying `!` as function return
269297
LL | async fn bar() -> ! {
270298
| ++++
271299

272-
error: aborting due to 18 previous errors
300+
error: infinite loop detected
301+
--> tests/ui/infinite_loops.rs:426:13
302+
|
303+
LL | / loop {
304+
LL | |
305+
LL | | do_something();
306+
LL | | }
307+
| |_____________^
308+
|
309+
help: if this is intentional, consider specifying `!` as function return
310+
|
311+
LL | let _loop_forever = async || -> ! {
312+
| ++++
313+
314+
error: infinite loop detected
315+
--> tests/ui/infinite_loops.rs:437:13
316+
|
317+
LL | / loop {
318+
LL | |
319+
LL | | do_something();
320+
LL | | }
321+
| |_____________^
322+
|
323+
help: if this is intentional, consider specifying `!` as function return
324+
|
325+
LL | let _ret_unit = async || -> ! {
326+
| ~
327+
328+
error: aborting due to 22 previous errors
273329

0 commit comments

Comments
 (0)