option_if_let_else suggests invalid code when using mut references in the else clause

[vbkaisetsu]

Summary

There is a conditional branching code that uses if let else for the return value of HashMap::get_mut().
Even though the else clause of this code uses a mutable reference of the HashMap, option_if_let_else suggests to use map_or_else(), but the suggested code causes an error in the borrow checker.

Lint Name

option_if_let_else

Reproducer

I tried this code:

// feature_ids is a HashMap.
if let Some(v) = feature_ids.get_mut(&fid) {
    *v += 1.0;
} else {
    feature_ids.insert(fid, 1.0);
}

I saw this happen:

    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#option_if_let_else
help: try
    |
150 ~                 feature_ids.get_mut(&fid).map_or_else(|| {
151 +                     feature_ids.insert(fid, 1.0);
152 +                 }, |v| {
153 +                     *v += 1.0;
154 +                 })

The suggested code is not compilable:

    |
150 |                 feature_ids.get_mut(&fid).map_or_else(|| {
    |                 ------------------------- ----------- ^^ second mutable borrow occurs here
    |                 |                         |
    |                 |                         first borrow later used by call
    |                 first mutable borrow occurs here
151 |                     feature_ids.insert(fid, 1.0);
    |                     ----------- second borrow occurs due to use of `feature_ids` in closure

Version

rustc 1.59.0-nightly (7abab1efb 2021-12-17)
binary: rustc
commit-hash: 7abab1efb21617ba6845fa86328dffa16cfcf1dc
commit-date: 2021-12-17
host: x86_64-apple-darwin
release: 1.59.0-nightly
LLVM version: 13.0.0

Additional Labels

No response

[giraffate]

The reproduction on the playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=0b7a467d0a5debdb45fbee8a74e34c81

[dswij]

@rustbot claim

[ghost]

A better way to do this is to use the entry API.

    *feature_ids.entry(&fid).or_default() += 1.0;

Maybe this is also a false negative (or a needed extension) of the map_entry lint.

[jpmckinney]

I have something similar like (map is defined outside a loop, expected and key can change with each loop iteration):

if let Some(old) = map.get(&key) {
    if *old != current {
        warn!("Relevant message");
    }
} else {
    map.insert(key, current);
}

I can rewrite this as:

if *map.entry(key).or_insert(current) != current {
    warn!("Relevant message");
}

But now I'm always doing a comparison, even when the entry didn't exist.