Bad suggestion when implicit loop .iter() should be made mutable

[edmorley]

Given the following code:
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=b3ca40d8d38fae14b7bab120a85b2b8a
(From AoC, in case it wasn't obvious 😆)

fn winning_losing_game_scores(bingo_game: BingoGame) -> (Option<u64>, Option<u64>) {
    let mut scores = Vec::new();

    for number in bingo_game.numbers_to_be_drawn {
        for card in bingo_game.cards {
            if card.still_playing && card.mark_number(number) {
                // (removed)
            }
        }
    }

    let winning_score = scores.first();
    let losing_score = scores.last();
    (winning_score.copied(), losing_score.copied())
}

struct BingoGame {
    numbers_to_be_drawn: Vec<u64>,
    cards: Vec<BingoCard>,
}

struct BingoCard {
    still_playing: bool,
    // (removed)
}

impl BingoCard {
    fn mark_number(&mut self, _number: u64) -> bool {
        // (removed)
        false
    }
}

The current output is:

error[[E0382]](https://doc.rust-lang.org/nightly/error-index.html#E0382): use of moved value: `bingo_game.cards`
   [--> src/lib.rs:5:21
](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=eda0bc06b2047145bfbda74c962a5e91#)    |
5   |         for card in bingo_game.cards {
    |                     ^^^^^^^^^^^^^^^^
    |                     |
    |                     `bingo_game.cards` moved due to this implicit call to `.into_iter()`, in previous iteration of loop
    |                     help: consider borrowing to avoid moving into the for loop: `&bingo_game.cards`
    |
note: this function takes ownership of the receiver `self`, which moves `bingo_game.cards`
    = note: move occurs because `bingo_game.cards` has type `Vec<BingoCard>`, which does not implement the `Copy` trait

error[[E0596]](https://doc.rust-lang.org/nightly/error-index.html#E0596): cannot borrow `card` as mutable, as it is not declared as mutable
 [--> src/lib.rs:6:38
](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=eda0bc06b2047145bfbda74c962a5e91#)  |
5 |         for card in bingo_game.cards {
  |             ---- help: consider changing this to be mutable: `mut card`
6 |             if card.still_playing && card.mark_number(number) {
  |                                      ^^^^^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable

Ideally the output should look like:

error[[E0596]](https://doc.rust-lang.org/nightly/error-index.html#E0596): cannot borrow `card` as mutable, as it is not declared as mutable
 [--> src/lib.rs:6:38
](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=eda0bc06b2047145bfbda74c962a5e91#)  |
5 |         for card in bingo_game.cards {
  |                     ---------------- help: help: consider changing this to: `&mut bingo_game.cards`
6 |             if card.still_playing && card.mark_number(number) {
  |                                      ^^^^^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable

ie: To suggest changing the implicit loop iteration for card in bingo_game.cards to either for card in &mut bingo_game.cards or else for card in bingo_game.cards.iter_mut().

CC #62387, #82081. (I've filed a new issue since the #62387 uses closures, and #82081 is about E0594 not E0596)

[TrolledWoods]

In this case the compiler suggestion would compile just fine, so I'm a bit unsure whether suggesting &mut item here is the right thing, since you may want to consume the item later in the loop body.

[edmorley]

@TrolledWoods Thank you for taking a look. It seems I over-reduced the testcase, since the original did not compile when following the compiler suggestion. I've updated the issue description with a less reduced testcase that still shows the issue.

[estebank]

I think the right change would be for the following

error[E0596]: cannot borrow `*card` as mutable, as it is behind a `&` reference
 --> src/lib.rs:6:38
  |
5 |         for card in &bingo_game.cards {
  |                     ----------------- this iterator yields `&` references
6 |             if card.still_playing && card.mark_number(number) {
  |                                      ^^^^^^^^^^^^^^^^^^^^^^^^ `card` is a `&` reference, so the data it refers to cannot be borrowed as mutable

For more information about this error, try `rustc --explain E0596`.

to suggest iterating on &mut bingo_game.cards instead.

[estebank]

Current output, no real change:

error[E0382]: use of moved value: `bingo_game.cards`
   --> src/lib.rs:5:21
    |
5   |         for card in bingo_game.cards {
    |                     ^^^^^^^^^^^^^^^^ `bingo_game.cards` moved due to this implicit call to `.into_iter()`, in previous iteration of loop
    |
note: `into_iter` takes ownership of the receiver `self`, which moves `bingo_game.cards`
   --> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/iter/traits/collect.rs:346:18
    |
346 |     fn into_iter(self) -> Self::IntoIter;
    |                  ^^^^
    = note: move occurs because `bingo_game.cards` has type `Vec<BingoCard>`, which does not implement the `Copy` trait
help: consider iterating over a slice of the `Vec<BingoCard>`'s content to avoid moving into the `for` loop
    |
5   |         for card in &bingo_game.cards {
    |                     +

error[E0596]: cannot borrow `card` as mutable, as it is not declared as mutable
 --> src/lib.rs:6:38
  |
6 |             if card.still_playing && card.mark_number(number) {
  |                                      ^^^^ cannot borrow as mutable
  |
help: consider changing this to be mutable
  |
5 |         for mut card in bingo_game.cards {
  |             +++

If we follow the suggestions:

error[E0596]: cannot borrow `*card` as mutable, as it is behind a `&` reference
 --> src/lib.rs:6:38
  |
5 |         for mut card in &bingo_game.cards {
  |                         ----------------- this iterator yields `&` references
6 |             if card.still_playing && card.mark_number(number) {
  |                                      ^^^^ `card` is a `&` reference, so the data it refers to cannot be borrowed as mutable
  |
help: use a mutable iterator instead
  |
5 |         for mut card in &mut bingo_game.cards {
  |                          +++

error[E0596]: cannot borrow `bingo_game.cards` as mutable, as `bingo_game` is not declared as mutable
 --> src/lib.rs:5:25
  |
5 |         for mut card in &mut bingo_game.cards {
  |                         ^^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable
  |
help: consider changing this to be mutable
  |
1 | fn winning_losing_game_scores(mut bingo_game: BingoGame) -> (Option<u64>, Option<u64>) {
  |                               +++

we eventually get to code that compiles.