Skip to content

Commit

Permalink
[needless_continue]: lint if the last stmt in for/while/loop is `co…
Browse files Browse the repository at this point in the history
…ntinue`, recursively

fixes: #4077
  • Loading branch information
lengyijun committed Sep 25, 2023
1 parent aa137a7 commit d890421
Show file tree
Hide file tree
Showing 7 changed files with 179 additions and 26 deletions.
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/unnecessary_to_owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ fn can_change_type<'a>(cx: &LateContext<'a>, mut expr: &'a Expr<'a>, mut ty: Ty<
for (_, node) in cx.tcx.hir().parent_iter(expr.hir_id) {
match node {
Node::Stmt(_) => return true,
Node::Block(..) => continue,
Node::Block(..) => {}
Node::Item(item) => {
if let ItemKind::Fn(_, _, body_id) = &item.kind
&& let output_ty = return_ty(cx, item.owner_id)
Expand Down
127 changes: 111 additions & 16 deletions clippy_lints/src/needless_continue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
//! This lint is **warn** by default.
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::source::{indent_of, snippet, snippet_block};
use rustc_ast::ast;
use rustc_ast::{ast, Block, Label};
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::Span;
Expand All @@ -46,6 +46,7 @@ declare_clippy_lint! {
/// that contain a `continue` statement in either their main blocks or their
/// `else`-blocks, when omitting the `else`-block possibly with some
/// rearrangement of code can make the code easier to understand.
/// The lint also checks if the last statement in the loop is a `continue`
///
/// ### Why is this bad?
/// Having explicit `else` blocks for `if` statements
Expand Down Expand Up @@ -110,6 +111,45 @@ declare_clippy_lint! {
/// # break;
/// }
/// ```
///
/// ```rust
/// fn foo() -> std::io::ErrorKind { std::io::ErrorKind::NotFound }
/// for _ in 0..10 {
/// match foo() {
/// std::io::ErrorKind::NotFound => {
/// eprintln!("not found");
/// continue
/// }
/// std::io::ErrorKind::TimedOut => {
/// eprintln!("timeout");
/// continue
/// }
/// _ => {
/// eprintln!("other error");
/// continue
/// }
/// }
/// }
/// ```
/// Could be rewritten as
///
///
/// ```rust
/// fn foo() -> std::io::ErrorKind { std::io::ErrorKind::NotFound }
/// for _ in 0..10 {
/// match foo() {
/// std::io::ErrorKind::NotFound => {
/// eprintln!("not found");
/// }
/// std::io::ErrorKind::TimedOut => {
/// eprintln!("timeout");
/// }
/// _ => {
/// eprintln!("other error");
/// }
/// }
/// }
/// ```
#[clippy::version = "pre 1.29.0"]
pub NEEDLESS_CONTINUE,
pedantic,
Expand Down Expand Up @@ -361,24 +401,79 @@ fn suggestion_snippet_for_continue_inside_else(cx: &EarlyContext<'_>, data: &Lin
)
}

fn check_and_warn(cx: &EarlyContext<'_>, expr: &ast::Expr) {
if_chain! {
if let ast::ExprKind::Loop(loop_block, ..) = &expr.kind;
if let Some(last_stmt) = loop_block.stmts.last();
if let ast::StmtKind::Expr(inner_expr) | ast::StmtKind::Semi(inner_expr) = &last_stmt.kind;
if let ast::ExprKind::Continue(_) = inner_expr.kind;
then {
span_lint_and_help(
cx,
NEEDLESS_CONTINUE,
last_stmt.span,
MSG_REDUNDANT_CONTINUE_EXPRESSION,
None,
DROP_CONTINUE_EXPRESSION_MSG,
);
fn check_last_stmt_in_if<F>(b: &ast::Expr, func: &F)
where
F: Fn(Option<&ast::Label>, Span),
{
match &b.kind {
ast::ExprKind::If(_, then_block, else_block) => {
check_last_stmt(then_block, func);
if let Some(else_block) = else_block {
check_last_stmt_in_if(else_block, func);
}
},
ast::ExprKind::Continue(..) => {
unreachable!()
},
ast::ExprKind::Block(b, _) => {
check_last_stmt(b, func);
},
_ => {},
}
}

fn check_last_stmt<F>(b: &Block, func: &F)
where
F: Fn(Option<&ast::Label>, Span),
{
if let Some(last_stmt) = b.stmts.last() &&
let ast::StmtKind::Expr(inner_expr) | ast::StmtKind::Semi(inner_expr) = &last_stmt.kind {
match &inner_expr.kind {
ast::ExprKind::Continue(continue_label) => {
func(continue_label.as_ref(), last_stmt.span);
},
ast::ExprKind::If(_, _, _) => {
check_last_stmt_in_if(inner_expr, func);
}
ast::ExprKind::Match(_, arms) => {
for arm in arms {
match &arm.body.kind {
ast::ExprKind::Continue(continue_label) => {
func(continue_label.as_ref(), arm.body.span);
}
ast::ExprKind::Block(b, _) => {
check_last_stmt(b, func);

}
_ => {}
}

}
}
ast::ExprKind::Block(b, _) => {
check_last_stmt(b, func);
}
_ => {},
}
}
}

fn check_and_warn(cx: &EarlyContext<'_>, expr: &ast::Expr) {
with_loop_block(expr, |loop_block, label| {
let p = |continue_label: Option<&Label>, span: Span| {
if compare_labels(label, continue_label) {
span_lint_and_help(
cx,
NEEDLESS_CONTINUE,
span,
MSG_REDUNDANT_CONTINUE_EXPRESSION,
None,
DROP_CONTINUE_EXPRESSION_MSG,
);
}
};
check_last_stmt(loop_block, &p);

for (i, stmt) in loop_block.stmts.iter().enumerate() {
with_if_expr(stmt, |if_expr, cond, then_block, else_expr| {
let data = &LintData {
Expand Down
1 change: 0 additions & 1 deletion clippy_lints/src/redundant_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ impl EarlyLintPass for RedundantElse {
ExprKind::If(_, next_then, Some(next_els)) => {
then = next_then;
els = next_els;
continue;
},
// else if without else
ExprKind::If(..) => return,
Expand Down
4 changes: 0 additions & 4 deletions clippy_lints/src/transmute/transmute_undefined_repr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,16 @@ pub(super) fn check<'tcx>(
| (ReducedTy::UnorderedFields(from_sub_ty), ReducedTy::OrderedFields(_, Some(to_sub_ty))) => {
from_ty = from_sub_ty;
to_ty = to_sub_ty;
continue;
},
(ReducedTy::OrderedFields(_, Some(from_sub_ty)), ReducedTy::Other(to_sub_ty)) if reduced_tys.to_fat_ptr => {
from_ty = from_sub_ty;
to_ty = to_sub_ty;
continue;
},
(ReducedTy::Other(from_sub_ty), ReducedTy::OrderedFields(_, Some(to_sub_ty)))
if reduced_tys.from_fat_ptr =>
{
from_ty = from_sub_ty;
to_ty = to_sub_ty;
continue;
},

// ptr <-> ptr
Expand All @@ -52,7 +49,6 @@ pub(super) fn check<'tcx>(
{
from_ty = from_sub_ty;
to_ty = to_sub_ty;
continue;
},

// fat ptr <-> (*size, *size)
Expand Down
2 changes: 1 addition & 1 deletion tests/missing-test-files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ fn explore_directory(dir: &Path) -> Vec<String> {
missing_files.push(path.to_str().unwrap().to_string());
}
},
_ => continue,
_ => {},
};
}
}
Expand Down
39 changes: 39 additions & 0 deletions tests/ui/needless_continue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,14 @@ fn simple_loop4() {
}
}

fn simple_loop5() {
loop {
println!("bleh");
{ continue }
//~^ ERROR: this `continue` expression is redundant
}
}

mod issue_2329 {
fn condition() -> bool {
unimplemented!()
Expand Down Expand Up @@ -151,3 +159,34 @@ mod issue_2329 {
}
}
}

mod issue_4077 {
fn main() {
'outer: loop {
'inner: loop {
do_something();
if some_expr() {
println!("bar-7");
continue 'outer;
} else if !some_expr() {
println!("bar-8");
continue 'inner;
} else {
println!("bar-9");
continue 'inner;
}
}
}
}

// The contents of these functions are irrelevant, the purpose of this file is
// shown in main.

fn do_something() {
std::process::exit(0);
}

fn some_expr() -> bool {
true
}
}
30 changes: 27 additions & 3 deletions tests/ui/needless_continue.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,16 @@ LL | continue
|
= help: consider dropping the `continue` expression

error: this `continue` expression is redundant
--> $DIR/needless_continue.rs:93:11
|
LL | { continue }
| ^^^^^^^^
|
= help: consider dropping the `continue` expression

error: this `else` block is redundant
--> $DIR/needless_continue.rs:136:24
--> $DIR/needless_continue.rs:144:24
|
LL | } else {
| ________________________^
Expand All @@ -117,7 +125,7 @@ LL | | }
}

error: there is no need for an explicit `else` block for this `if` expression
--> $DIR/needless_continue.rs:143:17
--> $DIR/needless_continue.rs:151:17
|
LL | / if condition() {
LL | |
Expand All @@ -136,5 +144,21 @@ LL | | }
println!("bar-5");
}

error: aborting due to 8 previous errors
error: this `continue` expression is redundant
--> $DIR/needless_continue.rs:173:21
|
LL | continue 'inner;
| ^^^^^^^^^^^^^^^^
|
= help: consider dropping the `continue` expression

error: this `continue` expression is redundant
--> $DIR/needless_continue.rs:176:21
|
LL | continue 'inner;
| ^^^^^^^^^^^^^^^^
|
= help: consider dropping the `continue` expression

error: aborting due to 11 previous errors

0 comments on commit d890421

Please sign in to comment.