Skip to content

Commit e27e13c

Browse files
committed
Ensure new_ret_no_self is not fired if impl Trait<Self> is returned.
1 parent 40af5be commit e27e13c

File tree

2 files changed

+54
-28
lines changed

2 files changed

+54
-28
lines changed

clippy_lints/src/methods/mod.rs

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ mod zst_offset;
103103
use bind_instead_of_map::BindInsteadOfMap;
104104
use clippy_utils::consts::{constant, Constant};
105105
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
106-
use clippy_utils::ty::{contains_adt_constructor, implements_trait, is_copy, is_type_diagnostic_item};
106+
use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item};
107107
use clippy_utils::{contains_return, is_trait_method, iter_input_pats, meets_msrv, msrvs, return_ty};
108108
use if_chain::if_chain;
109109
use rustc_hir as hir;
@@ -3394,36 +3394,10 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
33943394
if let hir::ImplItemKind::Fn(_, _) = impl_item.kind {
33953395
let ret_ty = return_ty(cx, impl_item.hir_id());
33963396

3397-
// walk the return type and check for Self (this does not check associated types)
3398-
if let Some(self_adt) = self_ty.ty_adt_def() {
3399-
if contains_adt_constructor(ret_ty, self_adt) {
3400-
return;
3401-
}
3402-
} else if ret_ty.contains(self_ty) {
3397+
if contains_ty_adt_constructor_opaque(cx, ret_ty, self_ty) {
34033398
return;
34043399
}
34053400

3406-
// if return type is impl trait, check the associated types
3407-
if let ty::Opaque(def_id, _) = *ret_ty.kind() {
3408-
// one of the associated types must be Self
3409-
for &(predicate, _span) in cx.tcx.explicit_item_bounds(def_id) {
3410-
if let ty::PredicateKind::Projection(projection_predicate) = predicate.kind().skip_binder() {
3411-
let assoc_ty = match projection_predicate.term.unpack() {
3412-
ty::TermKind::Ty(ty) => ty,
3413-
ty::TermKind::Const(_c) => continue,
3414-
};
3415-
// walk the associated type and check for Self
3416-
if let Some(self_adt) = self_ty.ty_adt_def() {
3417-
if contains_adt_constructor(assoc_ty, self_adt) {
3418-
return;
3419-
}
3420-
} else if assoc_ty.contains(self_ty) {
3421-
return;
3422-
}
3423-
}
3424-
}
3425-
}
3426-
34273401
if name == "new" && ret_ty != self_ty {
34283402
span_lint(
34293403
cx,

clippy_utils/src/ty.rs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,58 @@ pub fn contains_adt_constructor<'tcx>(ty: Ty<'tcx>, adt: AdtDef<'tcx>) -> bool {
5959
})
6060
}
6161

62+
/// Walks into `ty` and returns `true` if any inner type is an instance of the given type, or adt
63+
/// constructor of the same type.
64+
///
65+
/// This method also recurses into opaque type predicates, so call it with `impl Trait<U>` and `U`
66+
/// will also return `true`.
67+
pub fn contains_ty_adt_constructor_opaque<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, needle: Ty<'tcx>) -> bool {
68+
ty.walk().any(|inner| match inner.unpack() {
69+
GenericArgKind::Type(inner_ty) => {
70+
if inner_ty == needle {
71+
return true;
72+
}
73+
74+
if inner_ty.ty_adt_def() == needle.ty_adt_def() {
75+
return true;
76+
}
77+
78+
if let ty::Opaque(def_id, _) = *inner_ty.kind() {
79+
for &(predicate, _span) in cx.tcx.explicit_item_bounds(def_id) {
80+
match predicate.kind().skip_binder() {
81+
// For `impl Trait<U>`, it will register a predicate of `T: Trait<U>`, so we go through
82+
// and check substituions to find `U`.
83+
ty::PredicateKind::Trait(trait_predicate) => {
84+
if trait_predicate
85+
.trait_ref
86+
.substs
87+
.types()
88+
.skip(1) // Skip the implicit `Self` generic parameter
89+
.any(|ty| contains_ty_adt_constructor_opaque(cx, ty, needle))
90+
{
91+
return true;
92+
}
93+
},
94+
// For `impl Trait<Assoc=U>`, it will register a predicate of `<T as Trait>::Assoc = U`,
95+
// so we check the term for `U`.
96+
ty::PredicateKind::Projection(projection_predicate) => {
97+
if let ty::TermKind::Ty(ty) = projection_predicate.term.unpack() {
98+
if contains_ty_adt_constructor_opaque(cx, ty, needle) {
99+
return true;
100+
}
101+
};
102+
},
103+
_ => (),
104+
}
105+
}
106+
}
107+
108+
false
109+
},
110+
GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
111+
})
112+
}
113+
62114
/// Resolves `<T as Iterator>::Item` for `T`
63115
/// Do not invoke without first verifying that the type implements `Iterator`
64116
pub fn get_iterator_item_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {

0 commit comments

Comments
 (0)