diff --git a/clippy_lints/src/loops/never_loop.rs b/clippy_lints/src/loops/never_loop.rs index e97b7c941703..6817c3895956 100644 --- a/clippy_lints/src/loops/never_loop.rs +++ b/clippy_lints/src/loops/never_loop.rs @@ -1,13 +1,37 @@ 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::{indent_of, reindent_multiline, snippet, snippet_opt}; +use clippy_utils::ty::has_iter_method; +use rustc_ast::util::parser::PREC_POSTFIX; +use rustc_errors::Applicability; +use rustc_hir::{is_range_literal, Block, Expr, ExprKind, HirId, InlineAsmOperand, LoopSource, Pat, Stmt, StmtKind}; use rustc_lint::LateContext; +use rustc_span::Span; 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; + let parent_arm = cx.tcx.hir().get_parent_node(expr.hir_id); + let parent_match = cx.tcx.hir().expect_expr(cx.tcx.hir().get_parent_node(parent_arm)); + if let Some((pat, iterator, _, for_span)) = higher::for_loop(parent_match); + if let Some(sugg) = for_to_if_let_sugg(cx, for_span, iterator, pat); + then { + diag.span_suggestion_verbose( + expr.span, + "if you need the first element of the iterator, try writing", + sugg, + Applicability::HasPlaceholders, + ); + } + }; + }); + }, NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (), } } @@ -170,3 +194,30 @@ fn never_loop_expr_branch<'a, T: Iterator>>(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<'_>, for_span: Span, iterator: &Expr<'_>, pat: &Pat<'_>) -> Option { + // Check whether the iterator expr needs parens to parse as `(iterator).into_iter()`. + // Ranges need special casing because they are converted to a struct in HIR. + let (open_paren, close_paren) = if iterator.precedence().order() < PREC_POSTFIX || is_range_literal(iterator) { + ("(", ")") + } else { + ("", "") + }; + let iterator_snippet = snippet_opt(cx, iterator.span)?; + let pat_snippet = snippet(cx, pat.span, "_"); + let iter_method = has_iter_method(cx, cx.typeck_results().expr_ty(iterator)) + .map_or_else(|| "into_iter".into(), |s| s.to_string()); + + #[rustfmt::skip] + let sugg = format!( +"if let Some({pat}) = {open_paren}{iterator}{close_paren}.{iter_method}().next() {{ + /* ... */ +}}", + pat = pat_snippet, + open_paren = open_paren, + iterator = iterator_snippet, + close_paren = close_paren, + iter_method = iter_method + ); + Some(reindent_multiline(sugg.into(), true, indent_of(cx, for_span)).into()) +} diff --git a/tests/ui/never_loop.stderr b/tests/ui/never_loop.stderr index c00b4c78cf28..3c28532b4999 100644 --- a/tests/ui/never_loop.stderr +++ b/tests/ui/never_loop.stderr @@ -75,6 +75,13 @@ LL | | _ => return, LL | | } LL | | } | |_____^ + | +help: if you need the first element of the iterator, try writing + | +LL | if let Some(x) = (0..10).into_iter().next() { +LL | /* ... */ +LL | } + | error: this loop never actually loops --> $DIR/never_loop.rs:157:5