Auto merge of #12308 - y21:more_implied_bounds, r=xFrednet

Look for `implied_bounds_in_impls` in more positions

With this, we lint `impl Trait` implied bounds in more positions:
- Type alias impl trait
- Associated type position impl trait
- Argument position impl trait
  - these are not opaque types, but instead are desugared to `where` clauses, so we need extra logic for finding them (`check_generics`), however the rest of the logic is the same

Before this, we'd only lint RPIT `impl Trait`s.
"Hide whitespaces" and reviewing commits individually might make this easier

changelog: [`implied_bounds_in_impls`]: start linting implied bounds in APIT, ATPIT, TAIT

4 files changed, 169 insertions, 80 deletions

diff --git a/clippy_lints/src/implied_bounds_in_impls.rs b/clippy_lints/src/implied_bounds_in_impls.rs
index 174db30a7cd..74582f7f1de 100644
--- a/clippy_lints/src/implied_bounds_in_impls.rs
+++ b/clippy_lints/src/implied_bounds_in_impls.rs
@@ -2,16 +2,14 @@ use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::snippet;
 use rustc_errors::{Applicability, SuggestionStyle};
 use rustc_hir::def_id::DefId;
-use rustc_hir::intravisit::FnKind;
 use rustc_hir::{
-    Body, FnDecl, FnRetTy, GenericArg, GenericBound, ImplItem, ImplItemKind, ItemKind, OpaqueTy, TraitBoundModifier,
-    TraitItem, TraitItemKind, TyKind, TypeBinding,
+    GenericArg, GenericBound, GenericBounds, ItemKind, PredicateOrigin, TraitBoundModifier, TyKind, TypeBinding,
+    WherePredicate,
 };
 use rustc_hir_analysis::hir_ty_to_ty;
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::{self, ClauseKind, Generics, Ty, TyCtxt};
 use rustc_session::declare_lint_pass;
