mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-10 07:04:18 +00:00
Auto merge of #13149 - jusexton:issue-13123, r=dswij
Fix while_let_on_iterator dropping loop label when applying fix. Loop label was not persisted when displaying help and was therefore producing broken rust code when applying fixes. Solution was to store the `ast::Label` when creating a `higher::WhileLet` from an expression and add the label name to the lint suggestion and diagnostics. --- Fixes: https://github.com/rust-lang/rust-clippy/issues/13123 changelog: [`while_let_on_iterator`]: Fix issue dropping loop label when displaying help and applying fixes.
This commit is contained in:
commit
d20be39c7f
5 changed files with 33 additions and 4 deletions
|
@ -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,
|
||||
);
|
||||
}
|
||||
|
|
|
@ -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,
|
||||
|
@ -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,
|
||||
});
|
||||
}
|
||||
|
|
|
@ -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 {
|
||||
|
|
|
@ -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() {
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
Loading…
Reference in a new issue