Fix fix under loop may dropping loop label when applying fix.

This commit is contained in:
shenyifu 2024-07-29 14:48:47 +08:00
parent 668b659b47
commit 38a3462037
8 changed files with 67 additions and 8 deletions

View File

@ -2,6 +2,7 @@
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::source::snippet_with_applicability; use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{get_enclosing_block, is_integer_const}; use clippy_utils::{get_enclosing_block, is_integer_const};
use rustc_ast::Label;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_block, walk_expr}; use rustc_hir::intravisit::{walk_block, walk_expr};
use rustc_hir::{Expr, Pat}; use rustc_hir::{Expr, Pat};
@ -17,6 +18,7 @@ pub(super) fn check<'tcx>(
arg: &'tcx Expr<'_>, arg: &'tcx Expr<'_>,
body: &'tcx Expr<'_>, body: &'tcx Expr<'_>,
expr: &'tcx Expr<'_>, expr: &'tcx Expr<'_>,
label: Option<Label>,
) { ) {
// Look for variables that are incremented once per loop iteration. // Look for variables that are incremented once per loop iteration.
let mut increment_visitor = IncrementVisitor::new(cx); let mut increment_visitor = IncrementVisitor::new(cx);
@ -34,7 +36,7 @@ pub(super) fn check<'tcx>(
{ {
let mut applicability = Applicability::MaybeIncorrect; let mut applicability = Applicability::MaybeIncorrect;
let span = expr.span.with_hi(arg.span.hi()); let span = expr.span.with_hi(arg.span.hi());
let loop_label = label.map_or(String::new(), |l| format!("{}: ", l.ident.name));
let int_name = match ty.map(Ty::kind) { let int_name = match ty.map(Ty::kind) {
// usize or inferred // usize or inferred
Some(ty::Uint(UintTy::Usize)) | None => { Some(ty::Uint(UintTy::Usize)) | None => {
@ -45,7 +47,7 @@ pub(super) fn check<'tcx>(
format!("the variable `{name}` is used as a loop counter"), format!("the variable `{name}` is used as a loop counter"),
"consider using", "consider using",
format!( format!(
"for ({name}, {}) in {}.enumerate()", "{loop_label}for ({name}, {}) in {}.enumerate()",
snippet_with_applicability(cx, pat.span, "item", &mut applicability), snippet_with_applicability(cx, pat.span, "item", &mut applicability),
make_iterator_snippet(cx, arg, &mut applicability), make_iterator_snippet(cx, arg, &mut applicability),
), ),
@ -68,7 +70,7 @@ pub(super) fn check<'tcx>(
span, span,
"consider using", "consider using",
format!( format!(
"for ({name}, {}) in (0_{int_name}..).zip({})", "{loop_label}for ({name}, {}) in (0_{int_name}..).zip({})",
snippet_with_applicability(cx, pat.span, "item", &mut applicability), snippet_with_applicability(cx, pat.span, "item", &mut applicability),
make_iterator_snippet(cx, arg, &mut applicability), make_iterator_snippet(cx, arg, &mut applicability),
), ),

View File

@ -25,6 +25,7 @@
use clippy_config::msrvs::Msrv; use clippy_config::msrvs::Msrv;
use clippy_config::Conf; use clippy_config::Conf;
use clippy_utils::higher; use clippy_utils::higher;
use rustc_ast::Label;
use rustc_hir::{Expr, ExprKind, LoopSource, Pat}; use rustc_hir::{Expr, ExprKind, LoopSource, Pat};
use rustc_lint::{LateContext, LateLintPass}; use rustc_lint::{LateContext, LateLintPass};
use rustc_session::impl_lint_pass; use rustc_session::impl_lint_pass;
@ -760,6 +761,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
body, body,
loop_id, loop_id,
span, span,
label,
}) = for_loop }) = for_loop
{ {
// we don't want to check expanded macros // we don't want to check expanded macros
@ -768,7 +770,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if body.span.from_expansion() { if body.span.from_expansion() {
return; return;
} }
self.check_for_loop(cx, pat, arg, body, expr, span); self.check_for_loop(cx, pat, arg, body, expr, span, label);
if let ExprKind::Block(block, _) = body.kind { if let ExprKind::Block(block, _) = body.kind {
never_loop::check(cx, block, loop_id, span, for_loop.as_ref()); never_loop::check(cx, block, loop_id, span, for_loop.as_ref());
} }
@ -808,6 +810,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
} }
impl Loops { impl Loops {
#[allow(clippy::too_many_arguments)]
fn check_for_loop<'tcx>( fn check_for_loop<'tcx>(
&self, &self,
cx: &LateContext<'tcx>, cx: &LateContext<'tcx>,
@ -816,11 +819,12 @@ fn check_for_loop<'tcx>(
body: &'tcx Expr<'_>, body: &'tcx Expr<'_>,
expr: &'tcx Expr<'_>, expr: &'tcx Expr<'_>,
span: Span, span: Span,
label: Option<Label>,
) { ) {
let is_manual_memcpy_triggered = manual_memcpy::check(cx, pat, arg, body, expr); let is_manual_memcpy_triggered = manual_memcpy::check(cx, pat, arg, body, expr);
if !is_manual_memcpy_triggered { if !is_manual_memcpy_triggered {
needless_range_loop::check(cx, pat, arg, body, expr); needless_range_loop::check(cx, pat, arg, body, expr);
explicit_counter_loop::check(cx, pat, arg, body, expr); explicit_counter_loop::check(cx, pat, arg, body, expr, label);
} }
self.check_for_loop_arg(cx, pat, arg); self.check_for_loop_arg(cx, pat, arg);
for_kv_map::check(cx, pat, arg, body); for_kv_map::check(cx, pat, arg, body);

View File

@ -25,6 +25,8 @@ pub struct ForLoop<'tcx> {
pub loop_id: HirId, pub loop_id: HirId,
/// entire `for` loop span /// entire `for` loop span
pub span: Span, pub span: Span,
/// label
pub label: Option<ast::Label>,
} }
impl<'tcx> ForLoop<'tcx> { impl<'tcx> ForLoop<'tcx> {
@ -33,7 +35,7 @@ pub fn hir(expr: &Expr<'tcx>) -> Option<Self> {
if let ExprKind::DropTemps(e) = expr.kind if let ExprKind::DropTemps(e) = expr.kind
&& let ExprKind::Match(iterexpr, [arm], MatchSource::ForLoopDesugar) = e.kind && let ExprKind::Match(iterexpr, [arm], MatchSource::ForLoopDesugar) = e.kind
&& let ExprKind::Call(_, [arg]) = iterexpr.kind && let ExprKind::Call(_, [arg]) = iterexpr.kind
&& let ExprKind::Loop(block, ..) = arm.body.kind && let ExprKind::Loop(block, label, ..) = arm.body.kind
&& let [stmt] = block.stmts && let [stmt] = block.stmts
&& let hir::StmtKind::Expr(e) = stmt.kind && let hir::StmtKind::Expr(e) = stmt.kind
&& let ExprKind::Match(_, [_, some_arm], _) = e.kind && let ExprKind::Match(_, [_, some_arm], _) = e.kind
@ -45,6 +47,7 @@ pub fn hir(expr: &Expr<'tcx>) -> Option<Self> {
body: some_arm.body, body: some_arm.body,
loop_id: arm.body.hir_id, loop_id: arm.body.hir_id,
span: expr.span.ctxt().outer_expn_data().call_site, span: expr.span.ctxt().outer_expn_data().call_site,
label,
}); });
} }
None None

View File

@ -278,3 +278,16 @@ pub fn test2() {
} }
} }
} }
mod issue_13123 {
pub fn test() {
let mut vec = vec![1, 2, 3, 4];
let mut _index = 0;
'label: for v in vec {
_index += 1;
if v == 1 {
break 'label;
}
}
}
}

