Skip to content

Commit

Permalink
Remove IntoIterator impl for &mut EventReader (#9583)
Browse files Browse the repository at this point in the history
# Objective

The latest `clippy` release has a much more aggressive application of
the
[`explicit_iter_loop`](https://rust-lang.github.io/rust-clippy/master/index.html#/explicit_into_iter_loop?groups=pedantic)
pedantic lint.

As a result, clippy now suggests the following:

```diff
-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 `EventReader`s,
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()`
  • Loading branch information
nicopap authored Aug 29, 2023
1 parent da9a070 commit 4f212a5
Showing 1 changed file with 0 additions and 8 deletions.
8 changes: 0 additions & 8 deletions crates/bevy_ecs/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,14 +453,6 @@ impl<'w, 's, E: Event> EventReader<'w, 's, E> {
}
}

impl<'a, 'w, 's, E: Event> IntoIterator for &'a mut EventReader<'w, 's, E> {
type Item = &'a E;
type IntoIter = EventIterator<'a, E>;
fn into_iter(self) -> Self::IntoIter {
self.iter()
}
}

/// Sends events of type `T`.
///
/// # Usage
Expand Down

0 comments on commit 4f212a5

Please sign in to comment.