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.46k stars 1.54k forks source link

`map_entry` lint suggested by clippy gives compilation error when key is also used in else block #13306

Open prateekkumarweb opened 2 months ago

prateekkumarweb commented 2 months ago

Summary

Clippy gives incorrect suggestion of entry api when using contains_key followed by insert. The suggestion given causes compilation error because of key being used by else block which has already got moved when entry method is called in the if condition.

Reproducer

When running clippy with the following input:

use std::collections::HashMap;

#[derive(Debug)]
struct Env {
    enclosing: Option<Box<Env>>,
    values: HashMap<String, usize>,
}

impl Env {
    fn new(enclosing: Option<Box<Env>>) -> Self {
        Self {
            enclosing,
            values: HashMap::new(),
        }
    }

    fn assign(&mut self, name: String, value: usize) -> bool {
        if self.values.contains_key(&name) {
            self.values.insert(name, value);
            true
        } else if let Some(enclosing) = &mut self.enclosing {
            enclosing.assign(name, value)
        } else {
            false
        }
    }
}

fn main() {
    let mut env = Env::new(None);
    env.assign("x".to_string(), 10);
    dbg!(&env);
}

gives:

warning: usage of `contains_key` followed by `insert` on a `HashMap`
  --> src/main.rs:18:9
   |
18 | /         if self.values.contains_key(&name) {
19 | |             self.values.insert(name, value);
20 | |             true
21 | |         } else if let Some(enclosing) = &mut self.enclosing {
...  |
24 | |             false
25 | |         }
   | |_________^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_entry
   = note: `#[warn(clippy::map_entry)]` on by default
help: try
   |
18 ~         if let std::collections::hash_map::Entry::Occupied(mut e) = self.values.entry(name) {
19 +             e.insert(value);
20 +             true
21 +         } else if let Some(enclosing) = &mut self.enclosing {
22 +             enclosing.assign(name, value)
23 +         } else {
24 +             false
25 +         }
   |

Accepting this suggestion using --fix gives error.

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

  * src/main.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:
error[E0382]: use of moved value: `name`
  --> src/main.rs:22:30
   |
17 |     fn assign(&mut self, name: String, value: usize) -> bool {
   |                          ---- move occurs because `name` has type `std::string::String`, which does not implement the `Copy` trait
18 |         if let std::collections::hash_map::Entry::Occupied(mut e) = self.values.entry(name) {
   |                                                                                       ---- value moved here
...
22 |             enclosing.assign(name, value)
   |                              ^^^^ value used here after move

error: aborting due to 1 previous error

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

Version

rustc 1.80.1 (3f5fd8dd4 2024-08-06)
binary: rustc
commit-hash: 3f5fd8dd41153bc5fdca9427e9e05be2c767ba23
commit-date: 2024-08-06
host: aarch64-apple-darwin
release: 1.80.1
LLVM version: 18.1.7

Additional Labels

No response

alex-semenyuk commented 2 months ago

@rustbot label I-suggestion-causes-error