Auto merge of #8252 - dswij:8229, r=xFrednet

cover trait for `trait_duplication_in_bounds` closes #8229 changelog: [`trait_duplication_in_bounds`] covers trait functions with `Self` bounds
rust-lang · Jan 10, 2022 · 88cfd70 · 88cfd70
2 parents 5991695 + f4dc348
commit 88cfd70
Show file tree

Hide file tree

Showing 3 changed files with 139 additions and 3 deletions.
diff --git 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,53 @@ 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() {
+                                span_lint_and_help(
+                                    cx,
+                                    TRAIT_DUPLICATION_IN_BOUNDS,
+                                    span,
+                                    "this trait bound is already specified in trait declaration",
+                                    None,
+                                    "consider removing this trait bound",
+                                );
+                            }
+                        });
+                }
+            }
+        }
+    }
 }
 
 fn get_trait_res_span_from_bound(bound: &GenericBound<'_>) -> Option<(Res, Span)> {

diff --git a/tests/ui/trait_duplication_in_bounds.rs b/tests/ui/trait_duplication_in_bounds.rs
@@ -28,4 +28,49 @@ where
     unimplemented!();
 }
 
+trait T: Default {
+    fn f()
+    where
+        Self: Default;
+}
+
+trait U: Default {
+    fn f()
+    where
+        Self: Clone;
+}
+
+trait ZZ: Default {
+    fn g();
+    fn h();
+    fn f()
+    where
+        Self: Default + Clone;
+}
+
+trait BadTrait: Default + Clone {
+    fn f()
+    where
+        Self: Default + Clone;
+    fn g()
+    where
+        Self: Default;
+    fn h()
+    where
+        Self: Copy;
+}
+
+#[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
@@ -19,5 +19,45 @@ LL | fn bad_foo<T: Clone + Default, Z: Copy>(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 trait declaration
+  --> $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 trait declaration
+  --> $DIR/trait_duplication_in_bounds.rs:48:15
+   |
+LL |         Self: Default + Clone;
+   |               ^^^^^^^
+   |
+   = help: consider removing this trait bound
+
+error: this trait bound is already specified in trait declaration
+  --> $DIR/trait_duplication_in_bounds.rs:54:15
+   |
+LL |         Self: Default + Clone;
+   |               ^^^^^^^
+   |
+   = help: consider removing this trait bound
+
+error: this trait bound is already specified in trait declaration
+  --> $DIR/trait_duplication_in_bounds.rs:54:25
+   |
+LL |         Self: Default + Clone;
+   |                         ^^^^^
+   |
+   = help: consider removing this trait bound
+
+error: this trait bound is already specified in trait declaration
+  --> $DIR/trait_duplication_in_bounds.rs:57:15
+   |
+LL |         Self: Default;
+   |               ^^^^^^^
+   |
+   = help: consider removing this trait bound
+
+error: aborting due to 7 previous errors