Warn when iterating an array of ranges

[teor2345]

What it does

Warns users if they accidentally iterate range struct items, rather than iterating through the indexes in the range:

Suggests:

• iterating through the indexes in the range, or
• disambiguating the range struct iteration by providing a range type.

This lint will become more important once rustc allows iterating arrays by value, because that allows code like:

for _ in [0..5] {} // iterates through 1 range, not 5 indexes

For context, see rust-lang/rust#84147 (comment)

Categories (optional)

• Kind: clippy::correctness

What is the advantage of the recommended code over the original code

The recommended code either:

• does what the user actually intended, or
• makes it clear that the user intended something really unusual (iterating ranges themselves, not their indexes).

Drawbacks

Expressing iterating through an array of ranges becomes more verbose. But this disambiguation makes the intent of the code much clearer.

Example

for _ in [0..5] {} // iterates through 1 range, clippy warning

Is probably intended to be:

for _ in 0..5 {} // iterates through 5 indexes

And if range iteration is genuinely intended, it could be disambiguated as:

for _: Range<_> in [0..5] {} // iterates through 1 range, no clippy warning

[scottmcm]

nit: you can't actually annotate a type for the pattern in a for loop -- it's not supported in the grammar.

[matthiaskrgr]

i guess this should only lint on arrays of len 1?

what about code like this:

pub fn main() {
    for r in [0..7, 8..5, 37..42] {
        println!("{:?}",r);
    }
}

[llogiq]

Shouldn't that one be caught by single_element_loop?

[llogiq]

Looking into that, it catches for r in &[0..5], but not (yet) for r in [0..5].iter(). for r in [0..5] is currently caught by the compiler, because [Range; 1] does not implement IntoIterator in this edition.

[llogiq]

I'll extend single_element_loop to also catch the .iter() variant.

[camsteffen]

What if we just lint for _ in [any_single_element] since that is never really useful. Oh we already have that.

[llogiq]

Yes, but that appears to only work if the element is an ExprPath for some reason.

[pitaj]

It appears that, actually, single_element_loop won't do anything with a straight array passed in. I've made some changes in #8616 that should cover that case. It also expands coverage to .iter_mut() and .into_iter(), and wraps in parens if necessary.

[pitaj]

We probably still want to cover the range case specially at some point, to at least give a better diagnostic.

[christophbeberweil]

Hi, @ThinkerDreamer and myself worked on a pull request #11862 that lints the array itself, if it contains exactly one range. In this case, the lint suggests that the user wanted to iterate over the single range instead and suggests to rewrite the for loop accordingly.

[teor2345]

I think we can call this closed by #11862.