Skip to content

Commit b1b3860

Browse files
committed
Auto merge of #7541 - LeSeulArtichaut:for-never-loop, r=camsteffen
`never_loop`: suggest using an `if let` instead of a `for` loop changelog: suggest using an `if let` statement instead of a `for` loop that [`never_loop`]s Fixes #7537, r? `@camsteffen.`
2 parents 76c4a33 + fc0af8e commit b1b3860

File tree

2 files changed

+43
-4
lines changed

2 files changed

+43
-4
lines changed

clippy_lints/src/loops/never_loop.rs

+38-4
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,36 @@
1+
use super::utils::make_iterator_snippet;
12
use super::NEVER_LOOP;
2-
use clippy_utils::diagnostics::span_lint;
3-
use rustc_hir::{Block, Expr, ExprKind, HirId, InlineAsmOperand, Stmt, StmtKind};
3+
use clippy_utils::diagnostics::span_lint_and_then;
4+
use clippy_utils::higher;
5+
use clippy_utils::source::snippet;
6+
use rustc_errors::Applicability;
7+
use rustc_hir::{Block, Expr, ExprKind, HirId, InlineAsmOperand, LoopSource, Node, Pat, Stmt, StmtKind};
48
use rustc_lint::LateContext;
59
use std::iter::{once, Iterator};
610

711
pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
8-
if let ExprKind::Loop(block, _, _, _) = expr.kind {
12+
if let ExprKind::Loop(block, _, source, _) = expr.kind {
913
match never_loop_block(block, expr.hir_id) {
10-
NeverLoopResult::AlwaysBreak => span_lint(cx, NEVER_LOOP, expr.span, "this loop never actually loops"),
14+
NeverLoopResult::AlwaysBreak => {
15+
span_lint_and_then(cx, NEVER_LOOP, expr.span, "this loop never actually loops", |diag| {
16+
if_chain! {
17+
if let LoopSource::ForLoop = source;
18+
if let Some((_, Node::Expr(parent_match))) = cx.tcx.hir().parent_iter(expr.hir_id).nth(1);
19+
if let Some((pat, iterator, _, for_span)) = higher::for_loop(parent_match);
20+
then {
21+
// Suggests using an `if let` instead. This is `Unspecified` because the
22+
// loop may (probably) contain `break` statements which would be invalid
23+
// in an `if let`.
24+
diag.span_suggestion_verbose(
25+
for_span.with_hi(iterator.span.hi()),
26+
"if you need the first element of the iterator, try writing",
27+
for_to_if_let_sugg(cx, iterator, pat),
28+
Applicability::Unspecified,
29+
);
30+
}
31+
};
32+
});
33+
},
1134
NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (),
1235
}
1336
}
@@ -170,3 +193,14 @@ fn never_loop_expr_branch<'a, T: Iterator<Item = &'a Expr<'a>>>(e: &mut T, main_
170193
e.map(|e| never_loop_expr(e, main_loop_id))
171194
.fold(NeverLoopResult::AlwaysBreak, combine_branches)
172195
}
196+
197+
fn for_to_if_let_sugg(cx: &LateContext<'_>, iterator: &Expr<'_>, pat: &Pat<'_>) -> String {
198+
let pat_snippet = snippet(cx, pat.span, "_");
199+
let iter_snippet = make_iterator_snippet(cx, iterator, &mut Applicability::Unspecified);
200+
201+
format!(
202+
"if let Some({pat}) = {iter}.next()",
203+
pat = pat_snippet,
204+
iter = iter_snippet
205+
)
206+
}

tests/ui/never_loop.stderr

+5
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@ LL | | _ => return,
7575
LL | | }
7676
LL | | }
7777
| |_____^
78+
|
79+
help: if you need the first element of the iterator, try writing
80+
|
81+
LL | if let Some(x) = (0..10).next() {
82+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
7883

7984
error: this loop never actually loops
8085
--> $DIR/never_loop.rs:157:5

0 commit comments

Comments
 (0)