Can't ignore clippy::borrow_deref_ref

[tokatoka]

Summary

We have this function in our crate https://github.com/AFLplusplus/LibAFL/tree/main/libafl
https://github.com/AFLplusplus/LibAFL/blob/main/libafl/src/observers/mod.rs#L816

        #[pyo3(name = "match_name")]
        #[allow(clippy::all)]
        fn pymatch_name(&self, name: &str) -> Option<PythonObserver> {
            for ob in &self.list {
                if *ob.name() == *name {
                    return Some(ob.clone());
                }
            }
            None
        }

and clippy gives this warning

error: deref on an immutable reference
   --> libafl/src/observers/mod.rs:817:38
    |
817 |         fn pymatch_name(&self, name: &str) -> Option<PythonObserver> {
    |                                      ^^^^ help: if you would like to reborrow, try removing `&*`: `&str`
    |
note: the lint level is defined here
   --> libafl/src/lib.rs:14:9
    |
14  | #![deny(clippy::all)]
    |         ^^^^^^^^^^^
    = note: `#[deny(clippy::borrow_deref_ref)]` implied by `#[deny(clippy::all)]`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#borrow_deref_ref

We actually have allow(clippy::all) on this function, but still the clippy gives this error and it's not ignored.

Lint Name

borrow_deref_ref

Reproducer

You can clone the repo https://github.com/AFLplusplus/LibAFL/tree/main/libafl
and checkout to debug_restarting branch
and run

RUST_BACKTRACE=full cargo +nightly clippy --all --all-features --release --tests -- -Z macro-backtrace    -D clippy::all    -D clippy::pedantic    -W clippy::similar_names    -A clippy::type_repetition_in_bounds    -A clippy::missing-errors-doc    -A clippy::cast-possible-truncation    -A clippy::used-underscore-binding    -A clippy::ptr-as-ptr    -A clippy::missing-panics-doc    -A clippy::missing-docs-in-private-items    -A clippy::unseparated-literal-suffix    -A clippy::module-name-repetitions    -A clippy::unreadable-literal

or simply

./script/clippy.sh

Version

rustc 1.63.0-nightly (5435ed691 2022-06-07)
binary: rustc
commit-hash: 5435ed6916a59e8d5acba2149316a841c3905cbd
commit-date: 2022-06-07
host: x86_64-unknown-linux-gnu
release: 1.63.0-nightly
LLVM version: 14.0.5

Additional Labels

No response

[andreafioraldi]

Related to #7930 I guess

[Jarcho]

Fixing this is waiting on #8694.

In the meantime you can allow at the module or crate level.

[xFrednet]

rust-lang/rust#97660 is also related to this, as the lint is currently not emitted at the correct location.

[tgross35]

Is there a reason this lint tends to be emitted with pyO3 projects? I am running in to the same thing on a python crate and scratching my head a bit

Even simpler for me, I don't even use the * operator

#[pyfunction]
#[inline]
fn levenshtein(a: &str, b: &str, limit: Option<u32>) -> u32 {
    match limit {
        Some(v) => algorithms::levenshtein_limit(a, b, v),
        None => algorithms::levenshtein(a, b),
    }
}

[xFrednet]

That is most likely a problem originating from the #[pyfunction] macro, we have a pr to improve detection of these. Hopefully, this case should be covered by that PR #8694 🙃

[Rikorose]

I can still confirm on latest rust 1.63.0 stable clippy that fucntions implmented within a #[pymethods] impl still result in this lint:

#[pymethods]
impl _FdDataLoader {
  // ...
    fn start_epoch(&mut self, split: &str, seed: usize) -> PyResult<()> {
      // ...
    }
}

warning: deref on an immutable reference
   --> pyDF-data/src/lib.rs:221:38
    |
221 |     fn start_epoch(&mut self, split: &str, seed: usize) -> PyResult<()> {
    |                                      ^^^^ help: if you would like to reborrow, try removing `&*`: `&str`
    |
    = note: `#[warn(clippy::borrow_deref_ref)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#borrow_deref_ref

[xFrednet]

The PR #8694 took a longer time to review. It should hit stable in two releases, so by 1.65. The lint mentioned didn't get an extra check, though. Adding it should be simple with the new util functions from of the PR. (See is_from_proc_macro) 🙃

@rustbot label +good-first-issue

[lochetti]

Hi! I am trying to simulate the clippy warning here because I want to try to solve this issue. But I am having hard time to reproduce the lint warn.

Can you please help me out?

The code sample that I am using to try to reproduce is:

use pyo3::prelude::*;

fn main() {
    concat_strings("foo", "bar").ok();
}

#[pyfunction]
#[pyo3(name = "concat_strings")]
fn concat_strings(a: &str, b: &str) -> PyResult<String> {
    Ok(format!("{a}{b}"))
}

Just want to be sure if this snippet should trigger the lint warn or not. If not, can you try to provide another small snippet that triggers the warning?

PS: I am using pyo3 v0.18.3

[xFrednet]

I would guess that the triggering code has been fixed in pyfunction. However, the problem of proc macros still exists. As suggested in my previous comment, a check with is_from_proc_macro should be added to the lint. We have some lints that use that already and some tests macros which can be used :)

[lochetti]

Hey @xFrednet thanks for pointing out! I have added similar check to another lint, so I guess that I will be able to add here as well. I just wanted to try to reproduce the problem with pyo3 before, to make sure that the is_from_proc_macro check would fix the "original issue" :)

Anyway, I will continue here with the is_from_proc_macro approach.
Thanks again :)

@rustbot claim