View File

@ -57,5 +57,11 @@ LL | for _item in slice {
| |
= note: `idx_u32` is of type `u32`, making it ineligible for `Iterator::enumerate` = note: `idx_u32` is of type `u32`, making it ineligible for `Iterator::enumerate`
error: aborting due to 9 previous errors error: the variable `_index` is used as a loop counter
--> tests/ui/explicit_counter_loop.rs:286:9
|
LL | 'label: for v in vec {
| ^^^^^^^^^^^^^^^^^^^^ help: consider using: `'label: for (_index, v) in vec.into_iter().enumerate()`
error: aborting due to 10 previous errors

View File

@ -40,6 +40,16 @@ fn main() {
let _k = k; let _k = k;
} }
let m: HashMap<u64, u64> = HashMap::new();
let rm = &m;
'label: for k in rm.keys() {
//~^ ERROR: you seem to want to iterate on a map's keys
let _k = k;
if *k == 0u64 {
break 'label;
}
}
// The following should not produce warnings. // The following should not produce warnings.
let m: HashMap<u64, u64> = HashMap::new(); let m: HashMap<u64, u64> = HashMap::new();

View File

@ -40,6 +40,16 @@ fn main() {
let _k = k; let _k = k;
} }
let m: HashMap<u64, u64> = HashMap::new();
let rm = &m;
'label: for (k, _value) in rm {
//~^ ERROR: you seem to want to iterate on a map's keys
let _k = k;
if *k == 0u64 {
break 'label;
}
}
// The following should not produce warnings. // The following should not produce warnings.
let m: HashMap<u64, u64> = HashMap::new(); let m: HashMap<u64, u64> = HashMap::new();

View File

@ -55,5 +55,16 @@ help: use the corresponding method
LL | for k in rm.keys() { LL | for k in rm.keys() {
| ~ ~~~~~~~~~ | ~ ~~~~~~~~~
error: aborting due to 5 previous errors error: you seem to want to iterate on a map's keys
--> tests/ui/for_kv_map.rs:45:32
|
LL | 'label: for (k, _value) in rm {
| ^^
|
help: use the corresponding method
|
LL | 'label: for k in rm.keys() {
| ~ ~~~~~~~~~
error: aborting due to 6 previous errors