New Lint: temporary lock guards in if let / while let / match scrutinees

What it does

Recently, I am coding on some rust projects and noticed an easy mistake: a temporary lock guard can be held longer than expected when it is created inside the scrutinee of if let, while let, or match. This pattern is easy to run into when working with collections using methods like pop().

I also did a quick pattern search on GitHub and found several similar cases in real code, so this seems to be a practical footgun.

I’d be happy to work on a PR if this sounds like a good fit for Clippy.

What it does

Warns when a temporary lock guard created in the scrutinee of if let, while let, or match
causes the lock to be held for longer than it appears.

This is a common pitfall: the lock is acquired in an expression that visually looks small and local, yet the actual drop scope spans the entire construct. The 2024 Edition partially mitigates this by shortening the drop scope of temporary values, but the issue can still arise in the if body.

Example

if let

if let Some(item) = list.lock().unwrap().pop() {
    process_item(item);
}

This could be written as:

let item = {
    let mut guard = list.lock().unwrap();
    guard.pop()
};

if let Some(item) = item {
    process_item(item);
}

while let

while let Some(item) = list.lock().unwrap().pop() {
    process_item(item);
}

This could be written as:

while let Some(item) = {
    let mut guard = list.lock().unwrap();
    guard.pop()
} {
    process_item(item);
}

match

match list.lock().unwrap().pop() {
    Some(item) => process_item(item),
    None => {}
}

This could be written as:

let item = {
    let mut guard = list.lock().unwrap();
    guard.pop()
};

match item {
    Some(item) => process_item(item),
    None => {}
}

It also applies to collections like:

• Vec with pop()
• VecDeque with pop_front() / pop_back()
• BinaryHeap with pop()
• BTreeSet with pop_first() / pop_last()
• ...

So in all of these cases, the returned value is owned, so the guard usually does not need to remain alive that long.

By contrast, if the method returns a reference, like front(), then the guard must stay alive, so that is a different case and should usually not be linted.

Why is this bad?

Temporary values created inside larger expressions are dropped at the end of the statement.
For lock guards, this can mean that the mutex or rwlock remains held during the body of an
if let, while let, or match, which is easy to overlook and can lead to performance problems, unnecessary lock contention, or deadlocks in worse cases.

Category

nursery or suspicious

Suggested implementation

• Target if let, while let, and match
• Only lint known lock guard types first:
  • std::sync::MutexGuard
  • std::sync::RwLockReadGuard
  • std::sync::RwLockWriteGuard

If people think this is reasonable, I’d be glad to take it on and open a PR. Feedback on the implementation, category, and naming is very welcome.

Advantage

Makes lock scope explicit, improve the performance and helps prevent a subtle class of lock-holding bugs.

Drawbacks

May have some false positives, not sure yet.
But this seems like a good fit for nursery or suspicious first.

Example

if let

if let Some(item) = list.lock().unwrap().pop() {
    process_item(item);
}

This could be written as:

let item = {
    let mut guard = list.lock().unwrap();
    guard.pop()
};

if let Some(item) = item {
    process_item(item);
}

while let

while let Some(item) = list.lock().unwrap().pop() {
    process_item(item);
}

This could be written as:

loop {
    let item = {
        let mut guard = list.lock().unwrap();
        guard.pop()
    };

    let Some(item) = item else {
        break;
    };

    process_item(item);
}

match

match list.lock().unwrap().pop() {
    Some(item) => process_item(item),
    None => {}
}

This could be written as:

let item = {
    let mut guard = list.lock().unwrap();
    guard.pop()
};

match item {
    Some(item) => process_item(item),
    None => {}
}

Comparison with existing lints

This seems related to #11773 and #5219 (clippy::if_let_mutex), but those cases do not cover while let or match. Also, my proposed lint is especially relevant for collections-processing code such as pop()-based examples above.

Additional Context

To some extent, this kind of issue can easily arise when using collections, since they make the code more simple. It is natural to take the first value returned by pop and use it directly in the scrutinee expression.

Start by reviewing the existing clippy::if_let_mutex lint and related issues #11773 and #5219, then assess how the proposed lint should distinguish owned results from references. Define coverage for if let, while let, and match with the listed standard lock guards, and verify that collection methods such as pop() are warned on without introducing the noted false positives.