-use rustc_span::def_id::LocalDefId;
 use rustc_span::Span;
 
 declare_clippy_lint! {
@@ -54,7 +52,7 @@ declare_lint_pass!(ImpliedBoundsInImpls => [IMPLIED_BOUNDS_IN_IMPLS]);
 fn emit_lint(
     cx: &LateContext<'_>,
     poly_trait: &rustc_hir::PolyTraitRef<'_>,
-    opaque_ty: &rustc_hir::OpaqueTy<'_>,
+    bounds: GenericBounds<'_>,
     index: usize,
     // The bindings that were implied, used for suggestion purposes since removing a bound with associated types
     // means we might need to then move it to a different bound
@@ -73,10 +71,10 @@ fn emit_lint(
             // to include the `+` token that is ahead or behind,
             // so we don't end up with something like `impl + B` or `impl A + `
 
-            let implied_span_extended = if let Some(next_bound) = opaque_ty.bounds.get(index + 1) {
+            let implied_span_extended = if let Some(next_bound) = bounds.get(index + 1) {
                 poly_trait.span.to(next_bound.span().shrink_to_lo())
             } else if index > 0
-                && let Some(prev_bound) = opaque_ty.bounds.get(index - 1)
+                && let Some(prev_bound) = bounds.get(index - 1)
             {
                 prev_bound.span().shrink_to_hi().to(poly_trait.span.shrink_to_hi())
             } else {
@@ -240,9 +238,8 @@ struct ImplTraitBound<'tcx> {
 /// For `impl Deref + DerefMut + Eq` this returns `[Deref, PartialEq]`.
 /// The `Deref` comes from `DerefMut` because `trait DerefMut: Deref {}`, and `PartialEq` comes from
 /// `Eq`.
-fn collect_supertrait_bounds<'tcx>(cx: &LateContext<'tcx>, opaque_ty: &OpaqueTy<'tcx>) -> Vec<ImplTraitBound<'tcx>> {
-    opaque_ty
-        .bounds
+fn collect_supertrait_bounds<'tcx>(cx: &LateContext<'tcx>, bounds: GenericBounds<'tcx>) -> Vec<ImplTraitBound<'tcx>> {
+    bounds
         .iter()
         .filter_map(|bound| {
             if let GenericBound::Trait(poly_trait, TraitBoundModifier::None) = bound
@@ -295,64 +292,62 @@ fn find_bound_in_supertraits<'a, 'tcx>(
     })
 }
 
-fn check(cx: &LateContext<'_>, decl: &FnDecl<'_>) {
-    if let FnRetTy::Return(ty) = decl.output
-        && let TyKind::OpaqueDef(item_id, ..) = ty.kind
-        && let item = cx.tcx.hir().item(item_id)
-        && let ItemKind::OpaqueTy(opaque_ty) = item.kind
+fn check<'tcx>(cx: &LateContext<'tcx>, bounds: GenericBounds<'tcx>) {
+    if bounds.len() == 1 {
         // Very often there is only a single bound, e.g. `impl Deref<..>`, in which case
-        // we can avoid doing a bunch of stuff unnecessarily.
-        && opaque_ty.bounds.len() > 1
-    {
-        let supertraits = collect_supertrait_bounds(cx, opaque_ty);
+        // we can avoid doing a bunch of stuff unnecessarily; there will trivially be
+        // no duplicate bounds
+        return;
+    }
 
-        // Lint all bounds in the `impl Trait` type that we've previously also seen in the set of
-        // supertraits of each of the bounds.
-        // This involves some extra logic when generic arguments are present, since
-        // simply comparing trait `DefId`s won't be enough. We also need to compare the generics.
-        for (index, bound) in opaque_ty.bounds.iter().enumerate() {
-            if let GenericBound::Trait(poly_trait, TraitBoundModifier::None) = bound
-                && let [.., path] = poly_trait.trait_ref.path.segments
-                && let implied_args = path.args.map_or([].as_slice(), |a| a.args)
-                && let implied_bindings = path.args.map_or([].as_slice(), |a| a.bindings)
-                && let Some(def_id) = poly_trait.trait_ref.path.res.opt_def_id()
-                && let Some(bound) = find_bound_in_supertraits(cx, def_id, implied_args, &supertraits)
-                // If the implied bound has a type binding that also exists in the implied-by trait,
-                // then we shouldn't lint. See #11880 for an example.
-                && let assocs = cx.tcx.associated_items(bound.trait_def_id)
-                && !implied_bindings.iter().any(|binding| {
-                    assocs
-                        .filter_by_name_unhygienic(binding.ident.name)
-                        .next()
-                        .is_some_and(|assoc| assoc.kind == ty::AssocKind::Type)
-                    })
-            {
-                emit_lint(cx, poly_trait, opaque_ty, index, implied_bindings, bound);
-            }
+    let supertraits = collect_supertrait_bounds(cx, bounds);
+
+    // Lint all bounds in the `impl Trait` type that we've previously also seen in the set of
+    // supertraits of each of the bounds.
+    // This involves some extra logic when generic arguments are present, since
+    // simply comparing trait `DefId`s won't be enough. We also need to compare the generics.
+    for (index, bound) in bounds.iter().enumerate() {
+        if let GenericBound::Trait(poly_trait, TraitBoundModifier::None) = bound
+            && let [.., path] = poly_trait.trait_ref.path.segments
+            && let implied_args = path.args.map_or([].as_slice(), |a| a.args)
+            && let implied_bindings = path.args.map_or([].as_slice(), |a| a.bindings)
+            && let Some(def_id) = poly_trait.trait_ref.path.res.opt_def_id()
+            && let Some(bound) = find_bound_in_supertraits(cx, def_id, implied_args, &supertraits)
+            // If the implied bound has a type binding that also exists in the implied-by trait,
+            // then we shouldn't lint. See #11880 for an example.
+            && let assocs = cx.tcx.associated_items(bound.trait_def_id)
+            && !implied_bindings.iter().any(|binding| {
+                assocs
+                    .filter_by_name_unhygienic(binding.ident.name)
+                    .next()
+                    .is_some_and(|assoc| assoc.kind == ty::AssocKind::Type)
+                })
+        {
+            emit_lint(cx, poly_trait, bounds, index, implied_bindings, bound);
         }
     }
 }
 
-impl LateLintPass<'_> for ImpliedBoundsInImpls {
-    fn check_fn(
-        &mut self,
-        cx: &LateContext<'_>,
-        _: FnKind<'_>,
-        decl: &FnDecl<'_>,
-        _: &Body<'_>,
-        _: Span,
-        _: LocalDefId,
-    ) {
-        check(cx, decl);
-    }
-    fn check_trait_item(&mut self, cx: &LateContext<'_>, item: &TraitItem<'_>) {
-        if let TraitItemKind::Fn(sig, ..) = &item.kind {
-            check(cx, sig.decl);
+impl<'tcx> LateLintPass<'tcx> for ImpliedBoundsInImpls {
+    fn check_generics(&mut self, cx: &LateContext<'tcx>, generics: &rustc_hir::Generics<'tcx>) {
+        for predicate in generics.predicates {
+            if let WherePredicate::BoundPredicate(predicate) = predicate
+                // In theory, the origin doesn't really matter,
+                // we *could* also lint on explicit where clauses written out by the user,
+                // not just impl trait desugared ones, but that contradicts with the lint name...
+                && let PredicateOrigin::ImplTrait = predicate.origin
+            {
+                check(cx, predicate.bounds);
+            }
         }
     }
-    fn check_impl_item(&mut self, cx: &LateContext<'_>, item: &ImplItem<'_>) {
-        if let ImplItemKind::Fn(sig, ..) = &item.kind {
-            check(cx, sig.decl);
+
+    fn check_ty(&mut self, cx: &LateContext<'_>, ty: &rustc_hir::Ty<'_>) {
+        if let TyKind::OpaqueDef(item_id, ..) = ty.kind
+            && let item = cx.tcx.hir().item(item_id)
+            && let ItemKind::OpaqueTy(opaque_ty) = item.kind
+        {
+            check(cx, opaque_ty.bounds);
         }
     }
 }
diff --git a/tests/ui/implied_bounds_in_impls.fixed b/tests/ui/implied_bounds_in_impls.fixed
index 18f29e900c1..6fd4cbd80fe 100644
--- a/tests/ui/implied_bounds_in_impls.fixed
+++ b/tests/ui/implied_bounds_in_impls.fixed
@@ -1,5 +1,6 @@
 #![warn(clippy::implied_bounds_in_impls)]
 #![allow(dead_code)]
+#![feature(impl_trait_in_assoc_type, type_alias_impl_trait)]
 
 use std::ops::{Deref, DerefMut};
 
@@ -150,4 +151,26 @@ fn issue11880() {
     fn f5() -> impl Y<T = u32, U = String> {}
 }
 
+fn apit(_: impl DerefMut) {}
+
+trait Rpitit {
+    fn f() -> impl DerefMut;
+}
+
+trait Atpit {
+    type Assoc;
+    fn define() -> Self::Assoc;
+}
+impl Atpit for () {
+    type Assoc = impl DerefMut;
+    fn define() -> Self::Assoc {
+        &mut [] as &mut [()]
+    }
+}
+
+type Tait = impl DerefMut;
+fn define() -> Tait {
+    &mut [] as &mut [()]
+}
+
 fn main() {}
diff --git a/tests/ui/implied_bounds_in_impls.rs b/tests/ui/implied_bounds_in_impls.rs
index b7951e7dfba..52076c9f998 100644
--- a/tests/ui/implied_bounds_in_impls.rs
+++ b/tests/ui/implied_bounds_in_impls.rs
@@ -1,5 +1,6 @@
 #![warn(clippy::implied_bounds_in_impls)]
 #![allow(dead_code)]
+#![feature(impl_trait_in_assoc_type, type_alias_impl_trait)]
 
 use std::ops::{Deref, DerefMut};
 
@@ -150,4 +151,26 @@ fn issue11880() {
     fn f5() -> impl X<U = String> + Y<T = u32> {}
 }
 
+fn apit(_: impl Deref + DerefMut) {}
+
+trait Rpitit {
+    fn f() -> impl Deref + DerefMut;
+}
+
+trait Atpit {
+    type Assoc;
+    fn define() -> Self::Assoc;
+}
+impl Atpit for () {
+    type Assoc = impl Deref + DerefMut;
+    fn define() -> Self::Assoc {
+        &mut [] as &mut [()]
+    }
+}
+
+type Tait = impl Deref + DerefMut;
+fn define() -> Tait {
+    &mut [] as &mut [()]
+}
+
 fn main() {}
diff --git a/tests/ui/implied_bounds_in_impls.stderr b/tests/ui/implied_bounds_in_impls.stderr
index fa1938f77d9..805532a5680 100644
--- a/tests/ui/implied_bounds_in_impls.stderr
+++ b/tests/ui/implied_bounds_in_impls.stderr
@@ -1,5 +1,5 @@
 error: this bound is already specified as the supertrait of `DerefMut<Target = T>`
-  --> tests/ui/implied_bounds_in_impls.rs:12:36
+  --> tests/ui/implied_bounds_in_impls.rs:13:36
    |
 LL | fn deref_derefmut<T>(x: T) -> impl Deref<Target = T> + DerefMut<Target = T> {
    |                                    ^^^^^^^^^^^^^^^^^
@@ -13,7 +13,7 @@ LL + fn deref_derefmut<T>(x: T) -> impl DerefMut<Target = T> {
    |
 
 error: this bound is already specified as the supertrait of `GenericSubtrait<U, W, U>`
-  --> tests/ui/implied_bounds_in_impls.rs:29:37
+  --> tests/ui/implied_bounds_in_impls.rs:30:37
    |
 LL | fn generics_implied<U, W>() -> impl GenericTrait<W> + GenericSubtrait<U, W, U>
    |                                     ^^^^^^^^^^^^^^^
@@ -25,7 +25,7 @@ LL + fn generics_implied<U, W>() -> impl GenericSubtrait<U, W, U>
    |
 
 error: this bound is already specified as the supertrait of `GenericSubtrait<(), i32, V>`
-  --> tests/ui/implied_bounds_in_impls.rs:35:40
+  --> tests/ui/implied_bounds_in_impls.rs:36:40
    |
 LL | fn generics_implied_multi<V>() -> impl GenericTrait<i32> + GenericTrait2<V> + GenericSubtrait<(), i32, V> {}
    |                                        ^^^^^^^^^^^^^^^^^
@@ -37,7 +37,7 @@ LL + fn generics_implied_multi<V>() -> impl GenericTrait2<V> + GenericSubtrait<(
    |
 
 error: this bound is already specified as the supertrait of `GenericSubtrait<(), i32, V>`
-  --> tests/ui/implied_bounds_in_impls.rs:35:60
+  --> tests/ui/implied_bounds_in_impls.rs:36:60
    |
 LL | fn generics_implied_multi<V>() -> impl GenericTrait<i32> + GenericTrait2<V> + GenericSubtrait<(), i32, V> {}
    |                                                            ^^^^^^^^^^^^^^^^
@@ -49,7 +49,7 @@ LL + fn generics_implied_multi<V>() -> impl GenericTrait<i32> + GenericSubtrait<
    |
 
 error: this bound is already specified as the supertrait of `GenericSubtrait<(), T, V>`
-  --> tests/ui/implied_bounds_in_impls.rs:37:44
+  --> tests/ui/implied_bounds_in_impls.rs:38:44
    |
 LL | fn generics_implied_multi2<T, V>() -> impl GenericTrait<T> + GenericTrait2<V> + GenericSubtrait<(), T, V>
    |                                            ^^^^^^^^^^^^^^^
@@ -61,7 +61,7 @@ LL + fn generics_implied_multi2<T, V>() -> impl GenericTrait2<V> + GenericSubtra
    |
 
 error: this bound is already specified as the supertrait of `GenericSubtrait<(), T, V>`
-  --> tests/ui/implied_bounds_in_impls.rs:37:62
+  --> tests/ui/implied_bounds_in_impls.rs:38:62
    |
 LL | fn generics_implied_multi2<T, V>() -> impl GenericTrait<T> + GenericTrait2<V> + GenericSubtrait<(), T, V>
    |                                                              ^^^^^^^^^^^^^^^^
@@ -73,7 +73,7 @@ LL + fn generics_implied_multi2<T, V>() -> impl GenericTrait<T> + GenericSubtrai
    |
 
 error: this bound is already specified as the supertrait of `GenericSubtrait<(), i32, ()>`
-  --> tests/ui/implied_bounds_in_impls.rs:47:28
+  --> tests/ui/implied_bounds_in_impls.rs:48:28
    |
 LL | fn generics_same() -> impl GenericTrait<i32> + GenericSubtrait<(), i32, ()> {}
    |                            ^^^^^^^^^^^^^^^^^
@@ -85,7 +85,7 @@ LL + fn generics_same() -> impl GenericSubtrait<(), i32, ()> {}
    |
 
 error: this bound is already specified as the supertrait of `DerefMut<Target = u8>`
-  --> tests/ui/implied_bounds_in_impls.rs:51:20
+  --> tests/ui/implied_bounds_in_impls.rs:52:20
    |
 LL |     fn f() -> impl Deref + DerefMut<Target = u8>;
    |                    ^^^^^
@@ -97,7 +97,7 @@ LL +     fn f() -> impl DerefMut<Target = u8>;
    |
 
 error: this bound is already specified as the supertrait of `DerefMut<Target = u8>`
-  --> tests/ui/implied_bounds_in_impls.rs:56:20
+  --> tests/ui/implied_bounds_in_impls.rs:57:20
    |
 LL |     fn f() -> impl Deref + DerefMut<Target = u8> {
    |                    ^^^^^
@@ -109,7 +109,7 @@ LL +     fn f() -> impl DerefMut<Target = u8> {
    |
 
 error: this bound is already specified as the supertrait of `DerefMut<Target = u8>`
-  --> tests/ui/implied_bounds_in_impls.rs:62:20
+  --> tests/ui/implied_bounds_in_impls.rs:63:20
    |
 LL |     fn f() -> impl Deref + DerefMut<Target = u8> {
    |                    ^^^^^
@@ -121,7 +121,7 @@ LL +     fn f() -> impl DerefMut<Target = u8> {
    |
 
 error: this bound is already specified as the supertrait of `PartialOrd`
-  --> tests/ui/implied_bounds_in_impls.rs:73:41
+  --> tests/ui/implied_bounds_in_impls.rs:74:41
    |
 LL |     fn default_generic_param1() -> impl PartialEq + PartialOrd + Debug {}
    |                                         ^^^^^^^^^
@@ -133,7 +133,7 @@ LL +     fn default_generic_param1() -> impl PartialOrd + Debug {}
    |
 
 error: this bound is already specified as the supertrait of `PartialOrd`
-  --> tests/ui/implied_bounds_in_impls.rs:74:54
+  --> tests/ui/implied_bounds_in_impls.rs:75:54
    |
 LL |     fn default_generic_param2() -> impl PartialOrd + PartialEq + Debug {}
    |                                                      ^^^^^^^^^
@@ -145,7 +145,7 @@ LL +     fn default_generic_param2() -> impl PartialOrd + Debug {}
    |
 
 error: this bound is already specified as the supertrait of `DoubleEndedIterator`
-  --> tests/ui/implied_bounds_in_impls.rs:87:26
+  --> tests/ui/implied_bounds_in_impls.rs:88:26
    |
 LL |     fn my_iter() -> impl Iterator<Item = u32> + DoubleEndedIterator {
    |                          ^^^^^^^^^^^^^^^^^^^^
@@ -157,7 +157,7 @@ LL +     fn my_iter() -> impl DoubleEndedIterator<Item = u32> {
    |
 
 error: this bound is already specified as the supertrait of `Copy`
-  --> tests/ui/implied_bounds_in_impls.rs:92:27
+  --> tests/ui/implied_bounds_in_impls.rs:93:27
    |
 LL |     fn f() -> impl Copy + Clone {
    |                           ^^^^^
@@ -169,7 +169,7 @@ LL +     fn f() -> impl Copy {
    |
 
 error: this bound is already specified as the supertrait of `Trait2<i32>`
-  --> tests/ui/implied_bounds_in_impls.rs:106:21
+  --> tests/ui/implied_bounds_in_impls.rs:107:21
    |
 LL |     fn f2() -> impl Trait1<i32, U = i64> + Trait2<i32> {}
    |                     ^^^^^^^^^^^^^^^^^^^^
@@ -181,7 +181,7 @@ LL +     fn f2() -> impl Trait2<i32, U = i64> {}
    |
 
 error: this bound is already specified as the supertrait of `Trait4<i8, X = i32>`
-  --> tests/ui/implied_bounds_in_impls.rs:121:21
+  --> tests/ui/implied_bounds_in_impls.rs:122:21
    |
 LL |     fn f3() -> impl Trait3<i8, i16, i64, X = i32, Y = i128> + Trait4<i8, X = i32> {}
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -193,7 +193,7 @@ LL +     fn f3() -> impl Trait4<i8, X = i32, Y = i128> {}
    |
 
 error: this bound is already specified as the supertrait of `Y`
-  --> tests/ui/implied_bounds_in_impls.rs:148:21
+  --> tests/ui/implied_bounds_in_impls.rs:149:21
    |
 LL |     fn f3() -> impl X + Y {}
    |                     ^
@@ -205,7 +205,7 @@ LL +     fn f3() -> impl Y {}
    |
 
 error: this bound is already specified as the supertrait of `Y<T = u32>`
-  --> tests/ui/implied_bounds_in_impls.rs:149:21
+  --> tests/ui/implied_bounds_in_impls.rs:150:21
    |
 LL |     fn f4() -> impl X + Y<T = u32> {}
    |                     ^
@@ -217,7 +217,7 @@ LL +     fn f4() -> impl Y<T = u32> {}
    |
 
 error: this bound is already specified as the supertrait of `Y<T = u32>`
-  --> tests/ui/implied_bounds_in_impls.rs:150:21
+  --> tests/ui/implied_bounds_in_impls.rs:151:21
    |
 LL |     fn f5() -> impl X<U = String> + Y<T = u32> {}
    |                     ^^^^^^^^^^^^^
@@ -228,5 +228,53 @@ LL -     fn f5() -> impl X<U = String> + Y<T = u32> {}
 LL +     fn f5() -> impl Y<T = u32, U = String> {}
    |
 
-error: aborting due to 19 previous errors
+error: this bound is already specified as the supertrait of `DerefMut`
+  --> tests/ui/implied_bounds_in_impls.rs:154:17
+   |
+LL | fn apit(_: impl Deref + DerefMut) {}
+   |                 ^^^^^
+   |
+help: try removing this bound
+   |
+LL - fn apit(_: impl Deref + DerefMut) {}
+LL + fn apit(_: impl DerefMut) {}
+   |
+
+error: this bound is already specified as the supertrait of `DerefMut`
+  --> tests/ui/implied_bounds_in_impls.rs:157:20
+   |
+LL |     fn f() -> impl Deref + DerefMut;
+   |                    ^^^^^
+   |
+help: try removing this bound
+   |
+LL -     fn f() -> impl Deref + DerefMut;
+LL +     fn f() -> impl DerefMut;
+   |
+
+error: this bound is already specified as the supertrait of `DerefMut`
+  --> tests/ui/implied_bounds_in_impls.rs:165:23
+   |
+LL |     type Assoc = impl Deref + DerefMut;
+   |                       ^^^^^
+   |
+help: try removing this bound
+   |
+LL -     type Assoc = impl Deref + DerefMut;
+LL +     type Assoc = impl DerefMut;
+   |
+
+error: this bound is already specified as the supertrait of `DerefMut`
+  --> tests/ui/implied_bounds_in_impls.rs:171:18
+   |
+LL | type Tait = impl Deref + DerefMut;
+   |                  ^^^^^
+   |
+help: try removing this bound
+   |
+LL - type Tait = impl Deref + DerefMut;
+LL + type Tait = impl DerefMut;
+   |
+
+error: aborting due to 23 previous errors