From 9b329ed9c42bbf750fb3980bb3a91f4a6a56589a Mon Sep 17 00:00:00 2001 From: Allen Hsu Date: Sun, 1 May 2022 22:58:13 +1000 Subject: [PATCH] Support trait bounds with type parameters. --- clippy_lints/src/trait_bounds.rs | 118 ++++++++++++------ ...repeated_where_clause_or_trait_bound.fixed | 10 ++ .../repeated_where_clause_or_trait_bound.rs | 10 ++ ...epeated_where_clause_or_trait_bound.stderr | 8 +- 4 files changed, 106 insertions(+), 40 deletions(-) diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 43f8a16cf967..57ef4944f535 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -8,8 +8,8 @@ use rustc_data_structures::unhash::UnhashMap; use rustc_errors::Applicability; use rustc_hir::def::Res; use rustc_hir::{ - GenericBound, Generics, Item, ItemKind, Node, ParamName, Path, PathSegment, QPath, TraitItem, Ty, TyKind, - WherePredicate, + GenericArg, GenericBound, Generics, Item, ItemKind, Node, ParamName, Path, PathSegment, QPath, TraitItem, Ty, + TyKind, WherePredicate, }; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; @@ -289,44 +289,11 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) { } } -fn check_bounds_or_where_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) { - fn rollup_traits(cx: &LateContext<'_>, bounds: &[GenericBound<'_>], msg: &str) { - let mut map = FxHashMap::default(); - let mut repeated_spans = false; - for bound in bounds.iter().filter_map(get_trait_info_from_bound) { - let (definition, _, span_direct) = bound; - if map.insert(definition, span_direct).is_some() { - repeated_spans = true; - } - } - - if_chain! { - if repeated_spans; - if let Some(first_trait) = bounds.get(0); - if let Some(last_trait) = bounds.iter().last(); - then { - let all_trait_span = first_trait.span().to(last_trait.span()); - - let mut traits = map.values() - .filter_map(|span| snippet_opt(cx, *span)) - .collect::>(); - traits.sort_unstable(); - let traits = traits.join(" + "); - - span_lint_and_sugg( - cx, - REPEATED_WHERE_CLAUSE_OR_TRAIT_BOUND, - all_trait_span, - msg, - "try", - traits, - Applicability::MachineApplicable - ); - } - } - } +#[derive(PartialEq, Eq, Hash, Debug)] +struct ComparableBound(Res, Vec, Vec); - if gen.span.from_expansion() || (gen.params.is_empty() && gen.where_clause.predicates.is_empty()) { +fn check_bounds_or_where_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) { + if gen.span.from_expansion() { return; } @@ -355,3 +322,76 @@ fn get_trait_info_from_bound<'a>(bound: &'a GenericBound<'_>) -> Option<(Res, &' None } } + +fn try_into_comparable_bound(bound: &GenericBound<'_>) -> Option { + if let GenericBound::Trait(t, _) = bound { + Some(ComparableBound( + t.trait_ref.path.res, + t.trait_ref + .path + .segments + .iter() + .filter_map(|segment| { + // get trait bound type arguments + Some(segment.args?.args.iter().filter_map(|arg| { + if_chain! { + if let GenericArg::Type(ty) = arg; + if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind; + then { return Some(path.res) } + } + None + })) + }) + .flatten() + .collect(), + t.bound_generic_params + .iter() + .flat_map(|param| param.bounds.iter().filter_map(try_into_comparable_bound)) + .collect(), + )) + } else { + None + } +} + +fn rollup_traits(cx: &LateContext<'_>, bounds: &[GenericBound<'_>], msg: &str) { + let mut map = FxHashMap::default(); + let mut repeated_spans = false; + for bound in bounds.iter().filter_map(|bound| { + if let GenericBound::Trait(t, _) = bound { + Some((try_into_comparable_bound(bound)?, t.span)) + } else { + None + } + }) { + let (comparable_bound, span_direct) = bound; + if map.insert(comparable_bound, span_direct).is_some() { + repeated_spans = true; + } + } + + if_chain! { + if repeated_spans; + if let Some(first_trait) = bounds.get(0); + if let Some(last_trait) = bounds.iter().last(); + then { + let all_trait_span = first_trait.span().to(last_trait.span()); + + let mut traits = map.values() + .filter_map(|span| snippet_opt(cx, *span)) + .collect::>(); + traits.sort_unstable(); + let traits = traits.join(" + "); + + span_lint_and_sugg( + cx, + REPEATED_WHERE_CLAUSE_OR_TRAIT_BOUND, + all_trait_span, + msg, + "try", + traits, + Applicability::MachineApplicable + ); + } + } +} diff --git a/tests/ui/repeated_where_clause_or_trait_bound.fixed b/tests/ui/repeated_where_clause_or_trait_bound.fixed index 4cef01db0a4b..af7f741cd6cb 100644 --- a/tests/ui/repeated_where_clause_or_trait_bound.fixed +++ b/tests/ui/repeated_where_clause_or_trait_bound.fixed @@ -92,4 +92,14 @@ where fn no_error_separate_arg_bounds(program: impl AsRef<()>, dir: impl AsRef<()>, args: &[impl AsRef<()>]) {} +trait GenericTrait {} + +fn good_generic + GenericTrait>(arg0: T) { + unimplemented!(); +} + +fn bad_generic + GenericTrait>(arg0: T) { + unimplemented!(); +} + fn main() {} diff --git a/tests/ui/repeated_where_clause_or_trait_bound.rs b/tests/ui/repeated_where_clause_or_trait_bound.rs index 68c974ee5a42..c9573710c31f 100644 --- a/tests/ui/repeated_where_clause_or_trait_bound.rs +++ b/tests/ui/repeated_where_clause_or_trait_bound.rs @@ -92,4 +92,14 @@ where fn no_error_separate_arg_bounds(program: impl AsRef<()>, dir: impl AsRef<()>, args: &[impl AsRef<()>]) {} +trait GenericTrait {} + +fn good_generic + GenericTrait>(arg0: T) { + unimplemented!(); +} + +fn bad_generic + GenericTrait + GenericTrait>(arg0: T) { + unimplemented!(); +} + fn main() {} diff --git a/tests/ui/repeated_where_clause_or_trait_bound.stderr b/tests/ui/repeated_where_clause_or_trait_bound.stderr index 64b7c902957a..eb1f9ecde5b1 100644 --- a/tests/ui/repeated_where_clause_or_trait_bound.stderr +++ b/tests/ui/repeated_where_clause_or_trait_bound.stderr @@ -34,5 +34,11 @@ error: this where clause contains repeated elements LL | T: Clone + Clone + Clone + Copy, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy` -error: aborting due to 5 previous errors +error: this trait bound contains repeated elements + --> $DIR/repeated_where_clause_or_trait_bound.rs:101:19 + | +LL | fn bad_generic + GenericTrait + GenericTrait>(arg0: T) { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `GenericTrait + GenericTrait` + +error: aborting due to 6 previous errors