diff --git a/clippy_lints/src/self_named_constructor.rs b/clippy_lints/src/self_named_constructor.rs index c96618fea661..294a409816e8 100644 --- a/clippy_lints/src/self_named_constructor.rs +++ b/clippy_lints/src/self_named_constructor.rs @@ -1,9 +1,9 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::get_parent_as_impl; -use rustc_hir::{ImplItem, ImplItemKind, Node, TyKind}; +use clippy_utils::ty::{contains_adt_constructor, contains_ty}; +use clippy_utils::return_ty; +use rustc_hir::{ImplItem, ImplItemKind, Node}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::Symbol; declare_clippy_lint! { /// **What it does:** Warns when constructors have the same name as their types. @@ -42,18 +42,30 @@ declare_lint_pass!(SelfNamedConstructor => [SELF_NAMED_CONSTRUCTOR]); impl<'tcx> LateLintPass<'tcx> for SelfNamedConstructor { fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) { + match impl_item.kind { + ImplItemKind::Fn(_, _) => {}, + _ => return, + } + + let parent = cx.tcx.hir().get_parent_item(impl_item.hir_id()); + let item = cx.tcx.hir().expect_item(parent); + let self_ty = cx.tcx.type_of(item.def_id); + let ret_ty = return_ty(cx, impl_item.hir_id()); + if let Some(self_adt) = self_ty.ty_adt_def() { + if !contains_adt_constructor(ret_ty, self_adt) { + return; + } + } else if !contains_ty(ret_ty, self_ty) { + return; + } if_chain! { - // Get type name - if let Some(imp) = get_parent_as_impl(cx.tcx, impl_item.hir_id()); - if let TyKind::Path(ty_path) = &imp.self_ty.kind; - if let Some(ty_id) = cx.qpath_res(ty_path, imp.self_ty.hir_id).opt_def_id(); - if let Some(local_id) = ty_id.as_local(); - let ty_hir_id = cx.tcx.hir().local_def_id_to_hir_id(local_id); - if let Some(Node::Item(x)) = cx.tcx.hir().find(ty_hir_id); + if let Some(self_def) = self_ty.ty_adt_def(); + if let Some(self_local_did) = self_def.did.as_local(); + let self_id = cx.tcx.hir().local_def_id_to_hir_id(self_local_did); + if let Some(Node::Item(x)) = cx.tcx.hir().find(self_id); + let type_name = x.ident.name.as_str().to_lowercase(); + if impl_item.ident.name.as_str() == type_name; - // Get constructor name - if let ImplItemKind::Fn(_, _) = impl_item.kind; - if impl_item.ident.name == Symbol::intern(&x.ident.name.as_str().to_lowercase()); then { span_lint_and_then( cx, diff --git a/tests/ui/self_named_constructor.rs b/tests/ui/self_named_constructor.rs index 34e068cdfe63..e39f58f36eca 100644 --- a/tests/ui/self_named_constructor.rs +++ b/tests/ui/self_named_constructor.rs @@ -1,36 +1,50 @@ #![warn(clippy::self_named_constructor)] -struct Foo {} +struct Q; -impl Foo { - pub fn foo() -> Foo { - Foo {} +impl Q { + pub fn q() -> Q { + Q + } + + fn r() -> R { + R } } -struct Bar {} +struct R; -impl Bar { - pub fn new() -> Bar { - Bar {} +impl R { + pub fn new() -> R { + R } } -struct FooBar {} +struct V; -impl FooBar { - pub fn foobar() -> FooBar { - FooBar {} +impl V { + pub fn v() -> R { + R } } -#[derive(Default)] -pub struct Default {} +struct U; + +trait T { + type Item; +} -impl Foo { - fn bar() -> Bar { - Bar {} +impl T for U { + type Item = Self; +} + +impl U { + pub fn u() -> impl T { + U } } +#[derive(Default)] +pub struct Default; + fn main() {} diff --git a/tests/ui/self_named_constructor.stderr b/tests/ui/self_named_constructor.stderr index 423615af90c4..ff3bba08f87a 100644 --- a/tests/ui/self_named_constructor.stderr +++ b/tests/ui/self_named_constructor.stderr @@ -1,48 +1,32 @@ -error: constructor `foo` has the same name as the type +error: constructor `q` has the same name as the type --> $DIR/self_named_constructor.rs:6:5 | -LL | / pub fn foo() -> Foo { -LL | | Foo {} +LL | / pub fn q() -> Q { +LL | | Q LL | | } | |_____^ | = note: `-D clippy::self-named-constructor` implied by `-D warnings` -help: consider renaming `foo()` to `new()` +help: consider renaming `q()` to `new()` --> $DIR/self_named_constructor.rs:6:5 | -LL | / pub fn foo() -> Foo { -LL | | Foo {} -LL | | } - | |_____^ - -error: constructor `foobar` has the same name as the type - --> $DIR/self_named_constructor.rs:22:5 - | -LL | / pub fn foobar() -> FooBar { -LL | | FooBar {} -LL | | } - | |_____^ - | -help: consider renaming `foobar()` to `new()` - --> $DIR/self_named_constructor.rs:22:5 - | -LL | / pub fn foobar() -> FooBar { -LL | | FooBar {} +LL | / pub fn q() -> Q { +LL | | Q LL | | } | |_____^ error: constructor `default` has the same name as the type - --> $DIR/self_named_constructor.rs:27:10 + --> $DIR/self_named_constructor.rs:47:10 | LL | #[derive(Default)] | ^^^^^^^ | help: consider renaming `default()` to `new()` - --> $DIR/self_named_constructor.rs:27:10 + --> $DIR/self_named_constructor.rs:47:10 | LL | #[derive(Default)] | ^^^^^^^ = note: this error originates in the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors