New lint: while let Some(&x) = slice.iter().next() is infinite loop

[tesuji]

This code is an infinite loop: (Do not run this in playpen)

fn main() {
    let a = [1, 2, 3];
    while let Some(&x) = a.iter().next() {
        println!("{:?}", x);
    }
}

[flip1995]

Basically calling _.iter() in the while let expression will lead to an infinite loop (except the collection is empty).

[pop]

I'd love to contribute to Clippy. Since this is a good first issue I was hoping I could start working on it. Could I get some pointers on where in the code to get started?

[llogiq]

Cool! I usually start by writing a test case (create a new .rs file in tests/ui, see the other files there for examples). Then I look into where the lint fits in, for example clippy_lints/src/loops.rs might be a good fit. There you first need to add the lint declaration and add the lint to the LintPass. Finally you need some code to check for iter calls whose expr_ty implements Iterator.

[flip1995]

I'm a little late, but this doc gives you step by step instructions on how to write a new lint. For this lint the Author lint chapter is probably interesting for you.

[chibby0ne]

Hi all, if @pop is not going to tackle this issue, I'd like to take it. I've already taken a look at the resources, thanks @llogiq and @flip1995.
I think a LateLintPass fits here because we need the type information of the match arm.
From what I've seen this is the place where we should detect this condition and emit the lint message:

rust-clippy/clippy_lints/src/loops.rs

Line 560 in 54bf4ff

if let ExprKind::Match(ref match_expr, ref arms, MatchSource::WhileLetDesugar) = expr.kind {

This "desugars" the while let into a loop match, which in our case would be:

while let Some(&x) = a.iter().next() {
    println!("{:?}", x);
}

into

loop {
    match a.iter().next() {
        Some(&x) => { println!("{:?}", x); },
        _ => (),
        }
    }
}

Afterwards it decomposes the first arm using the pattern kind and the match_expre using the expression kind as well,

rust-clippy/clippy_lints/src/loops.rs

Lines 561 to 566 in 54bf4ff

	let pat = &arms[0].pat.kind;
	if let (
	&PatKind::TupleStruct(ref qpath, ref pat_args, _),
	&ExprKind::MethodCall(ref method_path, _, ref method_args),
	) = (pat, &match_expr.kind)
	{

So that, roughly speaking:

pat is Some(&x) => { println!("{:?}", x); },
match_expr is a Method call => a.iter().next()

Where I get lost is how to get the type information from the qpath, and method_path.
I've looked into match_type in utils, but it seems it expects a different type, than qpath, which is a hir::QPath or in method_path case it is hir::PathSegment.

I'll keep looking into it, but if you have any useful pointers into what I should look into (HIR, MIR, Ty) or examples I would greatly appreciate it, specially since I'm new to clippy and rustc internals.

[pop]

You seem primed and ready to take this one, so go for it @chibby0ne.

[flip1995]

From what I've seen this is the place where we should detect this condition and emit the lint message:

Seems good to me

So that, roughly speaking:
pat is Some(&x) => { println!("{:?}", x); },
match_expr is a Method call => a.iter().next()

Exactly, you should only have to look at the match_expr though.

how to get the type information from the qpath

You should be able to use the function

rust-clippy/clippy_lints/src/utils/mod.rs

Lines 178 to 187 in 737f0a6

	/// Matches a `QPath` against a slice of segment string literals.
	///
	/// There is also `match_path` if you are dealing with a `rustc::hir::Path` instead of a
	/// `rustc::hir::QPath`.
	///
	/// # Examples
	/// ```rust,ignore
	/// match_qpath(path, &["std", "rt", "begin_unwind"])
	/// ```
	pub fn match_qpath(path: &QPath, segments: &[&str]) -> bool {

You can grep the repo for match_qpath on how to use it.

[chibby0ne]

You seem primed and ready to take this one, so go for it @chibby0ne.

Thanks @pop I appreciate! 😄

[felix91gr]

How did you fare with this one, @chibby0ne? :)

[boxdot]

I started to look into this issue. But it seems that it contradicts #1654. There is also an example where recreating the iterator is actually useful:

use std::collections::HashSet;
let mut values = HashSet::new();
values.insert(1);

while let Some(..) = values.iter().next() {
    values.remove(&1);
}

Also see

rust-clippy/clippy_lints/src/loops/while_let_on_iterator.rs

Lines 26 to 34 in ddc2598

	// Don't lint when the iterator is recreated on every iteration
	if_chain! {
	if let ExprKind::MethodCall(..) | ExprKind::Call(..) = iter_expr.kind;
	if let Some(iter_def_id) = cx.tcx.get_diagnostic_item(sym::Iterator);
	if implements_trait(cx, cx.typeck_results().expr_ty(iter_expr), iter_def_id, &[]);
	then {
	return;
	}
	}

[bbb651]

This is also useful for streams, it's very easy to miss in streams that wait for events because they behave almost identically and manifest in subtle ways, I recently came across this:

 #[tokio::main]
 async fn main() -> anyhow::Result<()> {
-    while let Some(mode) = dark_light::subscribe().await?.next().await {
+    let mut stream = dark_light::subscribe().await?;
+    while let Some(mode) = stream.next().await {
         println!("System theme changed: {:?}", mode);
     }