Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the clippy::explicit_iter_loop lint #9834

Merged
merged 5 commits into from
Sep 19, 2023

Conversation

JoJoJet
Copy link
Member

@JoJoJet JoJoJet commented Sep 18, 2023

Objective

Replace instances of

for x in collection.iter{_mut}() {

with

for x in &{mut} collection {

This also changes CI to no longer suppress this lint. Note that since this lint only shows up when using clippy in pedantic mode, it was probably unnecessary to suppress this lint in the first place.

@JoJoJet JoJoJet added the C-Code-Quality A section of code that is hard to understand or change label Sep 18, 2023
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Sep 18, 2023
@alice-i-cecile
Copy link
Member

Once you fix merge conflicts I'll merge :p

@rparrett
Copy link
Contributor

rparrett commented Sep 18, 2023

This also changes CI to no longer suppress this lint. Note that since this lint only shows up when using clippy in pedantic mode, it was probably unnecessary to suppress this lint in the first place.

I was very confused for a second, but we used to use -Wclippy::explicit_iter_loop in CI, and when I updated for rust 1.72 I just changed that to -A when I could have simply removed it.

Do we want to add that back in now? iirc this should be possible now after #9583 (and this PR's changes).

@JoJoJet
Copy link
Member Author

JoJoJet commented Sep 18, 2023

Sure I can add the warning back -- seems reasonable.

@JoJoJet
Copy link
Member Author

JoJoJet commented Sep 19, 2023

On second thought, let's not add the warning yet. This suggestion is makes it worse.

373 |         for (access, _) in self.0.iter() {
    |                            ^^^^^^^^^^^^^ help: to write this more concisely, try: `&*self.0`

Seems like the suggestion will be fixed in the next clippy release (rust-lang/rust-clippy#11074), so we can enable the warning then.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Sep 19, 2023
Merged via the queue into bevyengine:main with commit d5d355a Sep 19, 2023
21 checks passed
@JoJoJet JoJoJet deleted the explicit_iter_loop branch September 19, 2023 05:55
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
# Objective

Replace instances of

```rust
for x in collection.iter{_mut}() {
```

with

```rust
for x in &{mut} collection {
```

This also changes CI to no longer suppress this lint. Note that since
this lint only shows up when using clippy in pedantic mode, it was
probably unnecessary to suppress this lint in the first place.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Code-Quality A section of code that is hard to understand or change S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants