X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Ftrait_bounds.rs;h=73bdcae9e392066b5eab3a9f36d72aff1ca81bec;hb=14d54f0f6eb3b03cb5d719949a25404a8fe563d9;hp=0ef70311fb1cde5a80e9f21b20f21ed511d1093f;hpb=6f25adbd5a1600653e8edf8de7135c397d73c07a;p=rust.git diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 0ef70311fb1..73bdcae9e39 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -1,20 +1,24 @@ -use crate::utils::{in_macro, snippet, snippet_with_applicability, span_lint_and_help, SpanlessHash}; +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::source::{snippet, snippet_with_applicability}; +use clippy_utils::SpanlessHash; use if_chain::if_chain; use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::unhash::UnhashMap; use rustc_errors::Applicability; -use rustc_hir::{GenericBound, Generics, WherePredicate}; +use rustc_hir::{def::Res, GenericBound, Generics, ParamName, Path, QPath, TyKind, WherePredicate}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::Span; declare_clippy_lint! { - /// **What it does:** This lint warns about unnecessary type repetitions in trait bounds + /// ### What it does + /// This lint warns about unnecessary type repetitions in trait bounds /// - /// **Why is this bad?** Repeating the type for every bound makes the code + /// ### Why is this bad? + /// Repeating the type for every bound makes the code /// less readable than combining the bounds /// - /// **Known problems:** None. - /// - /// **Example:** + /// ### Example /// ```rust /// pub fn foo(t: T) where T: Copy, T: Clone {} /// ``` @@ -29,6 +33,35 @@ "Types are repeated unnecessary in trait bounds use `+` instead of using `T: _, T: _`" } +declare_clippy_lint! { + /// ### What it does + /// Checks for cases where generics are being used and multiple + /// syntax specifications for trait bounds are used simultaneously. + /// + /// ### Why is this bad? + /// Duplicate bounds makes the code + /// less readable than specifing them only once. + /// + /// ### Example + /// ```rust + /// fn func(arg: T) where T: Clone + Default {} + /// ``` + /// + /// Could be written as: + /// + /// ```rust + /// fn func(arg: T) {} + /// ``` + /// or + /// + /// ```rust + /// fn func(arg: T) where T: Clone + Default {} + /// ``` + pub TRAIT_DUPLICATION_IN_BOUNDS, + pedantic, + "Check if the same trait bounds are specified twice during a function declaration" +} + #[derive(Copy, Clone)] pub struct TraitBounds { max_trait_bounds: u64, @@ -41,11 +74,26 @@ pub fn new(max_trait_bounds: u64) -> Self { } } -impl_lint_pass!(TraitBounds => [TYPE_REPETITION_IN_BOUNDS]); +impl_lint_pass!(TraitBounds => [TYPE_REPETITION_IN_BOUNDS, TRAIT_DUPLICATION_IN_BOUNDS]); impl<'tcx> LateLintPass<'tcx> for TraitBounds { fn check_generics(&mut self, cx: &LateContext<'tcx>, gen: &'tcx Generics<'_>) { - if in_macro(gen.span) { + self.check_type_repetition(cx, gen); + check_trait_bound_duplication(cx, gen); + } +} + +fn get_trait_res_span_from_bound(bound: &GenericBound<'_>) -> Option<(Res, Span)> { + if let GenericBound::Trait(t, _) = bound { + Some((t.trait_ref.path.res, t.span)) + } else { + None + } +} + +impl TraitBounds { + fn check_type_repetition(self, cx: &LateContext<'_>, gen: &'_ Generics<'_>) { + if gen.span.from_expansion() { return; } let hash = |ty| -> u64 { @@ -53,14 +101,14 @@ fn check_generics(&mut self, cx: &LateContext<'tcx>, gen: &'tcx Generics<'_>) { hasher.hash_ty(ty); hasher.finish() }; - let mut map = FxHashMap::default(); + let mut map: UnhashMap>> = UnhashMap::default(); let mut applicability = Applicability::MaybeIncorrect; for bound in gen.where_clause.predicates { if_chain! { if let WherePredicate::BoundPredicate(ref p) = bound; if p.bounds.len() as u64 <= self.max_trait_bounds; - if !in_macro(p.span); - let h = hash(&p.bounded_ty); + if !p.span.from_expansion(); + let h = hash(p.bounded_ty); if let Some(ref v) = map.insert(h, p.bounds.iter().collect::>()); then { @@ -101,3 +149,47 @@ fn check_generics(&mut self, cx: &LateContext<'tcx>, gen: &'tcx Generics<'_>) { } } } + +fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) { + if gen.span.from_expansion() || gen.params.is_empty() || gen.where_clause.predicates.is_empty() { + return; + } + + let mut map = FxHashMap::default(); + for param in gen.params { + if let ParamName::Plain(ref ident) = param.name { + let res = param + .bounds + .iter() + .filter_map(get_trait_res_span_from_bound) + .collect::>(); + map.insert(*ident, res); + } + } + + for predicate in gen.where_clause.predicates { + if_chain! { + if let WherePredicate::BoundPredicate(ref bound_predicate) = predicate; + if !bound_predicate.span.from_expansion(); + if let TyKind::Path(QPath::Resolved(_, Path { segments, .. })) = bound_predicate.bounded_ty.kind; + if let Some(segment) = segments.first(); + if let Some(trait_resolutions_direct) = map.get(&segment.ident); + then { + for (res_where, _) in bound_predicate.bounds.iter().filter_map(get_trait_res_span_from_bound) { + if let Some((_, span_direct)) = trait_resolutions_direct + .iter() + .find(|(res_direct, _)| *res_direct == res_where) { + span_lint_and_help( + cx, + TRAIT_DUPLICATION_IN_BOUNDS, + *span_direct, + "this trait bound is already specified in the where clause", + None, + "consider removing this trait bound", + ); + } + } + } + } + } +}