Allow using clippy::msrv as an outer attribute

[Alexendoo]

changelog: Allow specifying #[clippy::msrv] as an outer attribute

Probably not too useful to clippy users, but it makes the MSRV tests slightly cleaner

[rust-highfive]

r? @Jarcho

(rust-highfive has picked a reviewer for you, use r? to override)

[bors]

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

[Jarcho]

Why was this limited to inner attributes in the first place?

[Alexendoo]

I missed it the first time I looked through #6201 but just found #6201 (comment)

I think the reasoning applied to an earlier version of the PR, but it ended up using enter_lint_attrs which should work fine for both inner & outer attrs

[Jarcho]

Just want to make sure my interpretation is correct. This looks like when an msrv attribute is seen, it sets the msrv until the next msrv attribute, totally ignoring the scope of the attribute. This would make something like:

#[clippy::msrv = "1.28"]
fn foo() {}
fn bar() {}

apply the msrv to both foo and bar. If that's the case this is just encouraging a footgun on stable.

I think this should be done, but only if it's going to be scoped correctly.

[Alexendoo]

Yeah that's #6920, it being used on stable is a good point. I'll get that fixed up first

[flip1995]

I made the decision to limit this to inner attributes, because I didn't really see a use case where it would make sense to apply this to items/expressions/... and because the implementation of only inner attributes was simpler to do.

Together with the cfg_version feature of rustc also having this as an outer attribute makes sense.

[Jarcho]

It can make sense when combined with any cfg attribute. Some crates do have higher version requirements when certain features are used, or do their own version detection in a build script. How often it would be worth adding the msrv attribute for these cases is a different issue.

In the case of cfg_version version would it not be better to set that as the msrv if it's higher than the current value?

[Alexendoo]

Opened #9924

Didn't know about that feature, hopefully that gets to stable at some point

Picking it up automatically could be good, I would guess a cfg doesn't get passed to enter/exit_lint_attrs though

[flip1995]

In the case of cfg_version version would it not be better to set that as the msrv if it's higher than the current value?

As with all cfg attributes, we don't even see that attribute. We even see the code, if the feature is enabled or we don't see the code at all. We sadly cannot access cfg attributes.

[bors]

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

[bors]

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

[Jarcho]

LGTM. Github seems to be only sending me about half the email notifications right now, so you might need to approve yourself after rebasing

[Alexendoo]

Thanks! Hopefully they fix that 👀

@bors r=Jarcho

[bors]

📌 Commit 461e219 has been approved by Jarcho

It is now in the queue for this repository.

[bors]

⌛ Testing commit 461e219 with merge ef99041...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Jarcho
Pushing ef99041 to master...