Skip to content

Commit e582fcd

Browse files
lengyijunprofetia
lengyijun
authored and
profetia
committed
[needless_continue]: lint if the last stmt in for/while/loop is continue, recursively
fixes: #4077
1 parent 998c780 commit e582fcd

File tree

7 files changed

+172
-36
lines changed

7 files changed

+172
-36
lines changed

‎clippy_lints/src/methods/unnecessary_to_owned.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ fn can_change_type<'a>(cx: &LateContext<'a>, mut expr: &'a Expr<'a>, mut ty: Ty<
494494
for (_, node) in cx.tcx.hir().parent_iter(expr.hir_id) {
495495
match node {
496496
Node::Stmt(_) => return true,
497-
Node::Block(..) => continue,
497+
Node::Block(..) => {},
498498
Node::Item(item) => {
499499
if let ItemKind::Fn(_, _, body_id) = &item.kind
500500
&& let output_ty = return_ty(cx, item.owner_id)

‎clippy_lints/src/needless_continue.rs

Lines changed: 100 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
22
use clippy_utils::source::{indent_of, snippet, snippet_block};
3-
use rustc_ast::ast;
3+
use rustc_ast::{Block, Label, ast};
44
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
55
use rustc_session::declare_lint_pass;
66
use rustc_span::Span;
@@ -11,6 +11,7 @@ declare_clippy_lint! {
1111
/// that contain a `continue` statement in either their main blocks or their
1212
/// `else`-blocks, when omitting the `else`-block possibly with some
1313
/// rearrangement of code can make the code easier to understand.
14+
/// The lint also checks if the last statement in the loop is a `continue`
1415
///
1516
/// ### Why is this bad?
1617
/// Having explicit `else` blocks for `if` statements
@@ -75,6 +76,45 @@ declare_clippy_lint! {
7576
/// # break;
7677
/// }
7778
/// ```
79+
///
80+
/// ```rust
81+
/// fn foo() -> std::io::ErrorKind { std::io::ErrorKind::NotFound }
82+
/// for _ in 0..10 {
83+
/// match foo() {
84+
/// std::io::ErrorKind::NotFound => {
85+
/// eprintln!("not found");
86+
/// continue
87+
/// }
88+
/// std::io::ErrorKind::TimedOut => {
89+
/// eprintln!("timeout");
90+
/// continue
91+
/// }
92+
/// _ => {
93+
/// eprintln!("other error");
94+
/// continue
95+
/// }
96+
/// }
97+
/// }
98+
/// ```
99+
/// Could be rewritten as
100+
///
101+
///
102+
/// ```rust
103+
/// fn foo() -> std::io::ErrorKind { std::io::ErrorKind::NotFound }
104+
/// for _ in 0..10 {
105+
/// match foo() {
106+
/// std::io::ErrorKind::NotFound => {
107+
/// eprintln!("not found");
108+
/// }
109+
/// std::io::ErrorKind::TimedOut => {
110+
/// eprintln!("timeout");
111+
/// }
112+
/// _ => {
113+
/// eprintln!("other error");
114+
/// }
115+
/// }
116+
/// }
117+
/// ```
78118
#[clippy::version = "pre 1.29.0"]
79119
pub NEEDLESS_CONTINUE,
80120
pedantic,
@@ -144,15 +184,15 @@ impl EarlyLintPass for NeedlessContinue {
144184
///
145185
/// - The expression is a `continue` node.
146186
/// - The expression node is a block with the first statement being a `continue`.
147-
fn needless_continue_in_else(else_expr: &ast::Expr, label: Option<&ast::Label>) -> bool {
187+
fn needless_continue_in_else(else_expr: &ast::Expr, label: Option<&Label>) -> bool {
148188
match else_expr.kind {
149189
ast::ExprKind::Block(ref else_block, _) => is_first_block_stmt_continue(else_block, label),
150190
ast::ExprKind::Continue(l) => compare_labels(label, l.as_ref()),
151191
_ => false,
152192
}
153193
}
154194

155-
fn is_first_block_stmt_continue(block: &ast::Block, label: Option<&ast::Label>) -> bool {
195+
fn is_first_block_stmt_continue(block: &Block, label: Option<&Label>) -> bool {
156196
block.stmts.first().is_some_and(|stmt| match stmt.kind {
157197
ast::StmtKind::Semi(ref e) | ast::StmtKind::Expr(ref e) => {
158198
if let ast::ExprKind::Continue(ref l) = e.kind {
@@ -166,7 +206,7 @@ fn is_first_block_stmt_continue(block: &ast::Block, label: Option<&ast::Label>)
166206
}
167207

168208
/// If the `continue` has a label, check it matches the label of the loop.
169-
fn compare_labels(loop_label: Option<&ast::Label>, continue_label: Option<&ast::Label>) -> bool {
209+
fn compare_labels(loop_label: Option<&Label>, continue_label: Option<&Label>) -> bool {
170210
match (loop_label, continue_label) {
171211
// `loop { continue; }` or `'a loop { continue; }`
172212
(_, None) => true,
@@ -181,7 +221,7 @@ fn compare_labels(loop_label: Option<&ast::Label>, continue_label: Option<&ast::
181221
/// the AST object representing the loop block of `expr`.
182222
fn with_loop_block<F>(expr: &ast::Expr, mut func: F)
183223
where
184-
F: FnMut(&ast::Block, Option<&ast::Label>),
224+
F: FnMut(&Block, Option<&Label>),
185225
{
186226
if let ast::ExprKind::While(_, loop_block, label)
187227
| ast::ExprKind::ForLoop {
@@ -205,7 +245,7 @@ where
205245
/// - The `else` expression.
206246
fn with_if_expr<F>(stmt: &ast::Stmt, mut func: F)
207247
where
208-
F: FnMut(&ast::Expr, &ast::Expr, &ast::Block, &ast::Expr),
248+
F: FnMut(&ast::Expr, &ast::Expr, &Block, &ast::Expr),
209249
{
210250
match stmt.kind {
211251
ast::StmtKind::Semi(ref e) | ast::StmtKind::Expr(ref e) => {
@@ -231,14 +271,14 @@ struct LintData<'a> {
231271
/// The condition expression for the above `if`.
232272
if_cond: &'a ast::Expr,
233273
/// The `then` block of the `if` statement.
234-
if_block: &'a ast::Block,
274+
if_block: &'a Block,
235275
/// The `else` block of the `if` statement.
236276
/// Note that we only work with `if` exprs that have an `else` branch.
237277
else_expr: &'a ast::Expr,
238278
/// The 0-based index of the `if` statement in the containing loop block.
239279
stmt_idx: usize,
240280
/// The statements of the loop block.
241-
loop_block: &'a ast::Block,
281+
loop_block: &'a Block,
242282
}
243283

244284
const MSG_REDUNDANT_CONTINUE_EXPRESSION: &str = "this `continue` expression is redundant";
@@ -329,23 +369,61 @@ fn suggestion_snippet_for_continue_inside_else(cx: &EarlyContext<'_>, data: &Lin
329369
)
330370
}
331371

332-
fn check_and_warn(cx: &EarlyContext<'_>, expr: &ast::Expr) {
333-
if let ast::ExprKind::Loop(loop_block, loop_label, ..) = &expr.kind
334-
&& let Some(last_stmt) = loop_block.stmts.last()
372+
fn check_last_stmt_in_expr<F>(inner_expr: &ast::Expr, func: &F)
373+
where
374+
F: Fn(Option<&Label>, Span),
375+
{
376+
match &inner_expr.kind {
377+
ast::ExprKind::Continue(continue_label) => {
378+
func(continue_label.as_ref(), inner_expr.span);
379+
},
380+
ast::ExprKind::If(_, then_block, else_block) => {
381+
check_last_stmt_in_block(then_block, func);
382+
if let Some(else_block) = else_block {
383+
check_last_stmt_in_expr(else_block, func);
384+
}
385+
},
386+
ast::ExprKind::Match(_, arms, _) => {
387+
for arm in arms {
388+
if let Some(expr) = &arm.body {
389+
check_last_stmt_in_expr(expr, func);
390+
}
391+
}
392+
},
393+
ast::ExprKind::Block(b, _) => {
394+
check_last_stmt_in_block(b, func);
395+
},
396+
_ => {},
397+
}
398+
}
399+
400+
fn check_last_stmt_in_block<F>(b: &Block, func: &F)
401+
where
402+
F: Fn(Option<&Label>, Span),
403+
{
404+
if let Some(last_stmt) = b.stmts.last()
335405
&& let ast::StmtKind::Expr(inner_expr) | ast::StmtKind::Semi(inner_expr) = &last_stmt.kind
336-
&& let ast::ExprKind::Continue(continue_label) = inner_expr.kind
337-
&& compare_labels(loop_label.as_ref(), continue_label.as_ref())
338406
{
339-
span_lint_and_help(
340-
cx,
341-
NEEDLESS_CONTINUE,
342-
last_stmt.span,
343-
MSG_REDUNDANT_CONTINUE_EXPRESSION,
344-
None,
345-
DROP_CONTINUE_EXPRESSION_MSG,
346-
);
407+
check_last_stmt_in_expr(inner_expr, func);
347408
}
409+
}
410+
411+
fn check_and_warn(cx: &EarlyContext<'_>, expr: &ast::Expr) {
348412
with_loop_block(expr, |loop_block, label| {
413+
let p = |continue_label: Option<&Label>, span: Span| {
414+
if compare_labels(label, continue_label) {
415+
span_lint_and_help(
416+
cx,
417+
NEEDLESS_CONTINUE,
418+
span,
419+
MSG_REDUNDANT_CONTINUE_EXPRESSION,
420+
None,
421+
DROP_CONTINUE_EXPRESSION_MSG,
422+
);
423+
}
424+
};
425+
check_last_stmt_in_block(loop_block, &p);
426+
349427
for (i, stmt) in loop_block.stmts.iter().enumerate() {
350428
with_if_expr(stmt, |if_expr, cond, then_block, else_expr| {
351429
let data = &LintData {
@@ -400,7 +478,7 @@ fn erode_from_back(s: &str) -> String {
400478
if ret.is_empty() { s.to_string() } else { ret }
401479
}
402480

403-
fn span_of_first_expr_in_block(block: &ast::Block) -> Option<Span> {
481+
fn span_of_first_expr_in_block(block: &Block) -> Option<Span> {
404482
block.stmts.first().map(|stmt| stmt.span)
405483
}
406484

‎clippy_lints/src/redundant_else.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ impl EarlyLintPass for RedundantElse {
6969
ExprKind::If(_, next_then, Some(next_els)) => {
7070
then = next_then;
7171
els = next_els;
72-
continue;
7372
},
7473
// else if without else
7574
ExprKind::If(..) => return,

‎clippy_lints/src/transmute/transmute_undefined_repr.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,14 @@ pub(super) fn check<'tcx>(
3030
| (ReducedTy::UnorderedFields(from_sub_ty), ReducedTy::OrderedFields(Some(to_sub_ty))) => {
3131
from_ty = from_sub_ty;
3232
to_ty = to_sub_ty;
33-
continue;
3433
},
3534
(ReducedTy::OrderedFields(Some(from_sub_ty)), ReducedTy::Other(to_sub_ty)) if reduced_tys.to_fat_ptr => {
3635
from_ty = from_sub_ty;
3736
to_ty = to_sub_ty;
38-
continue;
3937
},
4038
(ReducedTy::Other(from_sub_ty), ReducedTy::OrderedFields(Some(to_sub_ty))) if reduced_tys.from_fat_ptr => {
4139
from_ty = from_sub_ty;
4240
to_ty = to_sub_ty;
43-
continue;
4441
},
4542

4643
// ptr <-> ptr
@@ -50,7 +47,6 @@ pub(super) fn check<'tcx>(
5047
{
5148
from_ty = from_sub_ty;
5249
to_ty = to_sub_ty;
53-
continue;
5450
},
5551

5652
// fat ptr <-> (*size, *size)

‎tests/missing-test-files.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ fn explore_directory(dir: &Path) -> Vec<String> {
5959
missing_files.push(path.to_str().unwrap().to_string());
6060
}
6161
},
62-
_ => continue,
62+
_ => {},
6363
};
6464
}
6565
}

‎tests/ui/needless_continue.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,14 @@ fn simple_loop4() {
8787
}
8888
}
8989

90+
fn simple_loop5() {
91+
loop {
92+
println!("bleh");
93+
{ continue }
94+
//~^ ERROR: this `continue` expression is redundant
95+
}
96+
}
97+
9098
mod issue_2329 {
9199
fn condition() -> bool {
92100
unimplemented!()
@@ -168,3 +176,34 @@ fn issue_13641() {
168176
}
169177
}
170178
}
179+
180+
mod issue_4077 {
181+
fn main() {
182+
'outer: loop {
183+
'inner: loop {
184+
do_something();
185+
if some_expr() {
186+
println!("bar-7");
187+
continue 'outer;
188+
} else if !some_expr() {
189+
println!("bar-8");
190+
continue 'inner;
191+
} else {
192+
println!("bar-9");
193+
continue 'inner;
194+
}
195+
}
196+
}
197+
}
198+
199+
// The contents of these functions are irrelevant, the purpose of this file is
200+
// shown in main.
201+
202+
fn do_something() {
203+
std::process::exit(0);
204+
}
205+
206+
fn some_expr() -> bool {
207+
true
208+
}
209+
}

‎tests/ui/needless_continue.stderr

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,15 @@ error: this `continue` expression is redundant
6363
--> tests/ui/needless_continue.rs:60:9
6464
|
6565
LL | continue;
66-
| ^^^^^^^^^
66+
| ^^^^^^^^
6767
|
6868
= help: consider dropping the `continue` expression
6969

7070
error: this `continue` expression is redundant
7171
--> tests/ui/needless_continue.rs:68:9
7272
|
7373
LL | continue;
74-
| ^^^^^^^^^
74+
| ^^^^^^^^
7575
|
7676
= help: consider dropping the `continue` expression
7777

@@ -91,8 +91,16 @@ LL | continue
9191
|
9292
= help: consider dropping the `continue` expression
9393

94+
error: this `continue` expression is redundant
95+
--> tests/ui/needless_continue.rs:93:11
96+
|
97+
LL | { continue }
98+
| ^^^^^^^^
99+
|
100+
= help: consider dropping the `continue` expression
101+
94102
error: this `else` block is redundant
95-
--> tests/ui/needless_continue.rs:136:24
103+
--> tests/ui/needless_continue.rs:144:24
96104
|
97105
LL | } else {
98106
| ________________________^
@@ -117,7 +125,7 @@ LL | | }
117125
}
118126

119127
error: there is no need for an explicit `else` block for this `if` expression
120-
--> tests/ui/needless_continue.rs:143:17
128+
--> tests/ui/needless_continue.rs:151:17
121129
|
122130
LL | / if condition() {
123131
LL | |
@@ -137,12 +145,28 @@ LL | | }
137145
}
138146

139147
error: this `continue` expression is redundant
140-
--> tests/ui/needless_continue.rs:166:13
148+
--> tests/ui/needless_continue.rs:174:13
141149
|
142150
LL | continue 'b;
143-
| ^^^^^^^^^^^^
151+
| ^^^^^^^^^^^
152+
|
153+
= help: consider dropping the `continue` expression
154+
155+
error: this `continue` expression is redundant
156+
--> tests/ui/needless_continue.rs:190:21
157+
|
158+
LL | continue 'inner;
159+
| ^^^^^^^^^^^^^^^
160+
|
161+
= help: consider dropping the `continue` expression
162+
163+
error: this `continue` expression is redundant
164+
--> tests/ui/needless_continue.rs:193:21
165+
|
166+
LL | continue 'inner;
167+
| ^^^^^^^^^^^^^^^
144168
|
145169
= help: consider dropping the `continue` expression
146170

147-
error: aborting due to 9 previous errors
171+
error: aborting due to 12 previous errors
148172

0 commit comments

Comments
 (0)