about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-02-13 17:05:03 +0000
committerbors <bors@rust-lang.org>2024-02-13 17:05:03 +0000
commit3e264ba13ac0e6c819101d3979ddb7d243cbf0dd (patch)
tree793888465ac4254f3d25859565bbc73b7ef16fae
parent3e3a09e2bb26ba3b8207019ea53fd9d7da4831d3 (diff)
parent92616c0aaac07bf589a2d40502dd5dec804e8318 (diff)
downloadrust-3e264ba13ac0e6c819101d3979ddb7d243cbf0dd.tar.gz
rust-3e264ba13ac0e6c819101d3979ddb7d243cbf0dd.zip
Auto merge of #11881 - y21:issue11880, r=Alexendoo
[`implied_bounds_in_impls`]: avoid linting on overlapping associated tys

Fixes #11880

Before this change, we were simply ignoring associated types (except for suggestion purposes), because of an incorrect assumption (see the comment that I also removed).

For something like
```rs
trait X { type T; }
trait Y: X { type T; }

// Can't constrain `X::T` through `Y`
fn f() -> impl X<T = i32> + Y<T = u32> { ... }
```
We now avoid linting if the implied bound (`X<T = i32>`) "names" associated types that also exists in the implying trait (`trait Y`). Here that would be the case.
But if we only wrote `impl X + Y<T = u32>` then that's ok because `X::T` was never constrained in the first place.

I haven't really thought about how this interacts with GATs, but I think it's fine. Fine as in, it might create false negatives, but hopefully no false positives.

(The diff is slightly annoying because of formatting things. Really the only thing that changed in the if chain is extracting the `implied_by_def_id` which is needed for getting associated types from the trait, and of course actually checking for overlap)

cc `@Jarcho` ? idk if you want to review this or not. I assume you looked into this code a bit to find this bug.

changelog: [`implied_bounds_in_impls`]: avoid linting when associated type from supertrait can't be constrained through the implying trait bound
-rw-r--r--clippy_lints/src/implied_bounds_in_impls.rs9
-rw-r--r--tests/ui/implied_bounds_in_impls.fixed29
-rw-r--r--tests/ui/implied_bounds_in_impls.rs29
-rw-r--r--tests/ui/implied_bounds_in_impls.stderr38
4 files changed, 104 insertions, 1 deletions
diff --git a/clippy_lints/src/implied_bounds_in_impls.rs b/clippy_lints/src/implied_bounds_in_impls.rs
index cc6ece45fcd..174db30a7cd 100644
--- a/clippy_lints/src/implied_bounds_in_impls.rs
+++ b/clippy_lints/src/implied_bounds_in_impls.rs
@@ -317,6 +317,15 @@ fn check(cx: &LateContext<'_>, decl: &FnDecl<'_>) {
                 && 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);
             }
diff --git a/tests/ui/implied_bounds_in_impls.fixed b/tests/ui/implied_bounds_in_impls.fixed
index fa117aaddcd..18f29e900c1 100644
--- a/tests/ui/implied_bounds_in_impls.fixed
+++ b/tests/ui/implied_bounds_in_impls.fixed
@@ -121,4 +121,33 @@ mod issue11435 {
     fn f3() -> impl Trait4<i8, X = i32, Y = i128> {}
 }
 
+fn issue11880() {
+    trait X {
+        type T;
+        type U;
+    }
+    trait Y: X {
+        type T;
+        type V;
+    }
+    impl X for () {
+        type T = i32;
+        type U = String;
+    }
+    impl Y for () {
+        type T = u32;
+        type V = Vec<u8>;
+    }
+
+    // Can't constrain `X::T` through `Y`
+    fn f() -> impl X<T = i32> + Y {}
+    fn f2() -> impl X<T = i32> + Y<T = u32> {}
+
+    // X::T is never constrained in the first place, so it can be omitted
+    // and left unconstrained
+    fn f3() -> impl Y {}
+    fn f4() -> impl Y<T = u32> {}
+    fn f5() -> impl Y<T = u32, U = String> {}
+}
+
 fn main() {}
diff --git a/tests/ui/implied_bounds_in_impls.rs b/tests/ui/implied_bounds_in_impls.rs
index c96aac151a7..b7951e7dfba 100644
--- a/tests/ui/implied_bounds_in_impls.rs
+++ b/tests/ui/implied_bounds_in_impls.rs
@@ -121,4 +121,33 @@ mod issue11435 {
     fn f3() -> impl Trait3<i8, i16, i64, X = i32, Y = i128> + Trait4<i8, X = i32> {}
 }
 
+fn issue11880() {
+    trait X {
+        type T;
+        type U;
+    }
+    trait Y: X {
+        type T;
+        type V;
+    }
+    impl X for () {
+        type T = i32;
+        type U = String;
+    }
+    impl Y for () {
+        type T = u32;
+        type V = Vec<u8>;
+    }
+
+    // Can't constrain `X::T` through `Y`
+    fn f() -> impl X<T = i32> + Y {}
+    fn f2() -> impl X<T = i32> + Y<T = u32> {}
+
+    // X::T is never constrained in the first place, so it can be omitted
+    // and left unconstrained
+    fn f3() -> impl X + Y {}
+    fn f4() -> impl X + Y<T = u32> {}
+    fn f5() -> impl X<U = String> + Y<T = u32> {}
+}
+
 fn main() {}
diff --git a/tests/ui/implied_bounds_in_impls.stderr b/tests/ui/implied_bounds_in_impls.stderr
index fb44f2aba17..3b09e4582e3 100644
--- a/tests/ui/implied_bounds_in_impls.stderr
+++ b/tests/ui/implied_bounds_in_impls.stderr
@@ -192,5 +192,41 @@ LL -     fn f3() -> impl Trait3<i8, i16, i64, X = i32, Y = i128> + Trait4<i8, X
 LL +     fn f3() -> impl Trait4<i8, X = i32, Y = i128> {}
    |
 
-error: aborting due to 16 previous errors
+error: this bound is already specified as the supertrait of `Y`
+  --> $DIR/implied_bounds_in_impls.rs:148:21
+   |
+LL |     fn f3() -> impl X + Y {}
+   |                     ^
+   |
+help: try removing this bound
+   |
+LL -     fn f3() -> impl X + Y {}
+LL +     fn f3() -> impl Y {}
+   |
+
+error: this bound is already specified as the supertrait of `Y<T = u32>`
+  --> $DIR/implied_bounds_in_impls.rs:149:21
+   |
+LL |     fn f4() -> impl X + Y<T = u32> {}
+   |                     ^
+   |
+help: try removing this bound
+   |
+LL -     fn f4() -> impl X + Y<T = u32> {}
+LL +     fn f4() -> impl Y<T = u32> {}
+   |
+
+error: this bound is already specified as the supertrait of `Y<T = u32>`
+  --> $DIR/implied_bounds_in_impls.rs:150:21
+   |
+LL |     fn f5() -> impl X<U = String> + Y<T = u32> {}
+   |                     ^^^^^^^^^^^^^
+   |
+help: try removing this bound
+   |
+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