Add ignore-msrv-check-for option to incompatible_msrv lint

[samueltardieu]

This option prevents the lint from triggering on entities that may have been feature gated by the user and thus don't need to be warned about.

Another use case would be when a user has enabled an unstable feature and wants to ignore the MSRV check for a given path. Such a false positive MSRV lint in Rust for Linux has been described in #14425. While we could maintain a list of every path created by unstable features, it looks easier to ignore the MSRV check for the specific functions that are used. Most of the time, users of unstable features will use a nightly compiler without specifying a MSRV, so this would not be needed.

This is particularly interesting when #14216 is merged, as its lintcheck output shows that hits are for feature-gated entities. This option will allow the crate authors not to disable the whole lint, or to #[expect] everywhere a feature-gated MSRV-incompatible entity is used.

changelog: [incompatible_msrv]: add new ignore-msrv-check-for option

[rustbot]

r? @Centri3

rustbot has assigned @Centri3.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[Centri3]

Code looks good, but the reasoning feels a bit frail IMO... I see how this is an issue but globally allowing certain paths feels like the wrong solution to me.

1: An alternative is to just ignore anything inside a feature gate, it would surely be easy, but I'm not sure about this. It's definitely a "FP" but it is hard for me to say we should treat it as one as just ignoring them might do more harm than good. (FNs, of course)

2: It is hard for the linter to know this additional info (that the MSRV is increased with a feature gate). I feel like another thought is that it is totally reasonable to suggest adding a clippy::msrv value alongside the cfg(feature) if we find one, or add this info somewhere alongside the original msrv attribute, but that might just be noise in 90% of cases or something.

Consider these two options, if they're less desirable than this then this looks ok.

[Centri3]

This should definitely include a cfg(feature) IMO so as to show the intent of this config option. Not ignoring the check when there's zero cfg(feature) attributes is an option too.

[samueltardieu]

In #14425 there is a case for ignoring the MSRV check without having a crate-local feature enabled.

[samueltardieu]

2: It is hard for the linter to know this additional info (that the MSRV is increased with a feature gate). I feel like another thought is that it is totally reasonable to suggest adding a clippy::msrv value alongside the cfg(feature) if we find one, or add this info somewhere alongside the original msrv attribute, but that might just be noise in 90% of cases or something.

I will think more about your suggestions, but this one seems a bit problematic to me. If I understand correctly, you are proposing to increase the MSRV used by Clippy when the feature is activated, using for example a #![cfg_attr(feature = "modern", clippy::msrv = "1.85")]. While this would certainly work for the entity we do not want to lint, this could also start triggering lints and suggesting fixes in unrelated places (in code used both when the feature is active and when it is inactive) that may not compile with lower compiler versions.

[Centri3]

That's fair for the case where we add it at the crate-level, but not if added alongside where cfg(feature) is actually used.

Also I think it would make sense to special case this usage to only apply under cfg(feature) but this would probably be confusing. (or add another attribute.)

[samueltardieu]

That's fair for the case where we add it at the crate-level, but not if added alongside where cfg(feature) is actually used.

This can be done already indeed without this option. The goal of this option was to avoid having to disable the incompatible_msrv lint every time the feature was checked, after I noticed the lintcheck output for #14216. I felt like adding an extra attribute at every occurrence may be annoying.

One thing we could do is merge #14216 first, then wait and see if someone requests such a flag, in which case we could look again at the current PR or to alternative solutions. What do you think?

I was going to reroll the dice on #14216 since it has not been reviewed after a few weeks, I'll assign it to you, feel free to reroll if you don't want to.

[Centri3]

I find disabling a lint and explicitly adding more info to be a different, but it would be similarly annoying.

Note I still do believe the second idea (adding the msrv attribute at the crate level) is fine if we add a new attribute to only apply under a specific feature cfg.
Idk, clippy::msrv(feature, 1.46.0) sounds perfectly fine to me, IMO. And this is different from just cfg as it'll only apply under this attribute instead. But that's the general outline I had, feel free to bikeshed

[samueltardieu]

@Centri3 I've updated the description with the another use case which IMO, by itself, would justify the existence of this new option (#14425).

[Centri3]

Mind if I nominate this for the meeting tomorrow? The option is also fine but I'm still on edge if it's the only way to do this.

[samueltardieu]

Mind if I nominate this for the meeting tomorrow? The option is also fine but I'm still on edge if it's the only way to do this.

Good idea!

[samueltardieu]

To be reworked, implementing ideas from today's meeting.

[samueltardieu]

Superseded by #14433