Rust-clippy: Minimum supported version of Rust (MSVR) config and support

Created on 29 Sep 2020  路  17Comments  路  Source: rust-lang/rust-clippy

Many lints suggest using new features in the std lib like clippy::manual_strip. I think these are great because they help people upgrade their code to take advantage of these features. However, there are many projects that support old versions of Rust that can't use these. These lints only make more work for those projects because they have to allows to fix their CI. See this recent commit in serde-yaml for example.

I'd like to add a key to clippy.toml to specify the minimum version of rust (and the stdlib) that a project supports. We can conditionally register these lints that target features not supported on that version.

I understand that eventually proper support for this will be added to Cargo and rustc to support this but at that stage we can just deprecate the setting and use the new stuff.

L-enhancement good-first-issue hacktoberfest

Most helpful comment

Hmm, the msrv list in docs mentions only four lints, but there are more in https://github.com/rust-lang/rust-clippy/issues/6097#issuecomment-731080791

All 17 comments

I think this makes sense. This should be possible by just adding a "lint configuration" and passing that to the relevant lint passes.

@flip1995 I'd like to pick this up!

This would be awesome to have. I know we have a bunch of these allows in the url crate already at least.

@djc Can you post a list of them here, so we can apply this configuration option to those lints? That would be really helpful!

In url we have these:

#[allow(clippy::option_as_ref_deref)] // introduced in 1.40, MSRV is 1.36
#[allow(clippy::manual_non_exhaustive)] // introduced in 1.40, MSRV is 1.36

The matches!() macro is also an important one that I ran into before.

clippy::match-like-matches-macro requires matches!() which is new in 1.42

@flip1995 do we need to add an msvr attribute to all the lints to denote the targeted Rust version?

No, just the lints mentioned here. And then, we can iteratively add it to lints in the future, if a issue for them comes up.

Just a heads up: manual_range_contains should have a msrv of 1.35.0.

THIS! Just got bitten by matches!() being used in postgres-protocol motivated by clippy.

Can we perhaps also add a lint that will suggest setting the msrv key for projects that don't have it? Explicit latest would turn it off. This would motivate people to at least think about supporting older versions.

Also, oddly enough, Debian stable contains rustc 1.41.1, but oddly enough cargo 1.42.1, so please make sure the version is compared against rustc not against cargo.

Besides making this lint only configurable in the config file, we may want to also make it configurable with an attribute, like #[clippy::msrv = "1.42.0"]. This is also done in the cognitive_complexity lint.

Can we perhaps also add a lint that will suggest setting the msrv key for projects that don't have it?

This could be implemented once cargo supports setting it, aka once the bikeshedding in https://github.com/rust-lang/rust/issues/65262 is over and someone implements it ;)

Otherwise I don't think a lint that always triggers on every crate to mention the msrv Clippy configuration is helpful.

manual_strip depends on methods first added in 1.45.

Some have already been mentioned, but I have a list of (13) lints that requires newer compilers:

  • redundant_field_names requires Rust 1.17 due to suggest feature stablized in that version.
  • redundant_static_lifetimes requires Rust 1.17 due to suggest feature stablized in that version.
  • filter_map_next requires Rust 1.30 due to suggest Iterator::find_map.
  • checked_conversions requires Rust 1.34 due to suggest TryFrom.
  • manual_range_contains requires Rust 1.35 due to suggest Range*::contains.
  • use_self requires Rust 1.37 due to suggest Self::Variant on enum.
  • mem_replace_with_default requires Rust 1.40 due to suggest mem::take.
  • manual_non_exhaustive requires Rust 1.40 due to suggest #[non_exhaustive]. Addressed in #6201
  • option_as_ref_deref requires Rust 1.40 due to suggest Option::{as_deref, as_deref_mut}. Addressed in #6201
  • map_unwrap_or requires Rust 1.41 due to suggest Result::{map_or, map_or_else}.
  • match_like_matches_macro requires Rust 1.42 due to suggest matches!. Addressed in #6201
  • manual_strip requires Rust 1.45 due to suggest str::{strip_prefix, strip_suffix}. Addressed in #6201
  • missing_const_for_fn requires Rust 1.46 due to match/if/loop in const fn needs that version.

(I have not checked the versions older than 1.31. I knew about filter_map_next and some lints because I had encountered it before.)

cc @Suyash458 ^

I would not add them in #6201. This PR is already big enough as it is. Let's add the remaining ones in a separate PR after #6201 is merged.

@flip1995 is this good to be closed now?

Hmm, the msrv list in docs mentions only four lints, but there are more in https://github.com/rust-lang/rust-clippy/issues/6097#issuecomment-731080791

@Suyash458 The lints also have to be added here: https://github.com/rust-lang/rust-clippy/blob/5c00931642357c835d5ba292d8c642ef7389021b/clippy_lints/src/utils/conf.rs#L109 Can you do this please in a follow up PR? I totally missed that during review. Thanks for pointing that out @Kixunil!

Was this page helpful?
0 / 5 - 0 ratings