diff options
| author | bors <bors@rust-lang.org> | 2021-08-11 14:49:37 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2021-08-11 14:49:37 +0000 |
| commit | b1b38604f23b3a51620e2f9454353eb2fbf028a3 (patch) | |
| tree | 2b9c16bbc3667dec6f429f6fbbb999edaf0c79da | |
| parent | 76c4a337d1a9daef05aad63548f55568dffa7424 (diff) | |
| parent | fc0af8e4d80272b1255a6582a03f846b6f3dc66a (diff) | |
| download | rust-b1b38604f23b3a51620e2f9454353eb2fbf028a3.tar.gz rust-b1b38604f23b3a51620e2f9454353eb2fbf028a3.zip | |
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.`
| -rw-r--r-- | clippy_lints/src/loops/never_loop.rs | 42 | ||||
| -rw-r--r-- | tests/ui/never_loop.stderr | 5 |
2 files changed, 43 insertions, 4 deletions
diff --git a/clippy_lints/src/loops/never_loop.rs b/clippy_lints/src/loops/never_loop.rs index e97b7c94170..6d9f6215ed4 100644 --- a/clippy_lints/src/loops/never_loop.rs +++ b/clippy_lints/src/loops/never_loop.rs @@ -1,13 +1,36 @@ +use super::utils::make_iterator_snippet; use super::NEVER_LOOP; -use clippy_utils::diagnostics::span_lint; -use rustc_hir::{Block, Expr, ExprKind, HirId, InlineAsmOperand, Stmt, StmtKind}; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::higher; +use clippy_utils::source::snippet; +use rustc_errors::Applicability; +use rustc_hir::{Block, Expr, ExprKind, HirId, InlineAsmOperand, LoopSource, Node, Pat, Stmt, StmtKind}; use rustc_lint::LateContext; use std::iter::{once, Iterator}; pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let ExprKind::Loop(block, _, _, _) = expr.kind { + if let ExprKind::Loop(block, _, source, _) = expr.kind { match never_loop_block(block, expr.hir_id) { - NeverLoopResult::AlwaysBreak => span_lint(cx, NEVER_LOOP, expr.span, "this loop never actually loops"), + NeverLoopResult::AlwaysBreak => { + span_lint_and_then(cx, NEVER_LOOP, expr.span, "this loop never actually loops", |diag| { + if_chain! { + if let LoopSource::ForLoop = source; + if let Some((_, Node::Expr(parent_match))) = cx.tcx.hir().parent_iter(expr.hir_id).nth(1); + if let Some((pat, iterator, _, for_span)) = higher::for_loop(parent_match); + then { + // 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`. + 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, + ); + } + }; + }); + }, NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (), } } @@ -170,3 +193,14 @@ fn never_loop_expr_branch<'a, T: Iterator<Item = &'a Expr<'a>>>(e: &mut T, main_ e.map(|e| never_loop_expr(e, main_loop_id)) .fold(NeverLoopResult::AlwaysBreak, combine_branches) } + +fn for_to_if_let_sugg(cx: &LateContext<'_>, iterator: &Expr<'_>, pat: &Pat<'_>) -> String { + let pat_snippet = snippet(cx, pat.span, "_"); + let iter_snippet = make_iterator_snippet(cx, iterator, &mut Applicability::Unspecified); + + format!( + "if let Some({pat}) = {iter}.next()", + pat = pat_snippet, + iter = iter_snippet + ) +} diff --git a/tests/ui/never_loop.stderr b/tests/ui/never_loop.stderr index c00b4c78cf2..9fbf3e9ce98 100644 --- a/tests/ui/never_loop.stderr +++ b/tests/ui/never_loop.stderr @@ -75,6 +75,11 @@ LL | | _ => return, LL | | } LL | | } | |_____^ + | +help: if you need the first element of the iterator, try writing + | +LL | if let Some(x) = (0..10).next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this loop never actually loops --> $DIR/never_loop.rs:157:5 |
