Add new lint missing_rust_version

[sorairolake]

This lint checks to see if the rust-version field is defined in Cargo.toml.

Starting with version 3, the resolver takes this field into account when picking dependencies. The MSRV-aware resolver has been stabilized since Rust 1.84.0. I think it would be better if this field is defined in order to make more effective use of the MSRV-aware resolver. Also, I think defining this field is useful even for older versions, as it would provide clearer diagnostics about the insufficient toolchain version.

I set the MSRV of this lint to 1.58.0. This is because this field is respected as of Rust 1.56.0, but the cargo_metadata crate always returns None if running with a version of Cargo older than 1.58.0.¹

Closes #14064

---

changelog: [missing_rust_version]: new lint

Footnotes

1. https://docs.rs/cargo_metadata/0.18.1/cargo_metadata/struct.Package.html#structfield.rust_version ↩

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Manishearth (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[Manishearth]

Opened an FCP: https://rust-lang.zulipchat.com/#narrow/channel/257328-clippy/topic/FCP.3A.20missing_rust_version

I wonder if this should be on by default: not all crates wish to specify an MSRV. Maybe they should anyway? I guess there's always a de facto MSRV.

Slight worry is that asking people to set this might mean they set it but don't CI it and it stops working, defeating the purpose of this field.

[Manishearth]

cc @rust-lang/cargo in case y'all have opinions on how strongly we should be nudging people here

[epage]

I do not think this lint should be added.

Setting package.rust-version comes with expectations and people blindly seeing and resolving a lint makes it unlikely people will be meeting those expectations.

[weihanglo]

Not an endorsement of adding this lint. Just wonder do we really mean to check every package in the dependency graph, including all deeper transitive dependencies?

[Manishearth]

We shouldn't, yeah.

[Manishearth]

Thanks, Ed! I'm inclined to close this if the Cargo team is not super sanguine about a lint like this.

It is probably worth nudging people to document MSRV, but perhaps that should be done socially rather than through Clippy.

[epage]

It is probably worth nudging people to document MSRV, but perhaps that should be done socially rather than through Clippy.

Note that my comment of being against this was personal and not speaking on behalf of the team but a core part of the MSRV-aware resolver RFC that the team did agree to is the importance of package.rust-version being verified.

It is probably worth nudging people to document MSRV, but perhaps that should be done socially rather than through Clippy.

The RFC also included a mechanism to allow a low-effort way of adding an MSRV, by allowing an opt-in to use the current toolchain for the MSRV (pinning it in the .crate on publish) with this being set in cargo new. See https://rust-lang.github.io/rfcs/3537-msrv-resolver.html#cargo-publish

That should help move the ecosystem to having MSRV set more often. Due to its nature of aggressively raising the MSRV, it will cause a lot of disruption for people without the MSRV-aware resolver and I'm waiting on the MSRV-aware resolver being out a bit longer before moving forward with it.

[joshtriplett]

I think it might make sense to have this as an allow-by-default lint, to help people who want to enforce this in a workspace. But I agree that, at least for now, it shouldn't be warning by default.

[epage]

I wonder if there will be much value with allowy-by-default to outweigh the misuse.

You have to enable in each workspace. You likely are also setting workspace.package.rust-version. cargo new will automatically inherit workspace.rust-version.

So it will help only if have an MSRV and are verifying your MSRV and one of

• You aren't using workspace inheritance
• You are not using cargo new to create packages

[epage]

One problem with merging this is clippy's handling of groups. Clippy lints must only belong to one group and groups have semantic meaning except for Cargo lints.

If we want something like this eventually, I would prefer waiting on rust-lang/cargo#12235 and conversations like https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/Sharing.20of.20lints/near/475872847

[joshtriplett]

I'm not going to advocate for it here; I primarily mean that it probably wouldn't do any harm if allow-by-default.

[Manishearth]

Yeah, I think this lint doesn't make too much sense as allow-by-default (for the reasons given by Ed), and it appears that some Cargo team members have a decent plan for nudging the community over to always-working MSRV. I'd rather have this topic be revisited if the Cargo team wants us to do more work on this.

Thanks all, for the discussion, and thanks for initiating this topic @sorairolake!

The RFC also included a mechanism to allow a low-effort way of adding an MSRV, by allowing an opt-in to use the current toolchain for the MSRV (pinning it in the .crate on publish) with this being set in cargo new. See https://rust-lang.github.io/rfcs/3537-msrv-resolver.html#cargo-publish

I wonder if there's value to having a publish-rust-version key that is autoincluded (either in Cargo.toml, or cargo-vcs-info.json) that strongly signals that this is a "non minimal supported rust version" that can be used by tooling to try and pick a version.

[epage]

I wonder if there's value to having a publish-rust-version key that is autoincluded (either in Cargo.toml, or cargo-vcs-info.json) that strongly signals that this is a "non minimal supported rust version" that can be used by tooling to try and pick a version.

The RFC talks about similar ideas at https://rust-lang.github.io/rfcs/3537-msrv-resolver.html#ensuring-the-registry-index-has-rust-version-without-affecting-quality