Remove IntoIterator impl for &mut EventReader

[nicopap]

Objective

The latest clippy release has a much more aggressive application of the explicit_iter_loop pedantic lint.

As a result, clippy now suggests the following:

-for event in events.iter() {
+for event in &mut events {

I'm generally in favor of this lint. Using for mut item in &mut query is also recommended over for mut item in query.iter_mut() for good reasons IMO.

But, it is my personal belief that &mut events is much less clear than events.iter().

Why? The reason is that the events from EventReader are not mutable, they are immutable references to each event in the event reader. &mut events suggests we are getting mutable access to events — similarly to &mut query — which is not the case. Using &mut events is therefore misleading.

IntoIterator requires a mutable EventReader because it updates the internal last_event_count, not because it let you mutate it.

So clippy's suggested improvement is a downgrade.

Solution

Do not implement IntoIterator for &mut events.

Without the impl, clippy won't suggest its "fix". This also prevents generally people from using &mut events for iterating EventReaders, which makes the ecosystem every-so-slightly better.

---

Changelog

• Removed IntoIterator impl for &mut EventReader

Migration Guide

• &mut EventReader does not implement IntoIterator anymore. replace for foo in &mut events by for foo in events.iter()

[JoJoJet]

Agreed. I am also in favor of renaming .iter() to something clearer like .read(). Having a function named .iter() that modifies state has always rubbed me the wrong way.