Clippy fix for "`contains_key` followed by `insert`" produces "borrow of moved value" error

[fishrockz]

When running cargo clippy --fix

[will@localhost girderstream]$ cargo clippy --fix
   Compiling girderstream v0.1.0 (/home/will/projects/rust/buildsystems/girderstream)
warning: failed to automatically apply fixes suggested by rustc to crate `girderstream`

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

  * src/project/project.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/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:
error[E0382]: borrow of moved value: `junctioned_provider`
   --> src/project/project.rs:185:54
    |
163 |         for (junctioned_provider, junctioned_element) in junctioned_project.provides {
    |              ------------------- move occurs because `junctioned_provider` has type `std::string::String`, which does not implement the `Copy` trait
...
182 |             if let std::collections::hash_map::Entry::Vacant(e) = self.provides.entry(junctioned_provider) {
    |                                                                                       ------------------- value moved here
...
185 |                 panic!("Cant have two elements with {junctioned_provider} even through a junction");
    |                                                      ^^^^^^^^^^^^^^^^^^^ value borrowed here after move
    |
    = note: this error originates in the macro `$crate::const_format_args` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

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

warning: usage of `contains_key` followed by `insert` on a `HashMap`
   --> src/project/project.rs:182:13
    |
182 | /             if self.provides.contains_key(&junctioned_provider) {
183 | |                 panic!("Cant have two elements with {junctioned_provider} even through a junction");
184 | |             } else {
185 | |                 self.provides
186 | |                     .insert(junctioned_provider, junctioned_element);
187 | |             }
    | |_____________^
    |
    = note: `#[warn(clippy::map_entry)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_entry
help: try this
    |
182 ~             if let std::collections::hash_map::Entry::Vacant(e) = self.provides.entry(junctioned_provider) {
183 +                 e.insert(junctioned_element);
184 +             } else {
185 +                 panic!("Cant have two elements with {junctioned_provider} even through a junction");
186 +             }
    |

warning: `girderstream` (lib test) generated 1 warning
warning: `girderstream` (lib) generated 1 warning (1 duplicate)
    Finished dev [unoptimized + debuginfo] target(s) in 21.37s

The code fails because I use the thing I test for in the hash map in the else. This seems fairly sensible so I am surprised this wasn't picked up already. Maybe I'm missing something silly.

Ether way the output suggested creating a issue so I have done

Meta

rustc --version --verbose:

rustc 1.64.0 (a55dd71d5 2022-09-19)
binary: rustc
commit-hash: a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52
commit-date: 2022-09-19
host: x86_64-unknown-linux-gnu
release: 1.64.0
LLVM version: 14.0.6

To reproduce clone https://gitlab.com/girderstream/girderstream/-/commit/edc55aa923ad4fdb51e4494998c4b681b6fcb260 and run cargo clippy --fix

This is the diff clippy suggests with cargo clippy --fix --broken-code https://gitlab.com/girderstream/girderstream/-/commit/f7cbc3b097d859805d1f27b56b0973fb3004e963

which can be fixed with https://gitlab.com/girderstream/girderstream/-/commit/83422f677efb4eba8eb7a54da5cecc6022992d38 but is not always a good idea to add a clone.

Its a shame you cant pass in a borrow to HashMap.entry()

[claraphyll]

This issue still exists. Maybe map_entry should only be applied when the key type is Copy.

[jmquigs]

The following simple program reproduces this in rustc 1.80.0-nightly (804421dff 2024-06-07)

use std::collections::HashMap;

fn main() {
    let mut hm:HashMap<String,bool> = HashMap::new();
    let key = "hello".to_string();
    if hm.contains_key(&key) {
        let bval = hm.get_mut(&key).unwrap();
        *bval = false;
    } else {
        hm.insert(key, true);
    }
}

causing this error after fix:

error[E0382]: borrow of moved value: `key`
 --> src/main.rs:9:31
  |
5 |     let key = "hello".to_string();
  |         --- move occurs because `key` has type `std::string::String`, which does not implement the `Copy` trait      
6 |     if let std::collections::hash_map::Entry::Vacant(e) = hm.entry(key) {
  |                                                                    --- value moved here
...
9 |         let bval = hm.get_mut(&key).unwrap();
  |                               ^^^^ value borrowed here after move

error: aborting due to 1 previous error

As @maxi0604 suggested, perhaps if the key is not Copy the lint could simply warn about this but not attempt a fix. An autofix does not seem simple because the get_mut line is now invalid - the key would need to have been cloned in the call to entry for that to work.