Skip to content

Commit 7574c98

Browse files
nbdd0121nbdd0121
committed
Fix new_return_no_self with recursive bounds
1 parent 3905f51 commit 7574c98

3 files changed

Lines changed: 105 additions & 50 deletions

File tree

‎clippy_utils/src/ty.rs‎

Lines changed: 53 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -69,50 +69,66 @@ pub fn contains_adt_constructor<'tcx>(ty: Ty<'tcx>, adt: AdtDef<'tcx>) -> bool {
6969
/// This method also recurses into opaque type predicates, so call it with `impl Trait<U>` and `U`
7070
/// will also return `true`.
7171
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-
}
72+
fn contains_ty_adt_constructor_opaque_inner<'tcx>(
73+
cx: &LateContext<'tcx>,
74+
ty: Ty<'tcx>,
75+
needle: Ty<'tcx>,
76+
seen: &mut FxHashSet<DefId>,
77+
) -> bool {
78+
ty.walk().any(|inner| match inner.unpack() {
79+
GenericArgKind::Type(inner_ty) => {
80+
if inner_ty == needle {
81+
return true;
82+
}
7783

78-
if inner_ty.ty_adt_def() == needle.ty_adt_def() {
79-
return true;
80-
}
84+
if inner_ty.ty_adt_def() == needle.ty_adt_def() {
85+
return true;
86+
}
87+
88+
if let ty::Opaque(def_id, _) = *inner_ty.kind() {
89+
if !seen.insert(def_id) {
90+
return false;
91+
}
8192

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) {
93+
for &(predicate, _span) in cx.tcx.explicit_item_bounds(def_id) {
94+
match predicate.kind().skip_binder() {
95+
// For `impl Trait<U>`, it will register a predicate of `T: Trait<U>`, so we go through
96+
// and check substituions to find `U`.
97+
ty::PredicateKind::Clause(ty::Clause::Trait(trait_predicate)) => {
98+
if trait_predicate
99+
.trait_ref
100+
.substs
101+
.types()
102+
.skip(1) // Skip the implicit `Self` generic parameter
103+
.any(|ty| contains_ty_adt_constructor_opaque_inner(cx, ty, needle, seen))
104+
{
103105
return true;
104106
}
105-
};
106-
},
107-
_ => (),
107+
},
108+
// For `impl Trait<Assoc=U>`, it will register a predicate of `<T as Trait>::Assoc = U`,
109+
// so we check the term for `U`.
110+
ty::PredicateKind::Clause(ty::Clause::Projection(projection_predicate)) => {
111+
if let ty::TermKind::Ty(ty) = projection_predicate.term.unpack() {
112+
if contains_ty_adt_constructor_opaque_inner(cx, ty, needle, seen) {
113+
return true;
114+
}
115+
};
116+
},
117+
_ => (),
118+
}
108119
}
109120
}
110-
}
111121

112-
false
113-
},
114-
GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
115-
})
122+
false
123+
},
124+
GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
125+
})
126+
}
127+
128+
// A hash set to ensure that the same opaque type (`impl Trait` in RPIT or TAIT) is not
129+
// visited twice.
130+
let mut seen = FxHashSet::default();
131+
contains_ty_adt_constructor_opaque_inner(cx, ty, needle, &mut seen)
116132
}
117133

118134
/// Resolves `<T as Iterator>::Item` for `T`

‎tests/ui/new_ret_no_self.rs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#![feature(type_alias_impl_trait)]
12
#![warn(clippy::new_ret_no_self)]
23
#![allow(dead_code)]
34

@@ -400,3 +401,25 @@ mod issue7344 {
400401
}
401402
}
402403
}
404+
405+
mod issue10041 {
406+
struct Bomb;
407+
408+
impl Bomb {
409+
// Hidden <Rhs = Self> default generic paramter.
410+
pub fn new() -> impl PartialOrd {
411+
0i32
412+
}
413+
}
414+
415+
// TAIT with self-referencing bounds
416+
type X = impl std::ops::Add<Output = X>;
417+
418+
struct Bomb2;
419+
420+
impl Bomb2 {
421+
pub fn new() -> X {
422+
0i32
423+
}
424+
}
425+
}

‎tests/ui/new_ret_no_self.stderr‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: methods called `new` usually return `Self`
2-
--> $DIR/new_ret_no_self.rs:49:5
2+
--> $DIR/new_ret_no_self.rs:50:5
33
|
44
LL | / pub fn new(_: String) -> impl R<Item = u32> {
55
LL | | S3
@@ -9,88 +9,104 @@ LL | | }
99
= note: `-D clippy::new-ret-no-self` implied by `-D warnings`
1010

