Skip to content

Commit

Permalink
Check if method is constructor
Browse files Browse the repository at this point in the history
  • Loading branch information
Anthony Huang committed Jun 29, 2021
1 parent c9ae73e commit 2481835
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 55 deletions.
38 changes: 25 additions & 13 deletions clippy_lints/src/self_named_constructor.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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,
Expand Down
48 changes: 31 additions & 17 deletions tests/ui/self_named_constructor.rs
Original file line number Diff line number Diff line change
@@ -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<Item = Self> {
U
}
}

#[derive(Default)]
pub struct Default;

fn main() {}
34 changes: 9 additions & 25 deletions tests/ui/self_named_constructor.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 2481835

Please sign in to comment.