Change `bindings_with_variant_name` to deny-by-default by timrobertsdev · Pull R...

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @lcnr (or someone else) soon.

Please see the contribution instructions for more information.

Not sure if I should comment on the issue or here on the PR. In any case, we can't just turn this into a deny-by-default lint without breaking Rust's backward compatibility guarantees, right? This should probably only ever happen in a new edition (e.g. starting from edition 2024) or after marking it as future-incompat and waiting for an extensive period of time. A crater run wouldn't be out of place either, I'd wager.

Crater silently ignores lints from dependencies and having deny by default lints impact the root should be fine, as that's something the user can easily change themselves. Afaik upgrading a lint to deny is therefore generally not considered breaking.

I encountered this lint multiple times myself and iirc I never wanted to continue compilation in these cases as this lint always detected a bug.

I think this requires compiler signoff?

@rfcbot fcp merge

Team member @lcnr has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

• proc-macros resolved by Change bindings_with_variant_name to deny-by-default #104154 (comment)

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot concern proc-macros
How does this lint in particular interact with evaluated proc macros? I could see a weird situation where a clash occurs intentionally on the part of the macro author, but will the link either not trigger at all or only trigger on the macro's users' code?

@estebank
I'm not sure what you had in mind, but I put together this (very) short macro to see:

use proc_macro::TokenStream;
use quote::quote;

#[proc_macro_attribute]
pub fn foo(_input: TokenStream, _attributes: TokenStream) -> TokenStream {
    quote! {
        pub enum Foo {
            Bar,
            Baz,
        }

        pub fn bar(foo: Foo) {
            match foo {
                Bar => {},
                Baz => {},
            }
        }
    }.into()
}

Used as:

use macro_crate::foo;
#[foo]
pub struct Dummy;

Results in no errors being reported when checked or built.

@rfcbot resolve proc-macros

+. I'm a big fan of this -- warning lints for general smells, but when there are specific situations that we know are essentially always mistakes, they ought to be deny-only.

(FYI @rust-lang/lang that this is happening.)

I agree that while technically valid Rust, it is absolutely improbable that anyone should want code that is formed like this to compile. Like "there's over 2 million Rust programmers and I expect less than 5 would want this to work, and they have already found out how to #![allow(nonstandard_style)]".

This is now entering its final comment period, as per the review above.

The latest upstream changes (presumably #104417) made this pull request unmergeable. Please resolve the merge conflicts.