Issues with `readonly_write_lock`

[paologallinaharbur]

Summary

We had our pipelines failing due to clippy::readonly_write_lock

We use read write locks, and maybe we are using it in a wrong way

Lint Name

readonly_write_lock

Reproducer

I tried this code with @sigilioso:

pub struct MyStruct {
    [...]
    rw_lock: RwLock<()>,
}
[...]
impl MyStruct{
    fn get() -> bool
    {
        let _read_guard = self.rw_lock.read().unwrap();
        [...]
        // reading data from filesystem
    }
   fn put() -> bool
    {
        let _write_guard = self.rw_lock.write().unwrap();
        [...]
        // writing data from filesystem
    }

I saw this happen:

(help: consider using a read lock instead: `self.rw_lock.read())

I do not expect any error, since we are using the rw_lock not to lock a specific piece of data, but a piece of code through _read_guard/_write_guard.

For this reason having a lock as RwLock<()> would always provide a similar error, since there is nothing to write

Version

rustc 1.79.0-beta.1 (6b544f5ff 2024-04-28)

Additional Labels

No response

[paologallinaharbur]

Possibly related to #12479 created by @y21

[y21]

Yeah, I suppose we could suppress the lint when the binding is prefixed with _ or for RwLock<()> specifically, but other than that I'm not sure if there's a good, general fix to this kind of false positive.
The core issue of "a lock that doesn't protect data but a critical section of code" seems hard to detect and directly conflicts with the fundamental idea of the lint.
(FWIW, I think a more idiomatic way to express this which wouldn't run into this false positive would be to model the resource as a struct and then have the operation as a method taking &mut self so that calling it requires a write lock)

[paologallinaharbur]

(FWIW, I think a more idiomatic way to express this which wouldn't run into this false positive would be to model the resource as a struct and then have the operation as a method taking &mut self so that calling it requires a write lock)

We have been discussing the same internally, however, it is difficult to model it and it would require a lot of code to do something that is a bit weird -> having a method mutable that is actually not muting anything (that would trigger a different linter)

IMO it makes sense to silent the alert if there is a _ before!