about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-08-11 14:49:37 +0000
committerbors <bors@rust-lang.org>2021-08-11 14:49:37 +0000
commitb1b38604f23b3a51620e2f9454353eb2fbf028a3 (patch)
tree2b9c16bbc3667dec6f429f6fbbb999edaf0c79da
parent76c4a337d1a9daef05aad63548f55568dffa7424 (diff)
parentfc0af8e4d80272b1255a6582a03f846b6f3dc66a (diff)
downloadrust-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.rs42
-rw-r--r--tests/ui/never_loop.stderr5
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