Skip to content

Various improvements to the incompatible_msrv lint #14433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 84 additions & 31 deletions clippy_lints/src/incompatible_msrv.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_in_test;
use clippy_utils::msrvs::Msrv;
use rustc_attr_parsing::{RustcVersion, StabilityLevel, StableSince};
use rustc_attr_parsing::{RustcVersion, Stability, StableSince};
use rustc_data_structures::fx::FxHashMap;
use rustc_hir::{Expr, ExprKind, HirId, QPath};
use rustc_lint::{LateContext, LateLintPass};
Expand Down Expand Up @@ -33,15 +33,54 @@ declare_clippy_lint! {
///
/// To fix this problem, either increase your MSRV or use another item
/// available in your current MSRV.
///
/// You can also locally change the MSRV that should be checked by Clippy,
/// for example if a feature in your crate (e.g., `modern_compiler`) should
/// allow you to use an item:
///
/// ```no_run
/// //! This crate has a MSRV of 1.3.0, but we also have an optional feature
/// //! `sleep_well` which requires at least Rust 1.4.0.
///
/// // When the `sleep_well` feature is set, do not warn for functions available
/// // in Rust 1.4.0 and below.
/// #![cfg_attr(feature = "sleep_well", clippy::msrv = "1.4.0")]
///
/// use std::time::Duration;
///
/// #[cfg(feature = "sleep_well")]
/// fn sleep_for_some_time() {
/// std::thread::sleep(Duration::new(1, 0)); // Will not trigger the lint
/// }
/// ```
///
/// You can also increase the MSRV in tests, by using:
///
/// ```no_run
/// // Use a much higher MSRV for tests while keeping the main one low
/// #![cfg_attr(test, clippy::msrv = "1.85.0")]
///
/// #[test]
/// fn my_test() {
/// // The tests can use items introduced in Rust 1.85.0 and lower
/// // without triggering the `incompatible_msrv` lint.
/// }
/// ```
#[clippy::version = "1.78.0"]
pub INCOMPATIBLE_MSRV,
suspicious,
"ensures that all items used in the crate are available for the current MSRV"
}

#[derive(Clone, Copy)]
enum Availability {
FeatureEnabled,
Since(RustcVersion),
}

pub struct IncompatibleMsrv {
msrv: Msrv,
is_above_msrv: FxHashMap<DefId, RustcVersion>,
availability_cache: FxHashMap<DefId, Availability>,
check_in_tests: bool,
}

