fix: needless_continue wrongly unmangled macros

Author: profetia

clippy_lints/src/needless_continue.rs

@@ -1,5 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_help;
-use clippy_utils::source::{indent_of, snippet, snippet_block, snippet_with_context};
+use clippy_utils::macros::root_macro_call;
+use clippy_utils::source::{indent_of, snippet_block, snippet_with_context};
 use clippy_utils::{higher, is_from_proc_macro};
 use rustc_ast::Label;
 use rustc_errors::Applicability;
@@ -132,7 +133,12 @@ declare_lint_pass!(NeedlessContinue => [NEEDLESS_CONTINUE]);
 
 impl<'tcx> LateLintPass<'tcx> for NeedlessContinue {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
-        if !expr.span.in_external_macro(cx.sess().source_map()) && !is_from_proc_macro(cx, expr) {
+        // We cannot use `from_expansion` because for loops, while loops and while let loops are desugared
+        // into `loop` expressions.
+        if !expr.span.in_external_macro(cx.sess().source_map())
+            && !is_from_proc_macro(cx, expr)
+            && root_macro_call(expr.span).is_none()
+        {
             check_and_warn(cx, expr);
         }
     }
@@ -193,15 +199,15 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessContinue {
 /// - The expression node is a block with the first statement being a `continue`.
 fn needless_continue_in_else(else_expr: &Expr<'_>, label: Option<&Label>) -> bool {
     match else_expr.kind {
-        ExprKind::Block(ref else_block, _) => is_first_block_stmt_continue(else_block, label),
+        ExprKind::Block(else_block, _) => is_first_block_stmt_continue(else_block, label),
         ExprKind::Continue(l) => compare_labels(label, l.label.as_ref()),
         _ => false,
     }
 }
 
 fn is_first_block_stmt_continue(block: &Block<'_>, label: Option<&Label>) -> bool {
     block.stmts.first().is_some_and(|stmt| match stmt.kind {
-        StmtKind::Semi(ref e) | StmtKind::Expr(ref e) => {
+        StmtKind::Semi(e) | StmtKind::Expr(e) => {
             if let ExprKind::Continue(ref l) = e.kind {
                 compare_labels(label, l.label.as_ref())
             } else {
@@ -366,7 +372,14 @@ fn suggestion_snippet_for_continue_inside_if(cx: &LateContext<'_>, data: &LintDa
 }
 
 fn suggestion_snippet_for_continue_inside_else(cx: &LateContext<'_>, data: &LintData<'_>) -> String {
-    let cond_code = snippet(cx, data.if_cond.span, "..");
+    let mut applicability = Applicability::MachineApplicable;
+    let (cond_code, _) = snippet_with_context(
+        cx,
+        data.if_cond.span,
+        data.if_expr.span.ctxt(),
+        "..",
+        &mut applicability,
+    );
 
     // Region B
     let block_code = erode_from_back(&snippet_block(cx, data.if_block.span, "..", Some(data.if_expr.span)));
@@ -402,7 +415,7 @@ fn suggestion_snippet_for_continue_inside_else(cx: &LateContext<'_>, data: &Lint
         }
         lines.join("\n")
     } else {
-        "".to_string()
+        String::new()
     };
 
     let indent_if = indent_of(cx, data.if_expr.span).unwrap_or(0);
@@ -417,7 +430,7 @@ fn check_last_stmt_in_expr<F>(cx: &LateContext<'_>, inner_expr: &Expr<'_>, func:
 where
     F: Fn(Option<&Label>, Span),
 {
-    match &inner_expr.kind {
+    match inner_expr.kind {
         ExprKind::Continue(continue_label) => {
             func(continue_label.label.as_ref(), inner_expr.span);
         },
@@ -432,7 +445,7 @@ where
             if !match_ty.is_unit() && !match_ty.is_never() {
                 return;
             }
-            for arm in arms.iter() {
+            for arm in arms {
                 check_last_stmt_in_expr(cx, arm.body, func);
             }
         },

tests/ui/needless_continue.rs

@@ -246,16 +246,99 @@ mod issue_4077 {
 }
 
 #[allow(clippy::let_unit_value)]
-fn issue14550(mut producer: impl Iterator<Item = Result<i32, u32>>) -> Result<u32, u32> {
-    let mut counter = 2;
-    loop {
-        match producer.next().unwrap() {
-            Ok(ok) => break Ok((ok + 1) as u32),
-            Err(12) => {
-                counter -= 1;
+mod issue14550 {
+    fn match_with_value(mut producer: impl Iterator<Item = Result<i32, u32>>) -> Result<u32, u32> {
+        let mut counter = 2;
+        loop {
+            match producer.next().unwrap() {
+                Ok(ok) => break Ok((ok + 1) as u32),
+                Err(12) => {
+                    counter -= 1;
+                    continue;
+                },
+                err => err?,
+            };
+        }
+    }
+
+    fn inside_macro() {
+        macro_rules! mac {
+            ($e:expr => $($rest:tt);*) => {
+                loop {
+                    match $e {
+                        1 => continue,
+                        2 => break,
+                        n => println!("{n}"),
+                    }
+                    $($rest;)*
+                }
+            };
+        }
+
+        mac!(2 => );
+        mac!(1 => {println!("foobar")});
+    }
+
+    mod partially_inside_macro {
+        macro_rules! select {
+            (
+                $expr:expr,
+                $( $pat:pat => $then:expr ),*
+            ) => {
+                fn foo() {
+                    loop {
+                        match $expr {
+                            $(
+                                $pat => $then,
+                            )*
+                        }
+                    }
+                }
+            };
+        }
+
+        select!(Some(1),
+            Some(1) => {
+                println!("one");
                 continue;
             },
-            err => err?,
-        };
+            Some(2) => {},
+            None => break,
+            _ => ()
+        );
+
+        macro_rules! choose {
+            (
+            $expr:expr,
+            $case:expr
+        ) => {
+                fn bar() {
+                    loop {
+                        match $expr {
+                            $case => {
+                                println!("matched");
+                                continue;
+                            },
+                            _ => {
+                                println!("not matched");
+                                break;
+                            },
+                        }
+                    }
+                }
+            };
+        }
+
+        choose!(todo!(), 5);
+    }
+}
+
+fn issue15548() {
+    loop {
+        if todo!() {
+        } else {
+            //~^ needless_continue
+            continue;
+        }
     }
 }

tests/ui/needless_continue.stderr

@@ -220,5 +220,21 @@ LL | |             }
                            do_something();
                        }
 
-error: aborting due to 15 previous errors
+error: this `else` block is redundant
+  --> tests/ui/needless_continue.rs:339:16
+   |
+LL |           } else {
+   |  ________________^
+LL | |
+LL | |             continue;
+LL | |         }
+   | |_________^
+   |
+   = help: consider dropping the `else` clause and merging the code that follows (in the loop) with the `if` block
+                   if todo!() {
+           // merged code follows:
+           
+                   }
+
+error: aborting due to 16 previous errors