From f690978023d7465d05abe9a1288100f192e74193 Mon Sep 17 00:00:00 2001 From: dswij Date: Thu, 6 Jan 2022 18:43:44 +0800 Subject: [PATCH] cover trait for `trait_duplication_in_bounds` --- clippy_lints/src/trait_bounds.rs | 68 ++++++++++++++++++--- tests/ui/trait_duplication_in_bounds.rs | 37 +++++++++++ tests/ui/trait_duplication_in_bounds.stderr | 34 ++++++++++- 3 files changed, 128 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index f2848ad3790..c91b24462c0 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -3,10 +3,14 @@ use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::{SpanlessEq, SpanlessHash}; use core::hash::{Hash, Hasher}; use if_chain::if_chain; -use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::unhash::UnhashMap; use rustc_errors::Applicability; -use rustc_hir::{def::Res, GenericBound, Generics, ParamName, Path, QPath, Ty, TyKind, WherePredicate}; +use rustc_hir::def::Res; +use rustc_hir::{ + 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}; use rustc_span::Span; @@ -84,6 +88,46 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds { self.check_type_repetition(cx, gen); check_trait_bound_duplication(cx, gen); } + + fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'tcx>) { + let Generics { where_clause, .. } = &item.generics; + let mut self_bounds_set = FxHashSet::default(); + + for predicate in 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(PathSegment { res: Some(Res::SelfTy(Some(def_id), _)), .. }) = segments.first(); + + if let Some( + Node::Item( + Item { + kind: ItemKind::Trait(_, _, _, self_bounds, _), + .. } + ) + ) = cx.tcx.hir().get_if_local(*def_id); + then { + if self_bounds_set.is_empty() { + for bound in self_bounds.iter() { + let Some((self_res, _)) = get_trait_res_span_from_bound(bound) else { continue }; + self_bounds_set.insert(self_res); + } + } + + bound_predicate + .bounds + .iter() + .filter_map(get_trait_res_span_from_bound) + .for_each(|(trait_item_res, span)| { + if self_bounds_set.get(&trait_item_res).is_some() { + emit_lint(cx, span); + } + }); + } + } + } + } } fn get_trait_res_span_from_bound(bound: &GenericBound<'_>) -> Option<(Res, Span)> { @@ -198,17 +242,21 @@ fn check_trait_bound_duplication(cx: &LateContext<'_>, gen: &'_ Generics<'_>) { 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", - ); + emit_lint(cx, *span_direct); } } } } } } + +fn emit_lint(cx: &LateContext<'_>, span: Span) { + span_lint_and_help( + cx, + TRAIT_DUPLICATION_IN_BOUNDS, + span, + "this trait bound is already specified in the where clause", + None, + "consider removing this trait bound", + ); +} diff --git a/tests/ui/trait_duplication_in_bounds.rs b/tests/ui/trait_duplication_in_bounds.rs index cb2b0054e35..3e490126273 100644 --- a/tests/ui/trait_duplication_in_bounds.rs +++ b/tests/ui/trait_duplication_in_bounds.rs @@ -28,4 +28,41 @@ where unimplemented!(); } +trait T: Default { + fn f() + where + Self: Default; +} + +trait U: Default { + fn f() + where + Self: Clone; +} + +trait ZZ: Default { + fn f() + where + Self: Default + Clone; +} + +trait BadTrait: Default + Clone { + fn f() + where + Self: Default + Clone; +} + +#[derive(Default, Clone)] +struct Life {} + +impl T for Life { + // this should not warn + fn f() {} +} + +impl U for Life { + // this should not warn + fn f() {} +} + fn main() {} diff --git a/tests/ui/trait_duplication_in_bounds.stderr b/tests/ui/trait_duplication_in_bounds.stderr index 027e1c75204..6326139c187 100644 --- a/tests/ui/trait_duplication_in_bounds.stderr +++ b/tests/ui/trait_duplication_in_bounds.stderr @@ -19,5 +19,37 @@ LL | fn bad_foo(arg0: T, arg1: Z) | = help: consider removing this trait bound -error: aborting due to 2 previous errors +error: this trait bound is already specified in the where clause + --> $DIR/trait_duplication_in_bounds.rs:34:15 + | +LL | Self: Default; + | ^^^^^^^ + | + = help: consider removing this trait bound + +error: this trait bound is already specified in the where clause + --> $DIR/trait_duplication_in_bounds.rs:46:15 + | +LL | Self: Default + Clone; + | ^^^^^^^ + | + = help: consider removing this trait bound + +error: this trait bound is already specified in the where clause + --> $DIR/trait_duplication_in_bounds.rs:52:15 + | +LL | Self: Default + Clone; + | ^^^^^^^ + | + = help: consider removing this trait bound + +error: this trait bound is already specified in the where clause + --> $DIR/trait_duplication_in_bounds.rs:52:25 + | +LL | Self: Default + Clone; + | ^^^^^ + | + = help: consider removing this trait bound + +error: aborting due to 6 previous errors