Expand All @@ -51,35 +90,32 @@ impl IncompatibleMsrv {
pub fn new(conf: &'static Conf) -> Self {
Self {
msrv: conf.msrv,
is_above_msrv: FxHashMap::default(),
availability_cache: FxHashMap::default(),
check_in_tests: conf.check_incompatible_msrv_in_tests,
}
}

fn get_def_id_version(&mut self, tcx: TyCtxt<'_>, def_id: DefId) -> RustcVersion {
if let Some(version) = self.is_above_msrv.get(&def_id) {
return *version;
/// Returns the availability of `def_id`, whether it is enabled through a feature or
/// available since a given version (the default being Rust 1.0.0).
fn get_def_id_availability(&mut self, tcx: TyCtxt<'_>, def_id: DefId) -> Availability {
if let Some(availability) = self.availability_cache.get(&def_id) {
return *availability;
}
let version = if let Some(version) = tcx
.lookup_stability(def_id)
.and_then(|stability| match stability.level {
StabilityLevel::Stable {
since: StableSince::Version(version),
..
} => Some(version),
_ => None,
}) {
version
let stability = tcx.lookup_stability(def_id);
let version = if stability.is_some_and(|stability| tcx.features().enabled(stability.feature)) {
Availability::FeatureEnabled
} else if let Some(StableSince::Version(version)) = stability.as_ref().and_then(Stability::stable_since) {
Availability::Since(version)
} else if let Some(parent_def_id) = tcx.opt_parent(def_id) {
self.get_def_id_version(tcx, parent_def_id)
self.get_def_id_availability(tcx, parent_def_id)
} else {
RustcVersion {
Availability::Since(RustcVersion {
major: 1,
minor: 0,
patch: 0,
}
})
};
self.is_above_msrv.insert(def_id, version);
self.availability_cache.insert(def_id, version);
version
}

Expand Down Expand Up @@ -110,40 +146,57 @@ impl IncompatibleMsrv {

if (self.check_in_tests || !is_in_test(cx.tcx, node))
&& let Some(current) = self.msrv.current(cx)
&& let version = self.get_def_id_version(cx.tcx, def_id)
&& let Availability::Since(version) = self.get_def_id_availability(cx.tcx, def_id)
&& version > current
{
span_lint(
span_lint_and_then(
cx,
INCOMPATIBLE_MSRV,
span,
format!(
"current MSRV (Minimum Supported Rust Version) is `{current}` but this item is stable since `{version}`"
),
|diag| {
if is_under_cfg_attribute(cx, node) {
diag.note_once("you may want to conditionally increase the MSRV considered by Clippy using the `clippy::msrv` attribute");
}
},
);
}
}
}

impl<'tcx> LateLintPass<'tcx> for IncompatibleMsrv {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
// TODO: check for const stability when in const context
match expr.kind {
ExprKind::MethodCall(_, _, _, span) => {
if let Some(method_did) = cx.typeck_results().type_dependent_def_id(expr.hir_id) {
self.emit_lint_if_under_msrv(cx, method_did, expr.hir_id, span);
}
},
ExprKind::Call(call, _) => {
// Desugaring into function calls by the compiler will use `QPath::LangItem` variants. Those should
// not be linted as they will not be generated in older compilers if the function is not available,
// and the compiler is allowed to call unstable functions.
if let ExprKind::Path(qpath @ (QPath::Resolved(..) | QPath::TypeRelative(..))) = call.kind
&& let Some(path_def_id) = cx.qpath_res(&qpath, call.hir_id).opt_def_id()
{
self.emit_lint_if_under_msrv(cx, path_def_id, expr.hir_id, call.span);
// Desugaring into function calls by the compiler will use `QPath::LangItem` variants. Those should
// not be linted as they will not be generated in older compilers if the function is not available,
// and the compiler is allowed to call unstable functions.
ExprKind::Path(qpath @ (QPath::Resolved(..) | QPath::TypeRelative(..))) => {
if let Some(path_def_id) = cx.qpath_res(&qpath, expr.hir_id).opt_def_id() {
self.emit_lint_if_under_msrv(cx, path_def_id, expr.hir_id, expr.span);
}
},
_ => {},
}
}
}

/// Heuristic checking if the node `hir_id` is under a `#[cfg()]` or `#[cfg_attr()]`
/// attribute.
fn is_under_cfg_attribute(cx: &LateContext<'_>, hir_id: HirId) -> bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this only add the note under cfg(feature)? I think this is only useful otherwise for cfg(test), which feels like an edge case to me IMO. I feel like it's silly to even lint in tests, but what are your thoughts here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My rationale was that the lint only triggers when a MSRV is set on the current crate. However, when features are used, the author might want to express the fact that, in some configurations, they can go beyond the default MSRV. If the used entity is not behind a crate feature or a cfg item (possibly set by the build script), then it is likely that this is a real issue with the default MSRV, and it is probably a bad idea to suggest silencing Clippy with a #[clippy::msrv] attribute there.

For example, anyhow has a MSRV of 1.39, but when compiled with more recent compilers, it takes advantage of std::backtrace::Backtrace. Those uses are cfg gated. I feel this is the best place to make the suggestion without risking making a counterproductive one.

cx.tcx.hir_parent_id_iter(hir_id).any(|id| {
cx.tcx.hir_attrs(id).iter().any(|attr| {
matches!(
attr.ident().map(|ident| ident.name),
Some(sym::cfg_trace | sym::cfg_attr_trace)
)
})
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ error: current MSRV (Minimum Supported Rust Version) is `1.3.0` but this item is
|
LL | sleep(Duration::new(1, 0));
| ^^^^^
|
= note: you may want to conditionally increase the MSRV considered by Clippy using the `clippy::msrv` attribute

error: aborting due to 3 previous errors

2 changes: 1 addition & 1 deletion tests/ui/checked_conversions.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ pub const fn issue_8898(i: u32) -> bool {
#[clippy::msrv = "1.33"]
fn msrv_1_33() {
let value: i64 = 33;
let _ = value <= (u32::MAX as i64) && value >= 0;
let _ = value <= (u32::max_value() as i64) && value >= 0;
}

#[clippy::msrv = "1.34"]
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/checked_conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,13 @@ pub const fn issue_8898(i: u32) -> bool {
#[clippy::msrv = "1.33"]
fn msrv_1_33() {
let value: i64 = 33;
let _ = value <= (u32::MAX as i64) && value >= 0;
let _ = value <= (u32::max_value() as i64) && value >= 0;
}

#[clippy::msrv = "1.34"]
fn msrv_1_34() {
let value: i64 = 34;
let _ = value <= (u32::MAX as i64) && value >= 0;
let _ = value <= (u32::max_value() as i64) && value >= 0;
//~^ checked_conversions
}

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/checked_conversions.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ LL | let _ = value <= u16::MAX as u32 && value as i32 == 5;
error: checked cast can be simplified
--> tests/ui/checked_conversions.rs:104:13
|
LL | let _ = value <= (u32::MAX as i64) && value >= 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u32::try_from(value).is_ok()`
LL | let _ = value <= (u32::max_value() as i64) && value >= 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u32::try_from(value).is_ok()`

error: aborting due to 17 previous errors

53 changes: 45 additions & 8 deletions tests/ui/incompatible_msrv.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![warn(clippy::incompatible_msrv)]
#![feature(custom_inner_attributes)]
#![feature(panic_internals)]
#![allow(stable_features)]
#![feature(strict_provenance)] // For use in test
#![clippy::msrv = "1.3.0"]

use std::collections::HashMap;
Expand All @@ -13,6 +14,8 @@ fn foo() {
let mut map: HashMap<&str, u32> = HashMap::new();
assert_eq!(map.entry("poneyland").key(), &"poneyland");
//~^ incompatible_msrv
//~| NOTE: `-D clippy::incompatible-msrv` implied by `-D warnings`
//~| HELP: to override `-D warnings` add `#[allow(clippy::incompatible_msrv)]`

if let Entry::Vacant(v) = map.entry("poneyland") {
v.into_key();
Expand Down Expand Up @@ -43,21 +46,22 @@ fn core_special_treatment(p: bool) {

// But still lint code calling `core` functions directly
if p {
core::panicking::panic("foo");
//~^ ERROR: is `1.3.0` but this item is stable since `1.6.0`
let _ = core::iter::once_with(|| 0);
//~^ incompatible_msrv
}

// Lint code calling `core` from non-`core` macros
macro_rules! my_panic {
($msg:expr) => {
core::panicking::panic($msg)
}; //~^ ERROR: is `1.3.0` but this item is stable since `1.6.0`
let _ = core::iter::once_with(|| $msg);
//~^ incompatible_msrv
};
}
my_panic!("foo");

// Lint even when the macro comes from `core` and calls `core` functions
assert!(core::panicking::panic("out of luck"));
//~^ ERROR: is `1.3.0` but this item is stable since `1.6.0`
assert!(core::iter::once_with(|| 0).next().is_some());
//~^ incompatible_msrv
}

#[clippy::msrv = "1.26.0"]
Expand All @@ -70,7 +74,40 @@ fn lang_items() {
#[clippy::msrv = "1.80.0"]
fn issue14212() {
let _ = std::iter::repeat_n((), 5);
//~^ ERROR: is `1.80.0` but this item is stable since `1.82.0`
//~^ incompatible_msrv
}

fn local_msrv_change_suggestion() {
let _ = std::iter::repeat_n((), 5);
//~^ incompatible_msrv

#[cfg(any(test, not(test)))]
{
let _ = std::iter::repeat_n((), 5);
//~^ incompatible_msrv
//~| NOTE: you may want to conditionally increase the MSRV

// Emit the additional note only once
let _ = std::iter::repeat_n((), 5);
//~^ incompatible_msrv
}
}

#[clippy::msrv = "1.78.0"]
fn feature_enable_14425(ptr: *const u8) -> usize {
// Do not warn, because it is enabled through a feature even though
// it is stabilized only since Rust 1.84.0.
let r = ptr.addr();

// Warn about this which has been introduced in the same Rust version
// but is not allowed through a feature.
r.isqrt()
//~^ incompatible_msrv
}

fn non_fn_items() {
let _ = std::io::ErrorKind::CrossesDevices;
//~^ incompatible_msrv
}

fn main() {}
Loading