1111
error: methods called `new` usually return `Self`
12-
--> $DIR/new_ret_no_self.rs:81:5
12+
--> $DIR/new_ret_no_self.rs:82:5
1313
|
1414
LL | / pub fn new() -> u32 {
1515
LL | | unimplemented!();
1616
LL | | }
1717
| |_____^
1818

1919
error: methods called `new` usually return `Self`
20-
--> $DIR/new_ret_no_self.rs:90:5
20+
--> $DIR/new_ret_no_self.rs:91:5
2121
|
2222
LL | / pub fn new(_: String) -> u32 {
2323
LL | | unimplemented!();
2424
LL | | }
2525
| |_____^
2626

2727
error: methods called `new` usually return `Self`
28-
--> $DIR/new_ret_no_self.rs:126:5
28+
--> $DIR/new_ret_no_self.rs:127:5
2929
|
3030
LL | / pub fn new() -> (u32, u32) {
3131
LL | | unimplemented!();
3232
LL | | }
3333
| |_____^
3434

3535
error: methods called `new` usually return `Self`
36-
--> $DIR/new_ret_no_self.rs:153:5
36+
--> $DIR/new_ret_no_self.rs:154:5
3737
|
3838
LL | / pub fn new() -> *mut V {
3939
LL | | unimplemented!();
4040
LL | | }
4141
| |_____^
4242

4343
error: methods called `new` usually return `Self`
44-
--> $DIR/new_ret_no_self.rs:171:5
44+
--> $DIR/new_ret_no_self.rs:172:5
4545
|
4646
LL | / pub fn new() -> Option<u32> {
4747
LL | | unimplemented!();
4848
LL | | }
4949
| |_____^
5050

5151
error: methods called `new` usually return `Self`
52-
--> $DIR/new_ret_no_self.rs:224:9
52+
--> $DIR/new_ret_no_self.rs:225:9
5353
|
5454
LL | fn new() -> String;
5555
| ^^^^^^^^^^^^^^^^^^^
5656

5757
error: methods called `new` usually return `Self`
58-
--> $DIR/new_ret_no_self.rs:236:9
58+
--> $DIR/new_ret_no_self.rs:237:9
5959
|
6060
LL | fn new(_: String) -> String;
6161
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6262

6363
error: methods called `new` usually return `Self`
64-
--> $DIR/new_ret_no_self.rs:271:9
64+
--> $DIR/new_ret_no_self.rs:272:9
6565
|
6666
LL | / fn new() -> (u32, u32) {
6767
LL | | unimplemented!();
6868
LL | | }
6969
| |_________^
7070

7171
error: methods called `new` usually return `Self`
72-
--> $DIR/new_ret_no_self.rs:298:9
72+
--> $DIR/new_ret_no_self.rs:299:9
7373
|
7474
LL | / fn new() -> *mut V {
7575
LL | | unimplemented!();
7676
LL | | }
7777
| |_________^
7878

7979
error: methods called `new` usually return `Self`
80-
--> $DIR/new_ret_no_self.rs:368:9
80+
--> $DIR/new_ret_no_self.rs:369:9
8181
|
8282
LL | / fn new(t: T) -> impl Into<i32> {
8383
LL | | 1
8484
LL | | }
8585
| |_________^
8686

8787
error: methods called `new` usually return `Self`
88-
--> $DIR/new_ret_no_self.rs:389:9
88+
--> $DIR/new_ret_no_self.rs:390:9
8989
|
9090
LL | / fn new(t: T) -> impl Trait2<(), i32> {
9191
LL | | unimplemented!()
9292
LL | | }
9393
| |_________^
9494

95-
error: aborting due to 12 previous errors
95+
error: methods called `new` usually return `Self`
96+
--> $DIR/new_ret_no_self.rs:410:9
97+
|
98+
LL | / pub fn new() -> impl PartialOrd {
99+
LL | | 0i32
100+
LL | | }
101+
| |_________^
102+
103+
error: methods called `new` usually return `Self`
104+
--> $DIR/new_ret_no_self.rs:421:9
105+
|
106+
LL | / pub fn new() -> X {
107+
LL | | 0i32
108+
LL | | }
109+
| |_________^
110+
111+
error: aborting due to 14 previous errors
96112

0 commit comments

Comments
 (0)