Skip to content

Commit

Permalink
never_loop: suggest using an if let instead of a for loop
Browse files Browse the repository at this point in the history
  • Loading branch information
LeSeulArtichaut committed Aug 9, 2021
1 parent 2dbf0c1 commit 7e317a5
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 4 deletions.
59 changes: 55 additions & 4 deletions clippy_lints/src/loops/never_loop.rs
Original file line number Diff line number Diff line change
@@ -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 => (),
}
}
Expand Down Expand Up @@ -170,3 +194,30 @@ 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<'_>, for_span: Span, iterator: &Expr<'_>, pat: &Pat<'_>) -> Option<String> {
// 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())
}
7 changes: 7 additions & 0 deletions tests/ui/never_loop.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 7e317a5

Please sign in to comment.