From 0cdf6e172cb861387d41ad01540a321a1e09c1b4 Mon Sep 17 00:00:00 2001 From: jusexton Date: Tue, 23 Jul 2024 02:55:58 -0500 Subject: [PATCH] Fix while_let_on_iterator dropping loop label when applying fix. --- clippy_lints/src/loops/while_let_on_iterator.rs | 7 +++++-- clippy_utils/src/higher.rs | 4 +++- tests/ui/while_let_on_iterator.fixed | 9 +++++++++ tests/ui/while_let_on_iterator.rs | 9 +++++++++ tests/ui/while_let_on_iterator.stderr | 8 +++++++- 5 files changed, 33 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/loops/while_let_on_iterator.rs b/clippy_lints/src/loops/while_let_on_iterator.rs index 194dd4752..c171fa1c6 100644 --- a/clippy_lints/src/loops/while_let_on_iterator.rs +++ b/clippy_lints/src/loops/while_let_on_iterator.rs @@ -14,7 +14,7 @@ use rustc_span::symbol::sym; use rustc_span::Symbol; pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let Some(higher::WhileLet { if_then, let_pat, let_expr, .. }) = higher::WhileLet::hir(expr) + if let Some(higher::WhileLet { if_then, let_pat, let_expr, label, .. }) = higher::WhileLet::hir(expr) // check for `Some(..)` pattern && let PatKind::TupleStruct(ref pat_path, some_pat, _) = let_pat.kind && is_res_lang_ctor(cx, cx.qpath_res(pat_path, let_pat.hir_id), LangItem::OptionSome) @@ -27,6 +27,9 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { && !uses_iter(cx, &iter_expr_struct, if_then) { let mut applicability = Applicability::MachineApplicable; + + let loop_label = label.map_or(String::new(), |l| format!("{}: ", l.ident.name)); + let loop_var = if let Some(some_pat) = some_pat.first() { if is_refutable(cx, some_pat) { // Refutable patterns don't work with for loops. @@ -57,7 +60,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { expr.span.with_hi(let_expr.span.hi()), "this loop could be written as a `for` loop", "try", - format!("for {loop_var} in {iterator}{by_ref}"), + format!("{loop_label}for {loop_var} in {iterator}{by_ref}"), applicability, ); } diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs index 277ba8427..7ce9bde1a 100644 --- a/clippy_utils/src/higher.rs +++ b/clippy_utils/src/higher.rs @@ -367,6 +367,7 @@ pub struct WhileLet<'hir> { pub let_expr: &'hir Expr<'hir>, /// `while let` loop body pub if_then: &'hir Expr<'hir>, + pub label: Option, /// `while let PAT = EXPR` /// ^^^^^^^^^^^^^^ pub let_span: Span, @@ -399,7 +400,7 @@ impl<'hir> WhileLet<'hir> { }), .. }, - _, + label, LoopSource::While, _, ) = expr.kind @@ -408,6 +409,7 @@ impl<'hir> WhileLet<'hir> { let_pat, let_expr, if_then, + label, let_span, }); } diff --git a/tests/ui/while_let_on_iterator.fixed b/tests/ui/while_let_on_iterator.fixed index 59b5c858d..b8087c6e0 100644 --- a/tests/ui/while_let_on_iterator.fixed +++ b/tests/ui/while_let_on_iterator.fixed @@ -456,6 +456,15 @@ fn fn_once_closure() { }); } +fn issue13123() { + let mut it = 0..20; + 'label: for n in it { + if n % 25 == 0 { + break 'label; + } + } +} + fn main() { let mut it = 0..20; for _ in it { diff --git a/tests/ui/while_let_on_iterator.rs b/tests/ui/while_let_on_iterator.rs index 559513d56..8e02f59b5 100644 --- a/tests/ui/while_let_on_iterator.rs +++ b/tests/ui/while_let_on_iterator.rs @@ -456,6 +456,15 @@ fn fn_once_closure() { }); } +fn issue13123() { + let mut it = 0..20; + 'label: while let Some(n) = it.next() { + if n % 25 == 0 { + break 'label; + } + } +} + fn main() { let mut it = 0..20; while let Some(..) = it.next() { diff --git a/tests/ui/while_let_on_iterator.stderr b/tests/ui/while_let_on_iterator.stderr index 8ff1f2364..d96b26acf 100644 --- a/tests/ui/while_let_on_iterator.stderr +++ b/tests/ui/while_let_on_iterator.stderr @@ -160,8 +160,14 @@ LL | while let Some(x) = it.next() { error: this loop could be written as a `for` loop --> tests/ui/while_let_on_iterator.rs:461:5 | +LL | 'label: while let Some(n) = it.next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'label: for n in it` + +error: this loop could be written as a `for` loop + --> tests/ui/while_let_on_iterator.rs:470:5 + | LL | while let Some(..) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in it` -error: aborting due to 27 previous errors +error: aborting due to 28 previous errors