emit lint when using `ref` with match ergonomics

[lcnr]

fn foo(x: &Option<u32>) -> Option<&u32> {
    match x {
        Some(ref inner) => Some(inner),
        None => None,
    }
}

because of match ergonomics, every binding inside of Some is already matched by ref implicitly, so the additional explicit ref is completely redundant. Consider the following example where l and r have the exact same type.

#![feature(type_name_of_val)]
use std::any::type_name_of_val;
fn foo(x: &Option<(u32, u32)>) {
    match x {
        Some((l, ref r)) => assert_eq!(
            type_name_of_val(l),
            type_name_of_val(r),
        ),
        None => unreachable!(),
    }
}

fn main() {
    foo(&Some((3, 3)));
}

This lint should either suggest to disable match ergonomics by adding & and &mut patterns in the relevant places or to remove the ref.

There are a lot of places even inside of rustc which hit this pattern, so this lint feels really desirable to me.

[matthiaskrgr]

could also be a clippy lint :upside_down_face:

[lcnr]

i feel like i encounter this often enough that having it in rustc itself would be nice. That lint should be part of the unused category, similar to the lint for unused braces.

[est31]

clippy's needless_borrow lint already does this, at least partially, e.g. https://github.com/rust-lang/rust-clippy/issues/9049 .

Generally the "this code can use new feature X" lints are included in clippy. Note that there is code out there that has really old MSRV requirements. clippy has a mechanism to handle such MSRV requirements, including cargo integration, but rustc lacks the ability to turn on lints based on MSRV support.

See also the thread of https://github.com/rust-lang/rust/pull/87512 and why it was rejected.

[est31]

Note that I'm not saying that rustc should never get such lints, ever. It's just a problem that should be addressed before such a lint is added.

For example the MSRV problem could be solved by turning on lints based on the edition. So it could enable version based lints only on editions where the first rustc version of the edition already had the feature the lint is suggesting. There are mechanisms for editions already, and editions also influence how rust code should be written. clippy defaults to MSRV == latest, which is obviously way too aggressive.

[lcnr]

Generally the "this code can use new feature X" lints are included in clippy. Note that there is code out there that has really old MSRV requirements. clippy has a mechanism to handle such MSRV requirements, including cargo integration, but rustc lacks the ability to turn on lints based on MSRV support.

My proposed lint is not a "this code can use new feature X", it is "this code either does not compile or is redundant" afaik.

If you have match ergonomics together with ref, then the code just didn't compile before match ergonomics were stable because you were missing some explicit & somewhere: https://rust.godbolt.org/z/7q67hqbna

[isabelleatkins]

@rustbot claim

[isabelleatkins]

@rustbot release-assignment