Skip to content

Commit 2481835

Browse files
author
Anthony Huang
committed
Check if method is constructor
1 parent c9ae73e commit 2481835

File tree

3 files changed

+65
-55
lines changed

3 files changed

+65
-55
lines changed

clippy_lints/src/self_named_constructor.rs

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
2-
use clippy_utils::get_parent_as_impl;
3-
use rustc_hir::{ImplItem, ImplItemKind, Node, TyKind};
2+
use clippy_utils::ty::{contains_adt_constructor, contains_ty};
3+
use clippy_utils::return_ty;
4+
use rustc_hir::{ImplItem, ImplItemKind, Node};
45
use rustc_lint::{LateContext, LateLintPass};
56
use rustc_session::{declare_lint_pass, declare_tool_lint};
6-
use rustc_span::Symbol;
77

88
declare_clippy_lint! {
99
/// **What it does:** Warns when constructors have the same name as their types.
@@ -42,18 +42,30 @@ declare_lint_pass!(SelfNamedConstructor => [SELF_NAMED_CONSTRUCTOR]);
4242

4343
impl<'tcx> LateLintPass<'tcx> for SelfNamedConstructor {
4444
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) {
45+
match impl_item.kind {
46+
ImplItemKind::Fn(_, _) => {},
47+
_ => return,
48+
}
49+
50+
let parent = cx.tcx.hir().get_parent_item(impl_item.hir_id());
51+
let item = cx.tcx.hir().expect_item(parent);
52+
let self_ty = cx.tcx.type_of(item.def_id);
53+
let ret_ty = return_ty(cx, impl_item.hir_id());
54+
if let Some(self_adt) = self_ty.ty_adt_def() {
55+
if !contains_adt_constructor(ret_ty, self_adt) {
56+
return;
57+
}
58+
} else if !contains_ty(ret_ty, self_ty) {
59+
return;
60+
}
4561
if_chain! {
46-
// Get type name
47-
if let Some(imp) = get_parent_as_impl(cx.tcx, impl_item.hir_id());
48-
if let TyKind::Path(ty_path) = &imp.self_ty.kind;
49-
if let Some(ty_id) = cx.qpath_res(ty_path, imp.self_ty.hir_id).opt_def_id();
50-
if let Some(local_id) = ty_id.as_local();
51-
let ty_hir_id = cx.tcx.hir().local_def_id_to_hir_id(local_id);
52-
if let Some(Node::Item(x)) = cx.tcx.hir().find(ty_hir_id);
62+
if let Some(self_def) = self_ty.ty_adt_def();
63+
if let Some(self_local_did) = self_def.did.as_local();
64+
let self_id = cx.tcx.hir().local_def_id_to_hir_id(self_local_did);
65+
if let Some(Node::Item(x)) = cx.tcx.hir().find(self_id);
66+
let type_name = x.ident.name.as_str().to_lowercase();
67+
if impl_item.ident.name.as_str() == type_name;
5368

54-
// Get constructor name
55-
if let ImplItemKind::Fn(_, _) = impl_item.kind;
56-
if impl_item.ident.name == Symbol::intern(&x.ident.name.as_str().to_lowercase());
5769
then {
5870
span_lint_and_then(
5971
cx,

tests/ui/self_named_constructor.rs

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,50 @@
11
#![warn(clippy::self_named_constructor)]
22

3-
struct Foo {}
3+
struct Q;
44

5-
impl Foo {
6-
pub fn foo() -> Foo {
7-
Foo {}
5+
impl Q {
6+
pub fn q() -> Q {
7+
Q
8+
}
9+
10+
fn r() -> R {
11+
R
812
}
913
}
1014

11-
struct Bar {}
15+
struct R;
1216

13-
impl Bar {
14-
pub fn new() -> Bar {
15-
Bar {}
17+
impl R {
18+
pub fn new() -> R {
19+
R
1620
}
1721
}
1822

19-
struct FooBar {}
23+
struct V;
2024

21-
impl FooBar {
22-
pub fn foobar() -> FooBar {
23-
FooBar {}
25+
impl V {
26+
pub fn v() -> R {
27+
R
2428
}
2529
}
2630

27-
#[derive(Default)]
28-
pub struct Default {}
31+
struct U;
32+
33+
trait T {
34+
type Item;
35+
}
2936

30-
impl Foo {
31-
fn bar() -> Bar {
32-
Bar {}
37+
impl T for U {
38+
type Item = Self;
39+
}
40+
41+
impl U {
42+
pub fn u() -> impl T<Item = Self> {
43+
U
3344
}
3445
}
3546

47+
#[derive(Default)]
48+
pub struct Default;
49+
3650
fn main() {}
Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,32 @@
1-
error: constructor `foo` has the same name as the type
1+
error: constructor `q` has the same name as the type
22
--> $DIR/self_named_constructor.rs:6:5
33
|
4-
LL | / pub fn foo() -> Foo {
5-
LL | | Foo {}
4+
LL | / pub fn q() -> Q {
5+
LL | | Q
66
LL | | }
77
| |_____^
88
|
99
= note: `-D clippy::self-named-constructor` implied by `-D warnings`
10-
help: consider renaming `foo()` to `new()`
10+
help: consider renaming `q()` to `new()`
1111
--> $DIR/self_named_constructor.rs:6:5
1212
|
13-
LL | / pub fn foo() -> Foo {
14-
LL | | Foo {}
15-
LL | | }
16-
| |_____^
17-
18-
error: constructor `foobar` has the same name as the type
19-
--> $DIR/self_named_constructor.rs:22:5
20-
|
21-
LL | / pub fn foobar() -> FooBar {
22-
LL | | FooBar {}
23-
LL | | }
24-
| |_____^
25-
|
26-
help: consider renaming `foobar()` to `new()`
27-
--> $DIR/self_named_constructor.rs:22:5
28-
|
29-
LL | / pub fn foobar() -> FooBar {
30-
LL | | FooBar {}
13+
LL | / pub fn q() -> Q {
14+
LL | | Q
3115
LL | | }
3216
| |_____^
3317

3418
error: constructor `default` has the same name as the type
35-
--> $DIR/self_named_constructor.rs:27:10
19+
--> $DIR/self_named_constructor.rs:47:10
3620
|
3721
LL | #[derive(Default)]
3822
| ^^^^^^^
3923
|
4024
help: consider renaming `default()` to `new()`
41-
--> $DIR/self_named_constructor.rs:27:10
25+
--> $DIR/self_named_constructor.rs:47:10
4226
|
4327
LL | #[derive(Default)]
4428
| ^^^^^^^
4529
= note: this error originates in the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info)
4630

47-
error: aborting due to 3 previous errors
31+
error: aborting due to 2 previous errors
4832

0 commit comments

Comments
 (0)