rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.44k stars 1.54k forks source link

False positive: useless_asref when used to go from `&&T` to `&T` #12785

Open mqudsi opened 6 months ago

mqudsi commented 6 months ago

Summary

Normally, &&T will automatically and silently behave like &T and using foo.map(|t| t.as_ref()) is unneeded. However the useless_asref lint has a false positive in the case where types need to line up, e.g. the return types of if and else, the arms of match brackets, or in this case, Option<T> and the result of Option::unwrap_or(...).

Lint Name

useless_asref

Reproducer

I tried this code:

    pub fn has_libraries(&self, libraries: &[&str]) -> bool {
        let stub = libraries
            .first()
            .map(|l| l.as_ref())
            .unwrap_or("has_libraries");
        // ...
    }

I saw this happen:

warning: this call to `as_ref` does nothing
   --> src/lib.rs:544:22
    |
544 |             .map(|l| l.as_ref())
    |                      ^^^^^^^^^^ help: try: `l`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_asref
    = note: `#[warn(clippy::useless_asref)]` on by default

I expected to see this happen:

The useless_asref lint is incorrect here since without the call rustc (1.80.0-nightly) will complain about mismatched types:

error[E0308]: mismatched types
   --> src/lib.rs:545:24
    |
545 |             .unwrap_or("has_libraries");
    |              --------- ^^^^^^^^^^^^^^^ expected `&&str`, found `&str`
    |              |
    |              arguments to this method are incorrect
    |
    = note: expected reference `&&_`
               found reference `&'static _`

Version

rustc 1.80.0-nightly (faefc618c 2024-05-07)
binary: rustc
commit-hash: faefc618cf48bd794cbc808448df1bf3f59f36af
commit-date: 2024-05-07
host: x86_64-unknown-linux-gnu
release: 1.80.0-nightly
LLVM version: 18.1.4

Additional Labels

@rustbot label +I-suggestion-causes-error

mqudsi commented 6 months ago

In this case, clippy should have actually suggested replacing .map(|l| l.as_ref()) with .copied(), which is clearer, doesn't trigger any other clippy lints, and is actually better for the optimizer (though this option is not necessarily always available).