Skip to content

Commit f70a074

Browse files
borsbors
committed
Auto merge of #7403 - Anthuang:redundant-method-names, r=Manishearth
New lint: [`self_named_constructor`] Adds the `self_named_constructor` lint for detecting when an implemented method has the same name as the type it is implemented for. changelog: [`self_named_constructor`] closes: #7142
2 parents 46363df + e9e10d2 commit f70a074

18 files changed

+252
-64
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -2772,6 +2772,7 @@ Released 2018-09-13
27722772
[`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push
27732773
[`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some
27742774
[`self_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_assignment
2775+
[`self_named_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_constructor
27752776
[`semicolon_if_nothing_returned`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned
27762777
[`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse
27772778
[`shadow_reuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_reuse

‎clippy_lints/src/lib.rs

+6
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,7 @@ mod regex;
330330
mod repeat_once;
331331
mod returns;
332332
mod self_assignment;
333+
mod self_named_constructor;
333334
mod semicolon_if_nothing_returned;
334335
mod serde_api;
335336
mod shadow;
@@ -900,6 +901,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
900901
returns::LET_AND_RETURN,
901902
returns::NEEDLESS_RETURN,
902903
self_assignment::SELF_ASSIGNMENT,
904+
self_named_constructor::SELF_NAMED_CONSTRUCTOR,
903905
semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED,
904906
serde_api::SERDE_API_MISUSE,
905907
shadow::SHADOW_REUSE,
@@ -1406,6 +1408,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
14061408
LintId::of(returns::LET_AND_RETURN),
14071409
LintId::of(returns::NEEDLESS_RETURN),
14081410
LintId::of(self_assignment::SELF_ASSIGNMENT),
1411+
LintId::of(self_named_constructor::SELF_NAMED_CONSTRUCTOR),
14091412
LintId::of(serde_api::SERDE_API_MISUSE),
14101413
LintId::of(single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
14111414
LintId::of(size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT),
@@ -1559,6 +1562,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15591562
LintId::of(redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES),
15601563
LintId::of(returns::LET_AND_RETURN),
15611564
LintId::of(returns::NEEDLESS_RETURN),
1565+
LintId::of(self_named_constructor::SELF_NAMED_CONSTRUCTOR),
15621566
LintId::of(single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
15631567
LintId::of(tabs_in_doc_comments::TABS_IN_DOC_COMMENTS),
15641568
LintId::of(to_digit_is_some::TO_DIGIT_IS_SOME),
@@ -2101,6 +2105,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
21012105
let scripts = conf.allowed_scripts.clone();
21022106
store.register_early_pass(move || box disallowed_script_idents::DisallowedScriptIdents::new(&scripts));
21032107
store.register_late_pass(|| box strlen_on_c_strings::StrlenOnCStrings);
2108+
store.register_late_pass(move || box self_named_constructor::SelfNamedConstructor);
2109+
21042110
}
21052111

21062112
#[rustfmt::skip]

‎clippy_lints/src/self_named_constructor.rs

+91
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
use clippy_utils::diagnostics::span_lint;
2+
use clippy_utils::return_ty;
3+
use clippy_utils::ty::{contains_adt_constructor, contains_ty};
4+
use rustc_hir::{Impl, ImplItem, ImplItemKind, ItemKind, Node};
5+
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_session::{declare_lint_pass, declare_tool_lint};
7+
8+
declare_clippy_lint! {
9+
/// **What it does:** Warns when constructors have the same name as their types.
10+
///
11+
/// **Why is this bad?** Repeating the name of the type is redundant.
12+
///
13+
/// **Known problems:** None.
14+
///
15+
/// **Example:**
16+
///
17+
/// ```rust,ignore
18+
/// struct Foo {}
19+
///
20+
/// impl Foo {
21+
/// pub fn foo() -> Foo {
22+
/// Foo {}
23+
/// }
24+
/// }
25+
/// ```
26+
/// Use instead:
27+
/// ```rust,ignore
28+
/// struct Foo {}
29+
///
30+
/// impl Foo {
31+
/// pub fn new() -> Foo {
32+
/// Foo {}
33+
/// }
34+
/// }
35+
/// ```
36+
pub SELF_NAMED_CONSTRUCTOR,
37+
style,
38+
"method should not have the same name as the type it is implemented for"
39+
}
40+
41+
declare_lint_pass!(SelfNamedConstructor => [SELF_NAMED_CONSTRUCTOR]);
42+
43+
impl<'tcx> LateLintPass<'tcx> for SelfNamedConstructor {
44+
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) {
45+
match impl_item.kind {
46+
ImplItemKind::Fn(ref sig, _) => {
47+
if sig.decl.implicit_self.has_implicit_self() {
48+
return;
49+
}
50+
},
51+
_ => return,
52+
}
53+
54+
let parent = cx.tcx.hir().get_parent_item(impl_item.hir_id());
55+
let item = cx.tcx.hir().expect_item(parent);
56+
let self_ty = cx.tcx.type_of(item.def_id);
57+
let ret_ty = return_ty(cx, impl_item.hir_id());
58+
59+
// Do not check trait impls
60+
if matches!(item.kind, ItemKind::Impl(Impl { of_trait: Some(_), .. })) {
61+
return;
62+
}
63+
64+
// Ensure method is constructor-like
65+
if let Some(self_adt) = self_ty.ty_adt_def() {
66+
if !contains_adt_constructor(ret_ty, self_adt) {
67+
return;
68+
}
69+
} else if !contains_ty(ret_ty, self_ty) {
70+
return;
71+
}
72+
73+
if_chain! {
74+
if let Some(self_def) = self_ty.ty_adt_def();
75+
if let Some(self_local_did) = self_def.did.as_local();
76+
let self_id = cx.tcx.hir().local_def_id_to_hir_id(self_local_did);
77+
if let Some(Node::Item(x)) = cx.tcx.hir().find(self_id);
78+
let type_name = x.ident.name.as_str().to_lowercase();
79+
if impl_item.ident.name.as_str() == type_name || impl_item.ident.name.as_str().replace("_", "") == type_name;
80+
81+
then {
82+
span_lint(
83+
cx,
84+
SELF_NAMED_CONSTRUCTOR,
85+
impl_item.span,
86+
&format!("constructor `{}` has the same name as the type", impl_item.ident.name),
87+
);
88+
}
89+
}
90+
}
91+
}

‎tests/ui/crashes/ice-6179.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
struct Foo {}
88

99
impl Foo {
10-
fn foo() -> Self {
10+
fn new() -> Self {
1111
impl Foo {
1212
fn bar() {}
1313
}

‎tests/ui/issue_4266.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ async fn all_to_one<'a>(a: &'a str, _b: &'a str) -> &'a str {
2525
struct Foo;
2626
impl Foo {
2727
// ok
28-
pub async fn foo(&mut self) {}
28+
pub async fn new(&mut self) -> Self {
29+
Foo {}
30+
}
2931
}
3032

3133
// rust-lang/rust#61115

‎tests/ui/missing-doc-impl.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,9 @@ pub trait E: Sized {
5959
}
6060

6161
impl Foo {
62-
pub fn foo() {}
62+
pub fn new() -> Self {
63+
Foo { a: 0, b: 0 }
64+
}
6365
fn bar() {}
6466
}
6567

‎tests/ui/missing-doc-impl.stderr

+7-5
Original file line numberDiff line numberDiff line change
@@ -78,23 +78,25 @@ LL | type AssociatedTypeDef = Self;
7878
error: missing documentation for an associated function
7979
--> $DIR/missing-doc-impl.rs:62:5
8080
|
81-
LL | pub fn foo() {}
82-
| ^^^^^^^^^^^^^^^
81+
LL | / pub fn new() -> Self {
82+
LL | | Foo { a: 0, b: 0 }
83+
LL | | }
84+
| |_____^
8385

8486
error: missing documentation for an associated function
85-
--> $DIR/missing-doc-impl.rs:63:5
87+
--> $DIR/missing-doc-impl.rs:65:5
8688
|
8789
LL | fn bar() {}
8890
| ^^^^^^^^^^^
8991

9092
error: missing documentation for an associated function
91-
--> $DIR/missing-doc-impl.rs:67:5
93+
--> $DIR/missing-doc-impl.rs:69:5
9294
|
9395
LL | pub fn foo() {}
9496
| ^^^^^^^^^^^^^^^
9597

9698
error: missing documentation for an associated function
97-
--> $DIR/missing-doc-impl.rs:71:5
99+
--> $DIR/missing-doc-impl.rs:73:5
98100
|
99101
LL | / fn foo2() -> u32 {
100102
LL | | 1

‎tests/ui/missing_const_for_fn/cant_be_const.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ mod with_drop {
8484

8585
impl A {
8686
// This can not be const because the type implements `Drop`.
87-
pub fn a(self) -> B {
87+
pub fn b(self) -> B {
8888
B
8989
}
9090
}

‎tests/ui/needless_bool/fixable.fixed

+2-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
dead_code,
77
clippy::no_effect,
88
clippy::if_same_then_else,
9-
clippy::needless_return
9+
clippy::needless_return,
10+
clippy::self_named_constructor
1011
)]
1112

1213
use std::cell::Cell;

‎tests/ui/needless_bool/fixable.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
dead_code,
77
clippy::no_effect,
88
clippy::if_same_then_else,
9-
clippy::needless_return
9+
clippy::needless_return,
10+
clippy::self_named_constructor
1011
)]
1112

1213
use std::cell::Cell;

‎tests/ui/needless_bool/fixable.stderr

+12-12
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this if-then-else expression returns a bool literal
2-
--> $DIR/fixable.rs:39:5
2+
--> $DIR/fixable.rs:40:5
33
|
44
LL | / if x {
55
LL | | true
@@ -11,7 +11,7 @@ LL | | };
1111
= note: `-D clippy::needless-bool` implied by `-D warnings`
1212

1313
error: this if-then-else expression returns a bool literal
14-
--> $DIR/fixable.rs:44:5
14+
--> $DIR/fixable.rs:45:5
1515
|
1616
LL | / if x {
1717
LL | | false
@@ -21,7 +21,7 @@ LL | | };
2121
| |_____^ help: you can reduce it to: `!x`
2222

2323
error: this if-then-else expression returns a bool literal
24-
--> $DIR/fixable.rs:49:5
24+
--> $DIR/fixable.rs:50:5
2525
|
2626
LL | / if x && y {
2727
LL | | false
@@ -31,7 +31,7 @@ LL | | };
3131
| |_____^ help: you can reduce it to: `!(x && y)`
3232

3333
error: this if-then-else expression returns a bool literal
34-
--> $DIR/fixable.rs:69:5
34+
--> $DIR/fixable.rs:70:5
3535
|
3636
LL | / if x {
3737
LL | | return true;
@@ -41,7 +41,7 @@ LL | | };
4141
| |_____^ help: you can reduce it to: `return x`
4242

4343
error: this if-then-else expression returns a bool literal
44-
--> $DIR/fixable.rs:77:5
44+
--> $DIR/fixable.rs:78:5
4545
|
4646
LL | / if x {
4747
LL | | return false;
@@ -51,7 +51,7 @@ LL | | };
5151
| |_____^ help: you can reduce it to: `return !x`
5252

5353
error: this if-then-else expression returns a bool literal
54-
--> $DIR/fixable.rs:85:5
54+
--> $DIR/fixable.rs:86:5
5555
|
5656
LL | / if x && y {
5757
LL | | return true;
@@ -61,7 +61,7 @@ LL | | };
6161
| |_____^ help: you can reduce it to: `return x && y`
6262

6363
error: this if-then-else expression returns a bool literal
64-
--> $DIR/fixable.rs:93:5
64+
--> $DIR/fixable.rs:94:5
6565
|
6666
LL | / if x && y {
6767
LL | | return false;
@@ -71,33 +71,33 @@ LL | | };
7171
| |_____^ help: you can reduce it to: `return !(x && y)`
7272

7373
error: equality checks against true are unnecessary
74-
--> $DIR/fixable.rs:101:8
74+
--> $DIR/fixable.rs:102:8
7575
|
7676
LL | if x == true {};
7777
| ^^^^^^^^^ help: try simplifying it as shown: `x`
7878
|
7979
= note: `-D clippy::bool-comparison` implied by `-D warnings`
8080

8181
error: equality checks against false can be replaced by a negation
82-
--> $DIR/fixable.rs:105:8
82+
--> $DIR/fixable.rs:106:8
8383
|
8484
LL | if x == false {};
8585
| ^^^^^^^^^^ help: try simplifying it as shown: `!x`
8686

8787
error: equality checks against true are unnecessary
88-
--> $DIR/fixable.rs:115:8
88+
--> $DIR/fixable.rs:116:8
8989
|
9090
LL | if x == true {};
9191
| ^^^^^^^^^ help: try simplifying it as shown: `x`
9292

9393
error: equality checks against false can be replaced by a negation
94-
--> $DIR/fixable.rs:116:8
94+
--> $DIR/fixable.rs:117:8
9595
|
9696
LL | if x == false {};
9797
| ^^^^^^^^^^ help: try simplifying it as shown: `!x`
9898

9999
error: this if-then-else expression returns a bool literal
100-
--> $DIR/fixable.rs:125:12
100+
--> $DIR/fixable.rs:126:12
101101
|
102102
LL | } else if returns_bool() {
103103
| ____________^

‎tests/ui/self_named_constructor.rs

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
#![warn(clippy::self_named_constructor)]
2+
3+
struct ShouldSpawn;
4+
struct ShouldNotSpawn;
5+
6+
impl ShouldSpawn {
7+
pub fn should_spawn() -> ShouldSpawn {
8+
ShouldSpawn
9+
}
10+
11+
fn should_not_spawn() -> ShouldNotSpawn {
12+
ShouldNotSpawn
13+
}
14+
}
15+
16+
impl ShouldNotSpawn {
17+
pub fn new() -> ShouldNotSpawn {
18+
ShouldNotSpawn
19+
}
20+
}
21+
22+
struct ShouldNotSpawnWithTrait;
23+
24+
trait ShouldNotSpawnTrait {
25+
type Item;
26+
}
27+
28+
impl ShouldNotSpawnTrait for ShouldNotSpawnWithTrait {
29+
type Item = Self;
30+
}
31+
32+
impl ShouldNotSpawnWithTrait {
33+
pub fn should_not_spawn_with_trait() -> impl ShouldNotSpawnTrait<Item = Self> {
34+
ShouldNotSpawnWithTrait
35+
}
36+
}
37+
38+
// Same trait name and same type name should not spawn the lint
39+
#[derive(Default)]
40+
pub struct Default;
41+
42+
trait TraitSameTypeName {
43+
fn should_not_spawn() -> Self;
44+
}
45+
impl TraitSameTypeName for ShouldNotSpawn {
46+
fn should_not_spawn() -> Self {
47+
ShouldNotSpawn
48+
}
49+
}
50+
51+
struct SelfMethodShouldNotSpawn;
52+
53+
impl SelfMethodShouldNotSpawn {
54+
fn self_method_should_not_spawn(self) -> Self {
55+
SelfMethodShouldNotSpawn
56+
}
57+
}
58+
59+
fn main() {}

0 commit comments

Comments
 (0)