Ensure new_ret_no_self is not fired if impl Trait<Self> is returned.

This commit is contained in:
Gary Guo 2022-10-27 18:49:07 +01:00
parent 40af5be525
commit e27e13c9ad
2 changed files with 54 additions and 28 deletions

View File

@ -103,7 +103,7 @@
use bind_instead_of_map::BindInsteadOfMap; use bind_instead_of_map::BindInsteadOfMap;
use clippy_utils::consts::{constant, Constant}; use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::{span_lint, span_lint_and_help}; use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
use clippy_utils::ty::{contains_adt_constructor, implements_trait, is_copy, is_type_diagnostic_item}; use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item};
use clippy_utils::{contains_return, is_trait_method, iter_input_pats, meets_msrv, msrvs, return_ty}; use clippy_utils::{contains_return, is_trait_method, iter_input_pats, meets_msrv, msrvs, return_ty};
use if_chain::if_chain; use if_chain::if_chain;
use rustc_hir as hir; use rustc_hir as hir;
@ -3394,36 +3394,10 @@ fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx hir::Impl
if let hir::ImplItemKind::Fn(_, _) = impl_item.kind { if let hir::ImplItemKind::Fn(_, _) = impl_item.kind {
let ret_ty = return_ty(cx, impl_item.hir_id()); let ret_ty = return_ty(cx, impl_item.hir_id());
// walk the return type and check for Self (this does not check associated types) if contains_ty_adt_constructor_opaque(cx, ret_ty, self_ty) {
if let Some(self_adt) = self_ty.ty_adt_def() {
if contains_adt_constructor(ret_ty, self_adt) {
return;
}
} else if ret_ty.contains(self_ty) {
return; return;
} }
// if return type is impl trait, check the associated types
if let ty::Opaque(def_id, _) = *ret_ty.kind() {
// one of the associated types must be Self
for &(predicate, _span) in cx.tcx.explicit_item_bounds(def_id) {
if let ty::PredicateKind::Projection(projection_predicate) = predicate.kind().skip_binder() {
let assoc_ty = match projection_predicate.term.unpack() {
ty::TermKind::Ty(ty) => ty,
ty::TermKind::Const(_c) => continue,
};
// walk the associated type and check for Self
if let Some(self_adt) = self_ty.ty_adt_def() {
if contains_adt_constructor(assoc_ty, self_adt) {
return;
}
} else if assoc_ty.contains(self_ty) {
return;
}
}
}
}
if name == "new" && ret_ty != self_ty { if name == "new" && ret_ty != self_ty {
span_lint( span_lint(
cx, cx,

View File

@ -59,6 +59,58 @@ pub fn contains_adt_constructor<'tcx>(ty: Ty<'tcx>, adt: AdtDef<'tcx>) -> bool {
}) })
} }
/// Walks into `ty` and returns `true` if any inner type is an instance of the given type, or adt
/// constructor of the same type.
///
/// This method also recurses into opaque type predicates, so call it with `impl Trait<U>` 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;
}
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<U>`, it will register a predicate of `T: Trait<U>`, so we go through
// and check substituions to find `U`.
ty::PredicateKind::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<Assoc=U>`, it will register a predicate of `<T as Trait>::Assoc = U`,
// so we check the term for `U`.
ty::PredicateKind::Projection(projection_predicate) => {
if let ty::TermKind::Ty(ty) = projection_predicate.term.unpack() {
if contains_ty_adt_constructor_opaque(cx, ty, needle) {
return true;
}
};
},
_ => (),
}
}
}
false
},
GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
})
}
/// Resolves `<T as Iterator>::Item` for `T` /// Resolves `<T as Iterator>::Item` for `T`
/// Do not invoke without first verifying that the type implements `Iterator` /// Do not invoke without first verifying that the type implements `Iterator`
pub fn get_iterator_item_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> { pub fn get_iterator_item_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {