Rust-clippy: "compat" lint category and related lints?

[kentfredric]

What it does

In general, most of the lints presently in clippy seem to promote "latest features" and "modern practices", which is fine for people who are developing end-of-the-dep chain apps.

However, there are those of us who would rather strive towards the broader goal of maximising source compatibility with a broad range of Rust's, or have a water mark MSRV they target.

My proposal is really a more "meta" proposal, that:

• Lint categories for compat exist
• Lints for various compat syntax problems exist
• That general lint categories can be used to "group" a collection of related lints based on your designated target, such as:

   clippy::compat::msrv::1_0_0

(Or something like that)

Why?

Presently the only safeguards against MSRV regression is multi-testing crates before publish, either by the maintainer, a CI, or both.

This means people who submit changes are unlikely to know they've made a change that breaks the MSRV goals, because they're less likely to test older rusts themselves.

Having clippy lints that targeted the most obvious cases of these that are achievable, would at least, mean an end user could run cargo clippy and get told they've added code changes that break rust compat, turned on by configured policy in the repo.

Caveats

Obviously, this is not a guarantee of any kind, because not all classes of compat issues can be stuffed into clippy.

My favourite example of a really silly one, that I don't think clippy can handle, is crates declaring edition = ["2018"] in Cargo.toml, but otherwise not doing anything that requires Edition 2018.

This is silly, because as a dependency, these crates compile and your dependent project compiles just fine, on every rust, except the 3 versions of rust that knew edition was a thing, and that it was also still unstable.

https://kentfredric.github.io/rust-vmatrix/crates-t/tt/tt/

And I also think its probably out-of-scope to verify your dependencies are all, also, meeting your MSRV goals.

But fortunately, there are other ways of managing that

[flip1995]

There's currently implementation work done, to specify the MSRV for Clippy, so Clippy doesn't suggest code that doesn't compile with the MSRV. #6201

IIUC, what you want is a step further: You want a lint, that checks functions/macros/... if they are also available in the specified MSRV? This might be possible, at least for things in std/core/alloc, which should cover most, if not all of the relevant stuff. We would need to check for the #[stable(feature = "<name>", since = "<version>")] attribute on those items. If someone wants to start implementing this, this should be built on top of #6201. (@Suyash458 do you want a new challenge after #6201? 😏)

except the 3 versions of rust that _knew_ edition was a thing, and that it was also still unstable.

This sounds like a bug to me (either in cargo or rustc).

And I also think its probably out-of-scope to verify your _dependencies_ are all, also, meeting your MSRV goals.

Yes this is out of scope. But it would be possible, if someone tries to build a tool, that checks all dependencies, aka building the deps tree and run cargo clippy on everything. Or add a flag to Clippy that does this, which might be easier (this flag would have a good chance to be perma-unstable though). Also this makes only sense, once the described lint is implemented, so let's address this one step at a time.

[kentfredric]

You want a lint, that checks functions/macros/... if they are also available in the specified MSRV?

Indeed. I mean, the "objective" is to make the best possible prediction, without needing to have the spare rustc lying around.

This sounds like a bug to me (either in cargo or rustc).

Yeah, its a pattern I've seen a few times:

• A given syntax is entirely ignored on old cargo/rustc and so nothing fails.
• A given syntax then is "added", but added "experimentally", and so, the existence of it fails due to "you're using an experimental feature, but you're not using an experimental rust".
• Given syntax is then stabilized, and so, it reverts to no longer failing.

And annoyingly, one can't "backport" the "ignore it" aspect to old rusts/cargo.

So the "error" is the user "requiring a syntax" that did the same thing on old rusts as it does now: Nothing.

As the evidence indicates the "required syntax" being completely ignored is inconsequential.

So the fix is "just stop writing that line in your code, and you'll support more rusts, and you won't lose anything".

Also this makes only sense, once the described lint is implemented, so let's address this one step at a time.

Agreed. :)

[ebroto]

I started writing a tool that does exactly this, but never finished it :sweat_smile:. In case it can help, it can be found here (forgive the missing README, this was never published :)

So, to give my 2 cents about what implementing this would imply. Checking library stability is kind of straightforward, there's a specific query in the type context (lookup_stability) that will return stability information for a DefId.

The main problem I see is language stability:

• it requires checking explicitly for each and every feature since 1.0 (e.g. are you using async syntax? underscore lifetimes? #[repr(align(_))] on an enum?, ...)
• Some of the feature gates were implemented at points that are inaccessible using the Callbacks API, for example in the parser. To give an example, a leading | in a match arm is not detectable as there is no trail of it after the code is parsed. Workarounds can be implemented for some cases, but for others it seems impossible without modifying rustc.
• Some stuff is insta-stabilized without a feature, so there is no clear track of it. See e.g. rust-lang/rust#69201 (attributes on if expressions). This can be handled case by case when it pops up, but just adds more complexity.
• Oh, let's not speak about changes like NLL :smile:

[kentfredric]

* it requires checking explicitly for each and every feature since 1.0 (e.g. are you using `async` syntax? underscore lifetimes? `#[repr(align(_))]` on an enum?, ...)

This is the sort of situation where striving to be a perfect solution robs us of a useful solution.

Sure, being able to test all the things would be nice to have, but if that's not viable, then a less comprehensive thing is still helpful.

At least with regards to "let the contributors test this themselves and locally run cargo clippy and make it happy".

Targeting the most common and most useful patterns to guard against would still be useful, just as clippy currently isn't comprehensive about telling you everything you're doing that's sub-optimal.

Its after all, just a linting tool, not a complete modelling of every past and future rust.

And having something that covers most of the problem and gets incrementally better still improves the status quo for the vast majority of crates that do effectively no MSRV testing, for reasons which I imagine to be "the overhead for doing such a thing is beyond their current patience, and requires more effort than just tweaking a single entry in their config"

[ebroto]

This is the sort of situation where striving to be a perfect solution robs us of a useful solution

Of course! My intention was not to dismiss the idea, just to explain the difficulties I found in case someone wants to try :)

IMO this can get big enough to do it as a separate tool, but I may be mistaken.