Skip to content

Commit

Permalink
Fix while_let_on_iterator dropping loop label when applying fix.
Browse files Browse the repository at this point in the history
  • Loading branch information
jusexton committed Jul 23, 2024
1 parent df1baed commit 0cdf6e1
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 4 deletions.
7 changes: 5 additions & 2 deletions clippy_lints/src/loops/while_let_on_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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.
Expand Down Expand Up @@ -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,
);
}
Expand Down
4 changes: 3 additions & 1 deletion clippy_utils/src/higher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ast::Label>,
/// `while let PAT = EXPR`
/// ^^^^^^^^^^^^^^
pub let_span: Span,
Expand Down Expand Up @@ -399,7 +400,7 @@ impl<'hir> WhileLet<'hir> {
}),
..
},
_,
label,
LoopSource::While,
_,
) = expr.kind
Expand All @@ -408,6 +409,7 @@ impl<'hir> WhileLet<'hir> {
let_pat,
let_expr,
if_then,
label,
let_span,
});
}
Expand Down
9 changes: 9 additions & 0 deletions tests/ui/while_let_on_iterator.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 9 additions & 0 deletions tests/ui/while_let_on_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/while_let_on_iterator.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 0cdf6e1

Please sign in to comment.