diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index 81b08ae5600..6ba916306ee 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -69,50 +69,66 @@ pub fn contains_adt_constructor<'tcx>(ty: Ty<'tcx>, adt: AdtDef<'tcx>) -> bool { /// This method also recurses into opaque type predicates, so call it with `impl Trait` and `U` /// will also return `true`. pub fn contains_ty_adt_constructor_opaque<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, needle: Ty<'tcx>) -> bool { - ty.walk().any(|inner| match inner.unpack() { - GenericArgKind::Type(inner_ty) => { - if inner_ty == needle { - return true; - } + fn contains_ty_adt_constructor_opaque_inner<'tcx>( + cx: &LateContext<'tcx>, + ty: Ty<'tcx>, + needle: Ty<'tcx>, + seen: &mut FxHashSet, + ) -> bool { + ty.walk().any(|inner| match inner.unpack() { + GenericArgKind::Type(inner_ty) => { + if inner_ty == needle { + return true; + } - if inner_ty.ty_adt_def() == needle.ty_adt_def() { - return true; - } + if inner_ty.ty_adt_def() == needle.ty_adt_def() { + return true; + } - if let ty::Opaque(def_id, _) = *inner_ty.kind() { - for &(predicate, _span) in cx.tcx.explicit_item_bounds(def_id) { - match predicate.kind().skip_binder() { - // For `impl Trait`, it will register a predicate of `T: Trait`, so we go through - // and check substituions to find `U`. - ty::PredicateKind::Clause(ty::Clause::Trait(trait_predicate)) => { - if trait_predicate - .trait_ref - .substs - .types() - .skip(1) // Skip the implicit `Self` generic parameter - .any(|ty| contains_ty_adt_constructor_opaque(cx, ty, needle)) - { - return true; - } - }, - // For `impl Trait`, it will register a predicate of `::Assoc = U`, - // so we check the term for `U`. - ty::PredicateKind::Clause(ty::Clause::Projection(projection_predicate)) => { - if let ty::TermKind::Ty(ty) = projection_predicate.term.unpack() { - if contains_ty_adt_constructor_opaque(cx, ty, needle) { + if let ty::Opaque(def_id, _) = *inner_ty.kind() { + if !seen.insert(def_id) { + return false; + } + + for &(predicate, _span) in cx.tcx.explicit_item_bounds(def_id) { + match predicate.kind().skip_binder() { + // For `impl Trait`, it will register a predicate of `T: Trait`, so we go through + // and check substituions to find `U`. + ty::PredicateKind::Clause(ty::Clause::Trait(trait_predicate)) => { + if trait_predicate + .trait_ref + .substs + .types() + .skip(1) // Skip the implicit `Self` generic parameter + .any(|ty| contains_ty_adt_constructor_opaque_inner(cx, ty, needle, seen)) + { return true; } - }; - }, - _ => (), + }, + // For `impl Trait`, it will register a predicate of `::Assoc = U`, + // so we check the term for `U`. + ty::PredicateKind::Clause(ty::Clause::Projection(projection_predicate)) => { + if let ty::TermKind::Ty(ty) = projection_predicate.term.unpack() { + if contains_ty_adt_constructor_opaque_inner(cx, ty, needle, seen) { + return true; + } + }; + }, + _ => (), + } } } - } - false - }, - GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false, - }) + false + }, + GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false, + }) + } + + // A hash set to ensure that the same opaque type (`impl Trait` in RPIT or TAIT) is not + // visited twice. + let mut seen = FxHashSet::default(); + contains_ty_adt_constructor_opaque_inner(cx, ty, needle, &mut seen) } /// Resolves `::Item` for `T` diff --git a/tests/ui/new_ret_no_self.rs b/tests/ui/new_ret_no_self.rs index f69982d63a8..beec42f08bb 100644 --- a/tests/ui/new_ret_no_self.rs +++ b/tests/ui/new_ret_no_self.rs @@ -1,3 +1,4 @@ +#![feature(type_alias_impl_trait)] #![warn(clippy::new_ret_no_self)] #![allow(dead_code)] @@ -400,3 +401,25 @@ mod issue7344 { } } } + +mod issue10041 { + struct Bomb; + + impl Bomb { + // Hidden default generic paramter. + pub fn new() -> impl PartialOrd { + 0i32 + } + } + + // TAIT with self-referencing bounds + type X = impl std::ops::Add; + + struct Bomb2; + + impl Bomb2 { + pub fn new() -> X { + 0i32 + } + } +} diff --git a/tests/ui/new_ret_no_self.stderr b/tests/ui/new_ret_no_self.stderr index bc13be47927..2eaebfb5cac 100644 --- a/tests/ui/new_ret_no_self.stderr +++ b/tests/ui/new_ret_no_self.stderr @@ -1,5 +1,5 @@ error: methods called `new` usually return `Self` - --> $DIR/new_ret_no_self.rs:49:5 + --> $DIR/new_ret_no_self.rs:50:5 | LL | / pub fn new(_: String) -> impl R { LL | | S3 @@ -9,7 +9,7 @@ LL | | } = note: `-D clippy::new-ret-no-self` implied by `-D warnings` error: methods called `new` usually return `Self` - --> $DIR/new_ret_no_self.rs:81:5 + --> $DIR/new_ret_no_self.rs:82:5 | LL | / pub fn new() -> u32 { LL | | unimplemented!(); @@ -17,7 +17,7 @@ LL | | } | |_____^ error: methods called `new` usually return `Self` - --> $DIR/new_ret_no_self.rs:90:5 + --> $DIR/new_ret_no_self.rs:91:5 | LL | / pub fn new(_: String) -> u32 { LL | | unimplemented!(); @@ -25,7 +25,7 @@ LL | | } | |_____^ error: methods called `new` usually return `Self` - --> $DIR/new_ret_no_self.rs:126:5 + --> $DIR/new_ret_no_self.rs:127:5 | LL | / pub fn new() -> (u32, u32) { LL | | unimplemented!(); @@ -33,7 +33,7 @@ LL | | } | |_____^ error: methods called `new` usually return `Self` - --> $DIR/new_ret_no_self.rs:153:5 + --> $DIR/new_ret_no_self.rs:154:5 | LL | / pub fn new() -> *mut V { LL | | unimplemented!(); @@ -41,7 +41,7 @@ LL | | } | |_____^ error: methods called `new` usually return `Self` - --> $DIR/new_ret_no_self.rs:171:5 + --> $DIR/new_ret_no_self.rs:172:5 | LL | / pub fn new() -> Option { LL | | unimplemented!(); @@ -49,19 +49,19 @@ LL | | } | |_____^ error: methods called `new` usually return `Self` - --> $DIR/new_ret_no_self.rs:224:9 + --> $DIR/new_ret_no_self.rs:225:9 | LL | fn new() -> String; | ^^^^^^^^^^^^^^^^^^^ error: methods called `new` usually return `Self` - --> $DIR/new_ret_no_self.rs:236:9 + --> $DIR/new_ret_no_self.rs:237:9 | LL | fn new(_: String) -> String; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: methods called `new` usually return `Self` - --> $DIR/new_ret_no_self.rs:271:9 + --> $DIR/new_ret_no_self.rs:272:9 | LL | / fn new() -> (u32, u32) { LL | | unimplemented!(); @@ -69,7 +69,7 @@ LL | | } | |_________^ error: methods called `new` usually return `Self` - --> $DIR/new_ret_no_self.rs:298:9 + --> $DIR/new_ret_no_self.rs:299:9 | LL | / fn new() -> *mut V { LL | | unimplemented!(); @@ -77,7 +77,7 @@ LL | | } | |_________^ error: methods called `new` usually return `Self` - --> $DIR/new_ret_no_self.rs:368:9 + --> $DIR/new_ret_no_self.rs:369:9 | LL | / fn new(t: T) -> impl Into { LL | | 1 @@ -85,12 +85,28 @@ LL | | } | |_________^ error: methods called `new` usually return `Self` - --> $DIR/new_ret_no_self.rs:389:9 + --> $DIR/new_ret_no_self.rs:390:9 | LL | / fn new(t: T) -> impl Trait2<(), i32> { LL | | unimplemented!() LL | | } | |_________^ -error: aborting due to 12 previous errors +error: methods called `new` usually return `Self` + --> $DIR/new_ret_no_self.rs:410:9 + | +LL | / pub fn new() -> impl PartialOrd { +LL | | 0i32 +LL | | } + | |_________^ + +error: methods called `new` usually return `Self` + --> $DIR/new_ret_no_self.rs:421:9 + | +LL | / pub fn new() -> X { +LL | | 0i32 +LL | | } + | |_________^ + +error: aborting due to 14 previous errors