Add a lint for unbound mutex guards

[IvanUkhov]

What it does

When working with various synchronization primitives, such as Mutex and RwLock, it is easy to make the mistake of not binding the corresponding guard to the current scope. It would be helpful to assist.

Advantage

No response

Drawbacks

No response

Example

struct Worker {
    mutex: std::sync::Arc<std::sync::Mutex<()>>,
}

impl Worker {
    fn work(&self) {
        let _ = self.mutex.lock().unwrap();
        // Do some work with an external resource.
    }
}

struct AsyncWorker {
    mutex: std::sync::Arc<tokio::sync::Mutex<()>>,
}

impl AsyncWorker {
    async fn work(&self) {
        let _ = self.mutex.lock().await;
        // Do some work with an external resource.
    }
}

Could be written as:

struct Worker {
    mutex: std::sync::Arc<std::sync::Mutex<()>>,
}

impl Worker {
    fn work(&self) {
        let _guard = self.mutex.lock().unwrap();
        // Do some work with an external resource.
    }
}

struct AsyncWorker {
    mutex: std::sync::Arc<tokio::sync::Mutex<()>>,
}

impl AsyncWorker {
    async fn work(&self) {
        let _guard = self.mutex.lock().await;
        // Do some work with an external resource.
    }
}

[y21]

This lint exists and emits errors on the reproducer you shared:

error: non-binding let on a synchronization lock
 --> src/lib.rs:7:13
  |
7 |         let _ = self.mutex.lock().unwrap();
  |             ^ this lock is not assigned to a binding and is immediately dropped
  |
  = note: `#[deny(let_underscore_lock)]` on by default
help: consider binding to an unused variable to avoid immediately dropping the value
  |
7 |         let _unused = self.mutex.lock().unwrap();
  |             ~~~~~~~
help: consider immediately dropping the value
  |
7 |         drop(self.mutex.lock().unwrap());
  |         ~~~~~                          +

[y21]

Ah, sorry; that error snippet from me is even a builtin compiler lint. The clippy lint is for external mutexes like the one in parking_lot which the compiler lint doesn't catch.

[IvanUkhov]

Oh, you are right that the one with std::sync is already caught by the compiler! I added it for completeness. Originally, I stumbled upon this problem for tokio::sync, and clippy did not say anything. Do you mean it should work for Tokio?

[y21]

Ah, I'd overlooked that your reproducer also tested it for tokio::sync::Mutex. Yeah, clippy currently only looks for lock_api::MutexGuard (used by parking_lot), but not tokio. I suppose we could also add support for tokio's MutexGuard (although the current lint implementation hardcodes external crate paths which we usually try to avoid because internal changes in the crate can easily break clippy)

[IvanUkhov]

Thank you. I just want to emphasize that it is not only Mutex but also the other primitives, such as RwLock.