diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 6d22abf2d33..f0810c906ef 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -936,10 +936,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { if let hir::ImplItemKind::Method(_, _) = implitem.node { let ret_ty = return_ty(cx, implitem.id); - // if return type is impl trait + // walk the return type and check for Self (this does not check associated types) + for inner_type in ret_ty.walk() { + if same_tys(cx, ty, inner_type) { return; } + } + + // if return type is impl trait, check the associated types if let TyKind::Opaque(def_id, _) = ret_ty.sty { - // then one of the associated types must be Self + // one of the associated types must be Self for predicate in cx.tcx.predicates_of(def_id).predicates.iter() { match predicate { (Predicate::Projection(poly_projection_predicate), _) => { diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 59c55168232..035ca2b0496 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -1920,7 +1920,6 @@ enum ImplicitHasherType<'tcx> { impl<'tcx> ImplicitHasherType<'tcx> { /// Checks that `ty` is a target type without a BuildHasher. - #[allow(clippy::new_ret_no_self)] fn new<'a>(cx: &LateContext<'a, 'tcx>, hir_ty: &hir::Ty) -> Option { if let TyKind::Path(QPath::Resolved(None, ref path)) = hir_ty.node { let params: Vec<_> = path.segments.last().as_ref()?.args.as_ref()? diff --git a/tests/ui/new_ret_no_self.rs b/tests/ui/new_ret_no_self.rs index 1a4b91cc9da..bed43f550f2 100644 --- a/tests/ui/new_ret_no_self.rs +++ b/tests/ui/new_ret_no_self.rs @@ -91,3 +91,87 @@ impl V { unimplemented!(); } } + +struct TupleReturnerOk; + +impl TupleReturnerOk { + // should not trigger lint + pub fn new() -> (Self, u32) { unimplemented!(); } +} + +struct TupleReturnerOk2; + +impl TupleReturnerOk2 { + // should not trigger lint (it doesn't matter which element in the tuple is Self) + pub fn new() -> (u32, Self) { unimplemented!(); } +} + +struct TupleReturnerOk3; + +impl TupleReturnerOk3 { + // should not trigger lint (tuple can contain multiple Self) + pub fn new() -> (Self, Self) { unimplemented!(); } +} + +struct TupleReturnerBad; + +impl TupleReturnerBad { + // should trigger lint + pub fn new() -> (u32, u32) { unimplemented!(); } +} + +struct MutPointerReturnerOk; + +impl MutPointerReturnerOk { + // should not trigger lint + pub fn new() -> *mut Self { unimplemented!(); } +} + +struct MutPointerReturnerOk2; + +impl MutPointerReturnerOk2 { + // should not trigger lint + pub fn new() -> *const Self { unimplemented!(); } +} + +struct MutPointerReturnerBad; + +impl MutPointerReturnerBad { + // should trigger lint + pub fn new() -> *mut V { unimplemented!(); } +} + +struct GenericReturnerOk; + +impl GenericReturnerOk { + // should not trigger lint + pub fn new() -> Option { unimplemented!(); } +} + +struct GenericReturnerBad; + +impl GenericReturnerBad { + // should trigger lint + pub fn new() -> Option { unimplemented!(); } +} + +struct NestedReturnerOk; + +impl NestedReturnerOk { + // should not trigger lint + pub fn new() -> (Option, u32) { unimplemented!(); } +} + +struct NestedReturnerOk2; + +impl NestedReturnerOk2 { + // should not trigger lint + pub fn new() -> ((Self, u32), u32) { unimplemented!(); } +} + +struct NestedReturnerOk3; + +impl NestedReturnerOk3 { + // should not trigger lint + pub fn new() -> Option<(Self, u32)> { unimplemented!(); } +} diff --git a/tests/ui/new_ret_no_self.stderr b/tests/ui/new_ret_no_self.stderr index ad26438d4ef..bab9627ca22 100644 --- a/tests/ui/new_ret_no_self.stderr +++ b/tests/ui/new_ret_no_self.stderr @@ -24,5 +24,23 @@ error: methods called `new` usually return `Self` 92 | | } | |_____^ -error: aborting due to 3 previous errors +error: methods called `new` usually return `Self` + --> $DIR/new_ret_no_self.rs:120:5 + | +120 | pub fn new() -> (u32, u32) { unimplemented!(); } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: methods called `new` usually return `Self` + --> $DIR/new_ret_no_self.rs:141:5 + | +141 | pub fn new() -> *mut V { unimplemented!(); } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: methods called `new` usually return `Self` + --> $DIR/new_ret_no_self.rs:155:5 + | +155 | pub fn new() -> Option { unimplemented!(); } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors