rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.27k stars 1.52k forks source link

mut qualifier removal causes compilation error #11794

Open matzipan opened 10 months ago

matzipan commented 10 months ago

Summary

Clippy produced the following message:

after fixes were automatically applied the compiler reported errors within these files:

  * app/src/backends/imap/sync.rs
  * app/src/models/identity.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see 
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust-clippy/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

The following errors were reported:
warning: variable does not need to be mutable
   --> app/src/backends/imap/sync.rs:139:9
    |
139 |     let mut iterator = UidFetchIterator::new(uid_range_start, uid_range_end);
    |         ----^^^^^^^^
    |         |
    |         help: remove this `mut`
    |
    = note: `#[warn(unused_mut)]` on by default

error[E0382]: use of moved value: `new_messages`
   --> app/src/models/identity.rs:410:12
    |
356 |         let (new_uid_validity, mut new_messages, flag_updates) = self
    |                                ---------------- move occurs because `new_messages` has type `std::vec::Vec<models::database::NewMessage>`, which does not implement the `Copy` trait
...
396 |                     Some(new_messages)
    |                          ------------ value moved here
...
410 |         Ok(new_messages)
    |            ^^^^^^^^^^^^ value used here after move

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0382`.
Original diagnostics will follow.

Removing the two mut qualifiers produces:

error[E0596]: cannot borrow `iterator` as mutable, as it is not declared as mutable
   --> app/src/backends/imap/sync.rs:141:60
    |
141 |     while let Some((fetch_range_start, fetch_range_end)) = iterator.next() {
    |                                                            ^^^^^^^^ cannot borrow as mutable
    |
help: consider changing this to be mutable
    |
139 |     let mut iterator = UidFetchIterator::new(uid_range_start, uid_range_end);
    |         +++

and

error[E0596]: cannot borrow `new_messages` as mutable, as it is not declared as mutable
   --> app/src/models/identity.rs:356:32
    |
356 | ...   let (new_uid_validity, new_messages, flag_updates) = self
    |                              ^^^^^^^^^^^^ not mutable
...
368 | ...               .store_messages_for_folder(&mut new_messages, folder, services::StoreType::Fresh { new_uid_validity })?;
    |                                              ----------------- cannot borrow as mutable
...
394 | ...                   .store_messages_for_folder(&mut new_messages, folder, services::StoreType::Incremental)?;
    |                                                  ----------------- cannot borrow as mutable
...
401 | ...                   .store_messages_for_folder(&mut new_messages, folder, services::StoreType::Fresh { new_uid_validity ...
    |                                                  ----------------- cannot borrow as mutable
    |
help: consider changing this to be mutable
    |
356 |         let (new_uid_validity, mut new_messages, flag_updates) = self
    |                                +++

Reproducer

The code in question:

Version

rustc 1.73.0 (cc66ad468 2023-10-03)
bash-5.2$ rustc -Vv
rustc 1.73.0 (cc66ad468 2023-10-03)
binary: rustc
commit-hash: cc66ad468955717ab92600c770da8c1601a4ff33
commit-date: 2023-10-03
host: x86_64-unknown-linux-gnu
release: 1.73.0
LLVM version: 17.0.2

Additional Labels

@rustbot label +I-suggestion-causes-error

matthri commented 10 months ago

Ok if I see this correctly there are multiple things going on. Since it seems you want to remove the unused_mut warning for your iterator let's look at this.

In your first code block the mut is indeed not needed.

    let mut iterator = UidFetchIterator::new(uid_range_start, uid_range_end);

    for (fetch_range_start, fetch_range_end) in iterator {
        let mut messages = connection

If I get it correctly the for-loop will use the IntoIterator Trait to convert your iterator and therefore your iterator variable does not need to be mutable. See also here for the desugaring of the for-loop.

In your second example you call the next() method directly on you iterator.

error[E0596]: cannot borrow `iterator` as mutable, as it is not declared as mutable
   --> app/src/backends/imap/sync.rs:141:60
    |
141 |     while let Some((fetch_range_start, fetch_range_end)) = iterator.next() {
    |                                                            ^^^^^^^^ cannot borrow as mutable
    |
help: consider changing this to be mutable
    |
139 |     let mut iterator = UidFetchIterator::new(uid_range_start, uid_range_end);
    |         +++

This produces an error since the next() method takes a mutable reference to self and therefore your iterator variable needs to be marked as mutable.

If i see it correctly the remaining errors are decoupled from the mut problematic and the issue there is that you are trying to use something that got moved out of scope.

I hope this helps a little bit, if you have any questions feel free to ask.

matzipan commented 10 months ago

Hi,

Thanks for the prompt response. It is indeed so. I think your suggestion matches the code that I eventually pushed: https://github.com/matzipan/envoyer/commit/ca27349ad56215837085a80ef376da3ad9038c1b

But I don't want this ticket to be misunderstood. It's not a support ticket from my side. I just opened it because the error message told me so.

If there's nothing to be fixed here in clopot, feel free to close.