From 87c28b94635b2ccf5518979b7ab7ee846365653b Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Tue, 20 Jun 2023 17:24:46 +0200 Subject: [PATCH 1/4] [`type_repetition_in_bounds`]: respect msrv for combining maybe bounds --- clippy_lints/src/lib.rs | 2 +- clippy_lints/src/trait_bounds.rs | 33 ++++++++++++++++------- clippy_utils/src/msrvs.rs | 1 + tests/ui/type_repetition_in_bounds.rs | 24 +++++++++++++++++ tests/ui/type_repetition_in_bounds.stderr | 10 ++++++- 5 files changed, 59 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 3fb4e6c8fa5..0008f8f6d46 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -792,7 +792,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(assertions_on_result_states::AssertionsOnResultStates)); store.register_late_pass(|_| Box::new(inherent_to_string::InherentToString)); let max_trait_bounds = conf.max_trait_bounds; - store.register_late_pass(move |_| Box::new(trait_bounds::TraitBounds::new(max_trait_bounds))); + store.register_late_pass(move |_| Box::new(trait_bounds::TraitBounds::new(max_trait_bounds, msrv()))); store.register_late_pass(|_| Box::new(comparison_chain::ComparisonChain)); let ignore_interior_mutability = conf.ignore_interior_mutability.clone(); store.register_late_pass(move |_| Box::new(mut_key::MutableKeyType::new(ignore_interior_mutability.clone()))); diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 3917d8f0225..2cf3529bd1b 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -1,4 +1,5 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; +use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::source::{snippet, snippet_opt, snippet_with_applicability}; use clippy_utils::{is_from_proc_macro, SpanlessEq, SpanlessHash}; use core::hash::{Hash, Hasher}; @@ -9,7 +10,7 @@ use rustc_errors::Applicability; use rustc_hir::def::Res; use rustc_hir::{ - GenericArg, GenericBound, Generics, Item, ItemKind, Node, Path, PathSegment, PredicateOrigin, QPath, + GenericArg, GenericBound, Generics, Item, ItemKind, LangItem, Node, Path, PathSegment, PredicateOrigin, QPath, TraitBoundModifier, TraitItem, TraitRef, Ty, TyKind, WherePredicate, }; use rustc_lint::{LateContext, LateLintPass}; @@ -86,15 +87,16 @@ "check if the same trait bounds are specified more than once during a generic declaration" } -#[derive(Copy, Clone)] +#[derive(Clone)] pub struct TraitBounds { max_trait_bounds: u64, + msrv: Msrv, } impl TraitBounds { #[must_use] - pub fn new(max_trait_bounds: u64) -> Self { - Self { max_trait_bounds } + pub fn new(max_trait_bounds: u64, msrv: Msrv) -> Self { + Self { max_trait_bounds, msrv } } } @@ -222,10 +224,24 @@ fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx Ty<'tcx>) { } } } + + extract_msrv_attr!(LateContext); } impl TraitBounds { - fn check_type_repetition<'tcx>(self, cx: &LateContext<'tcx>, gen: &'tcx Generics<'_>) { + /// Is the given bound a `?Sized` bound, and is combining it (i.e. `T: X + ?Sized`) an error on + /// this MSRV? See https://github.com/rust-lang/rust-clippy/issues/8772 for details. + fn cannot_combine_maybe_bound(&self, cx: &LateContext<'_>, bound: &GenericBound<'_>) -> bool { + if !self.msrv.meets(msrvs::COMBINED_MAYBE_BOUND) + && let GenericBound::Trait(tr, TraitBoundModifier::Maybe) = bound + { + cx.tcx.lang_items().get(LangItem::Sized) == tr.trait_ref.path.res.opt_def_id() + } else { + false + } + } + + fn check_type_repetition<'tcx>(&self, cx: &LateContext<'tcx>, gen: &'tcx Generics<'_>) { struct SpanlessTy<'cx, 'tcx> { ty: &'tcx Ty<'tcx>, cx: &'cx LateContext<'tcx>, @@ -256,10 +272,9 @@ impl Eq for SpanlessTy<'_, '_> {} if p.origin != PredicateOrigin::ImplTrait; if p.bounds.len() as u64 <= self.max_trait_bounds; if !p.span.from_expansion(); - if let Some(ref v) = map.insert( - SpanlessTy { ty: p.bounded_ty, cx }, - p.bounds.iter().collect::>() - ); + let bounds = p.bounds.iter().filter(|b| !self.cannot_combine_maybe_bound(cx, b)).collect::>(); + if !bounds.is_empty(); + if let Some(ref v) = map.insert(SpanlessTy { ty: p.bounded_ty, cx }, bounds); if !is_from_proc_macro(cx, p.bounded_ty); then { let trait_bounds = v diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index e1b1a6f7184..b1f336461d1 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -47,6 +47,7 @@ macro_rules! msrv_aliases { 1,18,0 { HASH_MAP_RETAIN, HASH_SET_RETAIN } 1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST, EXPECT_ERR } 1,16,0 { STR_REPEAT } + 1,15,0 { COMBINED_MAYBE_BOUND } } fn parse_msrv(msrv: &str, sess: Option<&Session>, span: Option) -> Option { diff --git a/tests/ui/type_repetition_in_bounds.rs b/tests/ui/type_repetition_in_bounds.rs index 1d36f4b3c7b..874d97f7a46 100644 --- a/tests/ui/type_repetition_in_bounds.rs +++ b/tests/ui/type_repetition_in_bounds.rs @@ -110,4 +110,28 @@ pub fn g() // This should not lint fn impl_trait(_: impl AsRef, _: impl AsRef) {} +#[clippy::msrv = "1.14.0"] +mod issue8772_fail { + pub trait Trait {} + + pub fn f(arg: usize) + where + T: Trait, Box<[String]>, bool> + 'static, + U: Clone + Sync + 'static, + { + } +} + +#[clippy::msrv = "1.15.0"] +mod issue8772_pass { + pub trait Trait {} + + pub fn f(arg: usize) + where + T: Trait, Box<[String]>, bool> + 'static, + U: Clone + Sync + 'static, + { + } +} + fn main() {} diff --git a/tests/ui/type_repetition_in_bounds.stderr b/tests/ui/type_repetition_in_bounds.stderr index 56867f75b07..54973c5bda5 100644 --- a/tests/ui/type_repetition_in_bounds.stderr +++ b/tests/ui/type_repetition_in_bounds.stderr @@ -35,5 +35,13 @@ LL | T: ?Sized, | = help: consider combining the bounds: `T: Clone + ?Sized` -error: aborting due to 4 previous errors +error: this type has already been used as a bound predicate + --> $DIR/type_repetition_in_bounds.rs:131:9 + | +LL | T: Trait, Box<[String]>, bool> + 'static, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider combining the bounds: `T: ?Sized + Trait, Box<[String]>, bool>` + +error: aborting due to 5 previous errors From 765a6e4a9014977b6f821df164633b3b7f5857f7 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Tue, 20 Jun 2023 17:36:38 +0200 Subject: [PATCH 2/4] put issue link between <> --- clippy_lints/src/trait_bounds.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 2cf3529bd1b..78e27b775fb 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -230,7 +230,7 @@ fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx Ty<'tcx>) { impl TraitBounds { /// Is the given bound a `?Sized` bound, and is combining it (i.e. `T: X + ?Sized`) an error on - /// this MSRV? See https://github.com/rust-lang/rust-clippy/issues/8772 for details. + /// this MSRV? See for details. fn cannot_combine_maybe_bound(&self, cx: &LateContext<'_>, bound: &GenericBound<'_>) -> bool { if !self.msrv.meets(msrvs::COMBINED_MAYBE_BOUND) && let GenericBound::Trait(tr, TraitBoundModifier::Maybe) = bound From 33b6d0d206f4519d0a196c1ae490e8e13c8fbcc6 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Thu, 22 Jun 2023 17:19:43 +0200 Subject: [PATCH 3/4] rename MSRV alias, add MSRV to lint doc --- clippy_lints/src/trait_bounds.rs | 2 +- clippy_lints/src/utils/conf.rs | 2 +- clippy_utils/src/msrvs.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 78e27b775fb..6db330dfa61 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -232,7 +232,7 @@ impl TraitBounds { /// Is the given bound a `?Sized` bound, and is combining it (i.e. `T: X + ?Sized`) an error on /// this MSRV? See for details. fn cannot_combine_maybe_bound(&self, cx: &LateContext<'_>, bound: &GenericBound<'_>) -> bool { - if !self.msrv.meets(msrvs::COMBINED_MAYBE_BOUND) + if !self.msrv.meets(msrvs::MAYBE_BOUND_IN_WHERE) && let GenericBound::Trait(tr, TraitBoundModifier::Maybe) = bound { cx.tcx.lang_items().get(LangItem::Sized) == tr.trait_ref.path.res.opt_def_id() diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 3cf2fbf1f28..54ee960c472 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -293,7 +293,7 @@ pub fn get_configuration_metadata() -> Vec { /// /// Suppress lints whenever the suggested change would cause breakage for other crates. (avoid_breaking_exported_api: bool = true), - /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN. + /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS. /// /// The minimum rust version that the project supports (msrv: Option = None), diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index b1f336461d1..0a4c3418a08 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -47,7 +47,7 @@ macro_rules! msrv_aliases { 1,18,0 { HASH_MAP_RETAIN, HASH_SET_RETAIN } 1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST, EXPECT_ERR } 1,16,0 { STR_REPEAT } - 1,15,0 { COMBINED_MAYBE_BOUND } + 1,15,0 { MAYBE_BOUND_IN_WHERE } } fn parse_msrv(msrv: &str, sess: Option<&Session>, span: Option) -> Option { From 75f9fbc93d7eb1fa0182c8491f6152917055dac4 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Fri, 23 Jun 2023 02:07:05 +0200 Subject: [PATCH 4/4] update lint configuration --- book/src/lint_configuration.md | 1 + 1 file changed, 1 insertion(+) diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 32cc5d1edcc..dd605b76a3d 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -147,6 +147,7 @@ The minimum rust version that the project supports * [`manual_is_ascii_check`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check) * [`manual_rem_euclid`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid) * [`manual_retain`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain) +* [`type_repetition_in_bounds`](https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds) ## `cognitive-complexity-threshold`