Skip to content

Commit dceb0d3

Browse files
committed
fix(missing_trait_methods): MSRV/unstable awareness
changelog: [`missing_trait_methods`]: do not diagnose unimplemented unstable trait fn's unless unstable feature is enabled Simply reusing the stability check that is already used by [`len_zero`]. I suspect that more lints need a fix similar to this one, likely everything that handles MSRV. As per comments in the PR, the simple check for a feature is insufficient, so to avoid false-positives, support for unstable is dropped. Inability to write hermetic tests is *very* problematic here, it's basically impossible to test that we actually don't lint the unstable methods. This is bad.
1 parent 64c7431 commit dceb0d3

9 files changed

Lines changed: 94 additions & 57 deletions

File tree

clippy_lints/src/len_zero.rs

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use clippy_utils::{parent_item_name, peel_ref_operators, sym};
99
use rustc_ast::ast::LitKind;
1010
use rustc_errors::Applicability;
1111
use rustc_hir::def_id::DefId;
12-
use rustc_hir::{BinOpKind, Expr, ExprKind, PatExprKind, PatKind, RustcVersion, StabilityLevel, StableSince};
12+
use rustc_hir::{BinOpKind, Expr, ExprKind, PatExprKind, PatKind};
1313
use rustc_lint::{LateContext, LateLintPass};
1414
use rustc_middle::ty::{self, Ty};
1515
use rustc_session::impl_lint_pass;
@@ -298,21 +298,7 @@ fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>, msrv: Msrv) -> bool {
298298
if item.is_fn() {
299299
let sig = cx.tcx.fn_sig(item.def_id).skip_binder();
300300
let ty = sig.skip_binder();
301-
ty.inputs().len() == 1
302-
&& cx.tcx.lookup_stability(item.def_id).is_none_or(|stability| {
303-
if let StabilityLevel::Stable { since, .. } = stability.level {
304-
let version = match since {
305-
StableSince::Version(version) => version,
306-
StableSince::Current => RustcVersion::CURRENT,
307-
StableSince::Err(_) => return false,
308-
};
309-
310-
msrv.meets(cx, version)
311-
} else {
312-
// Unstable fn, check if the feature is enabled.
313-
cx.tcx.features().enabled(stability.feature) && msrv.current(cx).is_none()
314-
}
315-
})
301+
ty.inputs().len() == 1 && msrv.is_stable(cx, item.def_id)
316302
} else {
317303
false
318304
}

clippy_lints/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -764,7 +764,7 @@ rustc_lint::late_lint_methods!(
764764
BoolToIntWithIf: bool_to_int_with_if::BoolToIntWithIf = bool_to_int_with_if::BoolToIntWithIf,
765765
BoxDefault: box_default::BoxDefault = box_default::BoxDefault,
766766
ImplicitSaturatingAdd: implicit_saturating_add::ImplicitSaturatingAdd = implicit_saturating_add::ImplicitSaturatingAdd,
767-
MissingTraitMethods: missing_trait_methods::MissingTraitMethods = missing_trait_methods::MissingTraitMethods,
767+
MissingTraitMethods: missing_trait_methods::MissingTraitMethods = missing_trait_methods::MissingTraitMethods::new(conf),
768768
FromRawWithVoidPtr: from_raw_with_void_ptr::FromRawWithVoidPtr = from_raw_with_void_ptr::FromRawWithVoidPtr,
769769
ConfusingXorAndPow: suspicious_xor_used_as_pow::ConfusingXorAndPow = suspicious_xor_used_as_pow::ConfusingXorAndPow,
770770
ManualIsAsciiCheck: manual_is_ascii_check::ManualIsAsciiCheck = manual_is_ascii_check::ManualIsAsciiCheck::new(conf),

clippy_lints/src/missing_trait_methods.rs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1+
use clippy_config::Conf;
12
use clippy_utils::diagnostics::span_lint_and_then;
23
use clippy_utils::is_lint_allowed;
34
use clippy_utils::macros::span_is_local;
5+
use clippy_utils::msrvs::Msrv;
46
use clippy_utils::source::snippet_opt;
57
use rustc_hir::def_id::DefIdSet;
68
use rustc_hir::{Impl, Item, ItemKind};
79
use rustc_lint::{LateContext, LateLintPass};
8-
use rustc_session::declare_lint_pass;
10+
use rustc_session::impl_lint_pass;
911

1012
declare_clippy_lint! {
1113
/// ### What it does
@@ -56,7 +58,17 @@ declare_clippy_lint! {
5658
"trait implementation uses default provided method"
5759
}
5860

59-
declare_lint_pass!(MissingTraitMethods => [MISSING_TRAIT_METHODS]);
61+
impl_lint_pass!(MissingTraitMethods => [MISSING_TRAIT_METHODS]);
62+
63+
pub struct MissingTraitMethods {
64+
msrv: Msrv,
65+
}
66+
67+
impl MissingTraitMethods {
68+
pub fn new(conf: &'static Conf) -> Self {
69+
Self { msrv: conf.msrv }
70+
}
71+
}
6072

6173
impl<'tcx> LateLintPass<'tcx> for MissingTraitMethods {
6274
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
@@ -79,6 +91,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingTraitMethods {
7991
.tcx
8092
.provided_trait_methods(trait_id)
8193
.filter(|assoc| !trait_item_ids.contains(&assoc.def_id))
94+
.filter(|assoc| self.msrv.is_stable(cx, assoc.def_id))
8295
{
8396
span_lint_and_then(
8497
cx,

clippy_utils/src/msrvs.rs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ use rustc_ast::Attribute;
33
use rustc_ast::attr::AttributeExt;
44
use rustc_attr_parsing::parse_version;
55
use rustc_data_structures::smallvec::SmallVec;
6-
use rustc_hir::{HirId, RustcVersion};
6+
use rustc_hir::def_id::DefId;
7+
use rustc_hir::{HirId, RustcVersion, StabilityLevel, StableSince};
78
use rustc_lint::LateContext;
89
use rustc_middle::ty::TyCtxt;
910
use rustc_session::Session;
@@ -175,6 +176,25 @@ impl Msrv {
175176
_ => {},
176177
}
177178
}
179+
180+
pub fn is_stable(self, cx: &LateContext<'_>, def_id: DefId) -> bool {
181+
cx.tcx.lookup_stability(def_id).is_none_or(|stability| {
182+
if let StabilityLevel::Stable { since, .. } = stability.level {
183+
let version = match since {
184+
StableSince::Version(version) => version,
185+
StableSince::Current => RustcVersion::CURRENT,
186+
StableSince::Err(_) => return false,
187+
};
188+
189+
self.meets(cx, version)
190+
} else {
191+
// Unstable fn.
192+
// FIXME: can we check that the feature is enabled?
193+
// Please see https://github.com/rust-lang/rust-clippy/pull/17309#discussion_r3486693263 for false-positive concerns.
194+
false
195+
}
196+
})
197+
}
178198
}
179199

180200
/// Tracks the current MSRV from `clippy.toml`, `Cargo.toml` or set via `#[clippy::msrv]` in early

tests/ui/len_zero_unstable.fixed

Lines changed: 0 additions & 7 deletions
This file was deleted.

tests/ui/len_zero_unstable.rs

Lines changed: 0 additions & 7 deletions
This file was deleted.

tests/ui/len_zero_unstable.stderr

Lines changed: 0 additions & 11 deletions
This file was deleted.

tests/ui/missing_trait_methods.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#![warn(clippy::missing_trait_methods)]
22
#![expect(clippy::needless_lifetimes)]
3+
#![allow(clippy::derive_ord_xor_partial_ord)]
34

45
trait A {
56
fn provided() {}
@@ -69,3 +70,29 @@ impl PartialEq<Partial> for Partial {
6970
todo!()
7071
}
7172
}
73+
74+
#[clippy::msrv = "1.20.0"]
75+
fn msrv0() {
76+
#[derive(PartialEq, Eq, PartialOrd)]
77+
struct S {}
78+
79+
impl Ord for S {
80+
fn cmp(&self, other: &S) -> std::cmp::Ordering {
81+
unreachable!()
82+
}
83+
}
84+
}
85+
86+
#[clippy::msrv = "1.21.0"]
87+
fn msrv1() {
88+
#[derive(PartialEq, Eq, PartialOrd)]
89+
struct S {}
90+
91+
impl Ord for S {
92+
//~^ missing_trait_methods
93+
//~| missing_trait_methods
94+
fn cmp(&self, other: &S) -> std::cmp::Ordering {
95+
unreachable!()
96+
}
97+
}
98+
}
Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,72 +1,88 @@
11
error: missing trait method provided by default: `provided`
2-
--> tests/ui/missing_trait_methods.rs:22:1
2+
--> tests/ui/missing_trait_methods.rs:23:1
33
|
44
LL | impl A for Partial {}
55
| ^^^^^^^^^^^^^^^^^^
66
|
77
help: implement the method
8-
--> tests/ui/missing_trait_methods.rs:5:5
8+
--> tests/ui/missing_trait_methods.rs:6:5
99
|
1010
LL | fn provided() {}
1111
| ^^^^^^^^^^^^^
1212
= note: `-D clippy::missing-trait-methods` implied by `-D warnings`
1313
= help: to override `-D warnings` add `#[allow(clippy::missing_trait_methods)]`
1414

1515
error: missing trait method provided by default: `b`
16-
--> tests/ui/missing_trait_methods.rs:25:1
16+
--> tests/ui/missing_trait_methods.rs:26:1
1717
|
1818
LL | impl B for Partial {
1919
| ^^^^^^^^^^^^^^^^^^
2020
|
2121
help: implement the method
22-
--> tests/ui/missing_trait_methods.rs:15:5
22+
--> tests/ui/missing_trait_methods.rs:16:5
2323
|
2424
LL | fn b<'a, T: AsRef<[u8]>>(a: &'a T) -> &'a [u8] {
2525
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2626

2727
error: missing trait method provided by default: `one`
28-
--> tests/ui/missing_trait_methods.rs:59:1
28+
--> tests/ui/missing_trait_methods.rs:60:1
2929
|
3030
LL | impl MissingMultiple for Partial {}
3131
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3232
|
3333
help: implement the method
34-
--> tests/ui/missing_trait_methods.rs:54:5
34+
--> tests/ui/missing_trait_methods.rs:55:5
3535
|
3636
LL | fn one() {}
3737
| ^^^^^^^^
3838

3939
error: missing trait method provided by default: `two`
40-
--> tests/ui/missing_trait_methods.rs:59:1
40+
--> tests/ui/missing_trait_methods.rs:60:1
4141
|
4242
LL | impl MissingMultiple for Partial {}
4343
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4444
|
4545
help: implement the method
46-
--> tests/ui/missing_trait_methods.rs:55:5
46+
--> tests/ui/missing_trait_methods.rs:56:5
4747
|
4848
LL | fn two() {}
4949
| ^^^^^^^^
5050

5151
error: missing trait method provided by default: `three`
52-
--> tests/ui/missing_trait_methods.rs:59:1
52+
--> tests/ui/missing_trait_methods.rs:60:1
5353
|
5454
LL | impl MissingMultiple for Partial {}
5555
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5656
|
5757
help: implement the method
58-
--> tests/ui/missing_trait_methods.rs:56:5
58+
--> tests/ui/missing_trait_methods.rs:57:5
5959
|
6060
LL | fn three() {}
6161
| ^^^^^^^^^^
6262

6363
error: missing trait method provided by default: `ne`
64-
--> tests/ui/missing_trait_methods.rs:67:1
64+
--> tests/ui/missing_trait_methods.rs:68:1
6565
|
6666
LL | impl PartialEq<Partial> for Partial {
6767
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6868
|
6969
= help: implement the missing `ne` method of the `PartialEq<Partial>` trait
7070

71-
error: aborting due to 6 previous errors
71+
error: missing trait method provided by default: `max`
72+
--> tests/ui/missing_trait_methods.rs:91:5
73+
|
74+
LL | impl Ord for S {
75+
| ^^^^^^^^^^^^^^
76+
|
77+
= help: implement the missing `max` method of the `Ord` trait
78+
79+
error: missing trait method provided by default: `min`
80+
--> tests/ui/missing_trait_methods.rs:91:5
81+
|
82+
LL | impl Ord for S {
83+
| ^^^^^^^^^^^^^^
84+
|
85+
= help: implement the missing `min` method of the `Ord` trait
86+
87+
error: aborting due to 8 previous errors
7288

0 commit comments

Comments
 (0)