Skip to content

Commit

Permalink
make never_loop applicability more flexible
Browse files Browse the repository at this point in the history
  • Loading branch information
lapla-cogito committed Feb 12, 2025
1 parent ffa1caf commit aaf870c
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 5 deletions.
23 changes: 19 additions & 4 deletions clippy_lints/src/loops/never_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::higher::ForLoop;
use clippy_utils::macros::root_macro_call_first_node;
use clippy_utils::source::snippet;
use clippy_utils::visitors::for_each_expr_without_closures;
use rustc_errors::Applicability;
use rustc_hir::{Block, Destination, Expr, ExprKind, HirId, InlineAsmOperand, Pat, Stmt, StmtKind, StructTailExpr};
use rustc_lint::LateContext;
use rustc_span::{Span, sym};
use std::iter::once;
use std::ops::ControlFlow;

pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
Expand All @@ -27,14 +29,19 @@ pub(super) fn check<'tcx>(
..
}) = for_loop
{
// Suggests using an `if let` instead. This is `Unspecified` because the
// loop may (probably) contain `break` statements which would be invalid
// in an `if let`.
let app = if let Some(block_expr) = block.expr
&& !is_loop_contains_break_continue(block_expr)
{
Applicability::MachineApplicable
} else {
Applicability::Unspecified
};

diag.span_suggestion_verbose(
for_span.with_hi(iterator.span.hi()),
"if you need the first element of the iterator, try writing",
for_to_if_let_sugg(cx, iterator, pat),
Applicability::Unspecified,
app,
);
}
});
Expand All @@ -43,6 +50,14 @@ pub(super) fn check<'tcx>(
}
}

fn is_loop_contains_break_continue(expr: &Expr<'_>) -> bool {
for_each_expr_without_closures(expr, |e| match e.kind {
ExprKind::Break(..) | ExprKind::Continue(..) => ControlFlow::Break(()),
_ => ControlFlow::Continue(()),
})
.is_some()
}

/// The `never_loop` analysis keeps track of three things:
///
/// * Has any (reachable) code path hit a `continue` of the main loop?
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/never_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,12 @@ pub fn issue12205() -> Option<()> {
}
}

fn no_break_or_continue_loop() {
for i in [1, 2, 3].iter().skip(1) {
return;
}
}

fn main() {
test1();
test2();
Expand Down
15 changes: 14 additions & 1 deletion tests/ui/never_loop.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -175,5 +175,18 @@ LL | | unimplemented!("not yet");
LL | | }
| |_____^

error: aborting due to 16 previous errors
error: this loop never actually loops
--> tests/ui/never_loop.rs:413:5
|
LL | / for i in [1, 2, 3].iter().skip(1) {
LL | | return;
LL | | }
| |_____^
|
help: if you need the first element of the iterator, try writing
|
LL | if let Some(i) = [1, 2, 3].iter().skip(1).next() {
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

error: aborting due to 17 previous errors

0 comments on commit aaf870c

Please sign in to comment.