clippy::manual_retain on self already borrowed as immutable

[alphastrata]

Summary

Just an incorrect suggestion? of a manual_retain when there's existing (perhaps non-trivial) borrows in the mix.

[rust playground minimal example] (https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=d8768d458b40ee2be9f055301ec61109)

Lint Name

manual_retain

Reproducer

I tried this code:

struct MyStruct {
    nums: Vec<usize>,
}

impl MyStruct {
    fn my_special_filter(&mut self) {
        let mut prunes: Vec<&usize> = Vec::new();

        let mut sorted: Vec<&usize> = self.nums.iter().collect();
        sorted.sort();

        sorted
            .iter()
            .filter(|num| **num % 2 == 0)
            .for_each(|v| prunes.push(v));

        self.nums = self
            .nums
            .iter()
            .filter(|gid| !prunes.contains(gid))
            .cloned()
            .collect();
    }

    // fn clippy_refactor(&mut self) {
    //     let mut prunes: Vec<&usize> = Vec::new();

    //     let mut sorted: Vec<&usize> = self.nums.iter().collect();
    //     sorted.sort();

    //     sorted
    //         .iter()
    //         .filter(|num| **num % 2 == 0)
    //         .for_each(|v| prunes.push(v));

    //     self.nums.retain(|num| !prunes.contains(&num))
    // }
}

fn main() {
    let mut ms = MyStruct {
        nums: (0..100).collect(),
    };

    ms.my_special_filter();

    assert_eq!(ms.nums.len(), 50);
}

I saw this happen:

warning: this expression can be written more simply using `.retain()`
  --> src/main.rs:18:9
   |
18 | /         self.nums = self
19 | |             .nums
20 | |             .iter()
21 | |             .filter(|gid| !prunes.contains(gid))
22 | |             .cloned()
23 | |             .collect();
   | |______________________^ help: consider calling `.retain()` instead: `self.nums.retain(|gid| !prunes.contains(gid))`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain
   = note: `#[warn(clippy::manual_retain)]` on by default

warning: `manual_retain_bug` (bin "manual_retain_bug") generated 2 warnings (run `cargo clippy --fix --bin "manual_retain_bug"` to apply 2 suggestions)

Which if you take the advice (see the commented out fn clippy_refactor() above, gives you:

    Checking manual_retain_bug v0.1.0 (/home/jer/Documents/rust/manual_retain_bug)
error[E0502]: cannot borrow `self.nums` as mutable because it is also borrowed as immutable
  --> src/main.rs:36:9
36 |         self.nums.retain(|num| !prunes.contains(&num))
   |         ^^^^^^^^^^------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |         |         |
   |         |         immutable borrow later used by call
   |         mutable borrow occurs here

For more information about this error, try `rustc --explain E0502`.
error: could not compile `manual_retain_bug` (bin "manual_retain_bug") due to previous error

I expected to see this happen: No lint in this instance.

Version

rustc 1.72.1 (d5c2e9c34 2023-09-13)
binary: rustc
commit-hash: d5c2e9c342b358556da91d61ed4133f6f50fc0c3
commit-date: 2023-09-13
host: x86_64-unknown-linux-gnu
release: 1.72.1
LLVM version: 16.0.5

Additional Labels

No response

[duckki]

I ran into a similar issue.

reproducer:

fn reproducer() {
    let mut list = vec![1, 2];
    list = list.iter().filter(|i| list.len() == **i).cloned().collect();
    println!("{:#?}", list);
}

clippy suggested:

fn reproducer_clippy_suggested() {
    let mut list = vec![1, 2];
    list.retain(|i| list.len() == **i); // original suggestion: wrong dereference level
    //list.retain(|i| list.len() == *i); // (after deref correction) also, fails due to borrow
    println!("{:#?}", list);
}

Two issues in the suggestion:

• The dereference level wasn't revised. Thus, leading to a compile error there.
• Even after deref was fixed, borrow check fails.

Clippy version: clippy 0.1.76 (07dca489 2024-02-04)