[needless_continue]: lint if the last stmt in for/while/loop is continue, recursively

lengyijun · lengyijun · commit 4a4599759610 · 2023-09-23T12:45:02.000+08:00
fixes: #4077
diff --git a/clippy_lints/src/methods/unnecessary_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs
@@ -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)
diff --git a/clippy_lints/src/needless_continue.rs b/clippy_lints/src/needless_continue.rs
@@ -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;
@@ -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
@@ -110,6 +111,45 @@ declare_clippy_lint! {
     ///     # break;
     /// }
     /// ```
+    ///
+    /// ```rust
+    /// fn foo() -> std::io::ErrorKind { unimplemented!() }
+    /// loop {
+    ///     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 { unimplemented!() }
+    /// loop {
+    ///     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,
@@ -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 {
diff --git a/clippy_lints/src/redundant_else.rs b/clippy_lints/src/redundant_else.rs
@@ -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,
diff --git a/clippy_lints/src/transmute/transmute_undefined_repr.rs b/clippy_lints/src/transmute/transmute_undefined_repr.rs
@@ -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
@@ -52,7 +49,6 @@ pub(super) fn check<'tcx>(
             {
                 from_ty = from_sub_ty;
                 to_ty = to_sub_ty;
-                continue;
             },
 
             // fat ptr <-> (*size, *size)
diff --git a/tests/missing-test-files.rs b/tests/missing-test-files.rs
@@ -60,7 +60,7 @@ fn explore_directory(dir: &Path) -> Vec<String> {
                             missing_files.push(path.to_str().unwrap().to_string());
                         }
                     },
-                    _ => continue,
+                    _ => {},
                 };
             }
         }
diff --git a/tests/ui/needless_continue.rs b/tests/ui/needless_continue.rs
@@ -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!()
@@ -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
+    }
+}
diff --git a/tests/ui/needless_continue.stderr b/tests/ui/needless_continue.stderr
@@ -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 {
    |  ________________________^
@@ -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 | |
@@ -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
 

Original file line number	Diff line number	Diff line change
`@@ -60,7 +60,7 @@ fn explore_directory(dir: &Path) -> Vec<String> {`
`60`	`60`	`missing_files.push(path.to_str().unwrap().to_string());`
`61`	`61`	`}`
`62`	`62`	`},`
`63`		`- _ => continue,`
	`63`	`+ _ => {},`
`64`	`64`	`};`
`65`	`65`	`}`
`66`	`66`	`}`