Skip to content

Commit c0235b0

Browse files
committed
Revert "Auto merge of rust-lang#9733 - nbdd0121:master, r=dswij"
This reverts commit 33137dd, reversing changes made to 4326814.
1 parent 3b67e6a commit c0235b0

File tree

4 files changed

+31
-121
lines changed

4 files changed

+31
-121
lines changed

clippy_lints/src/methods/mod.rs

+30-2
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ use bind_instead_of_map::BindInsteadOfMap;
105105
use clippy_utils::consts::{constant, Constant};
106106
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
107107
use clippy_utils::msrvs::{self, Msrv};
108-
use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item};
108+
use clippy_utils::ty::{contains_adt_constructor, implements_trait, is_copy, is_type_diagnostic_item};
109109
use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, return_ty};
110110
use if_chain::if_chain;
111111
use rustc_hir as hir;
@@ -3414,10 +3414,38 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
34143414
if let hir::ImplItemKind::Fn(_, _) = impl_item.kind {
34153415
let ret_ty = return_ty(cx, impl_item.hir_id());
34163416

3417-
if contains_ty_adt_constructor_opaque(cx, ret_ty, self_ty) {
3417+
// walk the return type and check for Self (this does not check associated types)
3418+
if let Some(self_adt) = self_ty.ty_adt_def() {
3419+
if contains_adt_constructor(ret_ty, self_adt) {
3420+
return;
3421+
}
3422+
} else if ret_ty.contains(self_ty) {
34183423
return;
34193424
}
34203425

3426+
// if return type is impl trait, check the associated types
3427+
if let ty::Opaque(def_id, _) = *ret_ty.kind() {
3428+
// one of the associated types must be Self
3429+
for &(predicate, _span) in cx.tcx.explicit_item_bounds(def_id) {
3430+
if let ty::PredicateKind::Clause(ty::Clause::Projection(projection_predicate)) =
3431+
predicate.kind().skip_binder()
3432+
{
3433+
let assoc_ty = match projection_predicate.term.unpack() {
3434+
ty::TermKind::Ty(ty) => ty,
3435+
ty::TermKind::Const(_c) => continue,
3436+
};
3437+
// walk the associated type and check for Self
3438+
if let Some(self_adt) = self_ty.ty_adt_def() {
3439+
if contains_adt_constructor(assoc_ty, self_adt) {
3440+
return;
3441+
}
3442+
} else if assoc_ty.contains(self_ty) {
3443+
return;
3444+
}
3445+
}
3446+
}
3447+
}
3448+
34213449
if name == "new" && ret_ty != self_ty {
34223450
span_lint(
34233451
cx,

clippy_utils/src/ty.rs

-52
Original file line numberDiff line numberDiff line change
@@ -63,58 +63,6 @@ pub fn contains_adt_constructor<'tcx>(ty: Ty<'tcx>, adt: AdtDef<'tcx>) -> bool {
6363
})
6464
}
6565

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

tests/ui/new_ret_no_self.rs

-50
Original file line numberDiff line numberDiff line change
@@ -350,53 +350,3 @@ impl RetOtherSelf<T> {
350350
RetOtherSelf(RetOtherSelfWrapper(t))
351351
}
352352
}
353-
354-
mod issue7344 {
355-
struct RetImplTraitSelf<T>(T);
356-
357-
impl<T> RetImplTraitSelf<T> {
358-
// should not trigger lint
359-
fn new(t: T) -> impl Into<Self> {
360-
Self(t)
361-
}
362-
}
363-
364-
struct RetImplTraitNoSelf<T>(T);
365-
366-
impl<T> RetImplTraitNoSelf<T> {
367-
// should trigger lint
368-
fn new(t: T) -> impl Into<i32> {
369-
1
370-
}
371-
}
372-
373-
trait Trait2<T, U> {}
374-
impl<T, U> Trait2<T, U> for () {}
375-
376-
struct RetImplTraitSelf2<T>(T);
377-
378-
impl<T> RetImplTraitSelf2<T> {
379-
// should not trigger lint
380-
fn new(t: T) -> impl Trait2<(), Self> {
381-
unimplemented!()
382-
}
383-
}
384-
385-
struct RetImplTraitNoSelf2<T>(T);
386-
387-
impl<T> RetImplTraitNoSelf2<T> {
388-
// should trigger lint
389-
fn new(t: T) -> impl Trait2<(), i32> {
390-
unimplemented!()
391-
}
392-
}
393-
394-
struct RetImplTraitSelfAdt<'a>(&'a str);
395-
396-
impl<'a> RetImplTraitSelfAdt<'a> {
397-
// should not trigger lint
398-
fn new<'b: 'a>(s: &'b str) -> impl Into<RetImplTraitSelfAdt<'b>> {
399-
RetImplTraitSelfAdt(s)
400-
}
401-
}
402-
}

tests/ui/new_ret_no_self.stderr

+1-17
Original file line numberDiff line numberDiff line change
@@ -76,21 +76,5 @@ LL | | unimplemented!();
7676
LL | | }
7777
| |_________^
7878

79-
error: methods called `new` usually return `Self`
80-
--> $DIR/new_ret_no_self.rs:368:9
81-
|
82-
LL | / fn new(t: T) -> impl Into<i32> {
83-
LL | | 1
84-
LL | | }
85-
| |_________^
86-
87-
error: methods called `new` usually return `Self`
88-
--> $DIR/new_ret_no_self.rs:389:9
89-
|
90-
LL | / fn new(t: T) -> impl Trait2<(), i32> {
91-
LL | | unimplemented!()
92-
LL | | }
93-
| |_________^
94-
95-
error: aborting due to 12 previous errors
79+
error: aborting due to 10 previous errors
9680

0 commit comments

Comments
 (0)