From a0469806e50ed60439b651f955c6d778220de462 Mon Sep 17 00:00:00 2001 From: yanglsh Date: Sun, 27 Jul 2025 18:06:59 +0800 Subject: [PATCH] fix: `never_loop` forget to remove break in nested loop --- clippy_lints/src/loops/never_loop.rs | 8 +++- tests/ui/never_loop.rs | 9 ----- tests/ui/never_loop.stderr | 36 +++--------------- tests/ui/never_loop_fixable.fixed | 17 ++++++++- tests/ui/never_loop_fixable.rs | 23 +++++++++++- tests/ui/never_loop_fixable.stderr | 56 +++++++++++++++++++++++++++- 6 files changed, 104 insertions(+), 45 deletions(-) diff --git a/clippy_lints/src/loops/never_loop.rs b/clippy_lints/src/loops/never_loop.rs index 8a253ae5810f..8b53d8917a9d 100644 --- a/clippy_lints/src/loops/never_loop.rs +++ b/clippy_lints/src/loops/never_loop.rs @@ -77,7 +77,7 @@ fn contains_any_break_or_continue(block: &Block<'_>) -> bool { /// The first two bits of information are in this enum, and the last part is in the /// `local_labels` variable, which contains a list of `(block_id, reachable)` pairs ordered by /// scope. -#[derive(Clone)] +#[derive(Clone, Debug)] enum NeverLoopResult { /// A continue may occur for the main loop. MayContinueMainLoop, @@ -207,6 +207,8 @@ fn all_spans_after_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> Vec { } return vec![stmt.span]; + } else if let Node::Block(_) = cx.tcx.parent_hir_node(expr.hir_id) { + return vec![expr.span]; } vec![] @@ -356,7 +358,9 @@ fn never_loop_expr<'tcx>( }; let result = combine_seq(result, || { if cx.typeck_results().expr_ty(expr).is_never() { - NeverLoopResult::Diverging { break_spans: vec![] } + NeverLoopResult::Diverging { + break_spans: all_spans_after_expr(cx, expr), + } } else { NeverLoopResult::Normal } diff --git a/tests/ui/never_loop.rs b/tests/ui/never_loop.rs index 48d4b8ad1519..ae9806588a17 100644 --- a/tests/ui/never_loop.rs +++ b/tests/ui/never_loop.rs @@ -431,15 +431,6 @@ fn stmt_after_return() { } fn loop_label() { - 'outer: for v in 0..10 { - //~^ never_loop - loop { - //~^ never_loop - break 'outer; - } - return; - } - for v in 0..10 { //~^ never_loop 'inner: loop { diff --git a/tests/ui/never_loop.stderr b/tests/ui/never_loop.stderr index 54b463266a3a..c587d240f2aa 100644 --- a/tests/ui/never_loop.stderr +++ b/tests/ui/never_loop.stderr @@ -185,32 +185,6 @@ LL ~ error: this loop never actually loops --> tests/ui/never_loop.rs:434:5 | -LL | / 'outer: for v in 0..10 { -LL | | -LL | | loop { -... | -LL | | return; -LL | | } - | |_____^ - | -help: if you need the first element of the iterator, try writing - | -LL - 'outer: for v in 0..10 { -LL + if let Some(v) = (0..10).next() { - | - -error: this loop never actually loops - --> tests/ui/never_loop.rs:436:9 - | -LL | / loop { -LL | | -LL | | break 'outer; -LL | | } - | |_________^ - -error: this loop never actually loops - --> tests/ui/never_loop.rs:443:5 - | LL | / for v in 0..10 { LL | | LL | | 'inner: loop { @@ -226,7 +200,7 @@ LL + if let Some(v) = (0..10).next() { | error: this loop never actually loops - --> tests/ui/never_loop.rs:445:9 + --> tests/ui/never_loop.rs:436:9 | LL | / 'inner: loop { LL | | @@ -235,7 +209,7 @@ LL | | } | |_________^ error: this loop never actually loops - --> tests/ui/never_loop.rs:471:5 + --> tests/ui/never_loop.rs:462:5 | LL | / 'a: for _ in 0..1 { LL | | @@ -251,7 +225,7 @@ LL ~ | error: this loop never actually loops - --> tests/ui/never_loop.rs:477:5 + --> tests/ui/never_loop.rs:468:5 | LL | / 'a: for i in 0..1 { LL | | @@ -275,7 +249,7 @@ LL ~ | error: this loop never actually loops - --> tests/ui/never_loop.rs:492:5 + --> tests/ui/never_loop.rs:483:5 | LL | / for v in 0..10 { LL | | @@ -297,5 +271,5 @@ LL ~ LL ~ | -error: aborting due to 24 previous errors +error: aborting due to 22 previous errors diff --git a/tests/ui/never_loop_fixable.fixed b/tests/ui/never_loop_fixable.fixed index 5bc9ff1bb4df..9683531478d8 100644 --- a/tests/ui/never_loop_fixable.fixed +++ b/tests/ui/never_loop_fixable.fixed @@ -1,4 +1,4 @@ -#![allow(clippy::iter_next_slice, clippy::needless_return)] +#![allow(clippy::iter_next_slice, clippy::needless_return, clippy::redundant_pattern_matching)] fn no_break_or_continue_loop() { if let Some(i) = [1, 2, 3].iter().next() { @@ -18,3 +18,18 @@ fn no_break_or_continue_loop_outer() { } } } + +fn loop_label() { + if let Some(v) = (0..10).next() { + //~^ never_loop + + + } +} + +fn issue15350() { + if let Some(_) = (0..100).next() { + //~^ never_loop + + } +} diff --git a/tests/ui/never_loop_fixable.rs b/tests/ui/never_loop_fixable.rs index 9782bc107e9a..81610ba65249 100644 --- a/tests/ui/never_loop_fixable.rs +++ b/tests/ui/never_loop_fixable.rs @@ -1,4 +1,4 @@ -#![allow(clippy::iter_next_slice, clippy::needless_return)] +#![allow(clippy::iter_next_slice, clippy::needless_return, clippy::redundant_pattern_matching)] fn no_break_or_continue_loop() { for i in [1, 2, 3].iter() { @@ -18,3 +18,24 @@ fn no_break_or_continue_loop_outer() { } } } + +fn loop_label() { + 'outer: for v in 0..10 { + //~^ never_loop + loop { + //~^ never_loop + break 'outer; + } + return; + } +} + +fn issue15350() { + 'bar: for _ in 0..100 { + //~^ never_loop + loop { + //~^ never_loop + break 'bar; + } + } +} diff --git a/tests/ui/never_loop_fixable.stderr b/tests/ui/never_loop_fixable.stderr index ee02d9a42976..bc238067a7f0 100644 --- a/tests/ui/never_loop_fixable.stderr +++ b/tests/ui/never_loop_fixable.stderr @@ -31,5 +31,59 @@ LL - for i in [1, 2, 3].iter() { LL + if let Some(i) = [1, 2, 3].iter().next() { | -error: aborting due to 2 previous errors +error: this loop never actually loops + --> tests/ui/never_loop_fixable.rs:23:5 + | +LL | / 'outer: for v in 0..10 { +LL | | +LL | | loop { +... | +LL | | return; +LL | | } + | |_____^ + | +help: if you need the first element of the iterator, try writing + | +LL ~ if let Some(v) = (0..10).next() { +LL | +LL ~ +LL ~ + | + +error: this loop never actually loops + --> tests/ui/never_loop_fixable.rs:25:9 + | +LL | / loop { +LL | | +LL | | break 'outer; +LL | | } + | |_________^ + +error: this loop never actually loops + --> tests/ui/never_loop_fixable.rs:34:5 + | +LL | / 'bar: for _ in 0..100 { +LL | | +LL | | loop { +... | +LL | | } + | |_____^ + | +help: if you need the first element of the iterator, try writing + | +LL ~ if let Some(_) = (0..100).next() { +LL | +LL ~ + | + +error: this loop never actually loops + --> tests/ui/never_loop_fixable.rs:36:9 + | +LL | / loop { +LL | | +LL | | break 'bar; +LL | | } + | |_________^ + +error: aborting due to 6 previous errors