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
This commit is contained in:
bors 2021-07-19 06:49:51 +00:00
commit f70a07454b
18 changed files with 252 additions and 64 deletions

View file

@ -2772,6 +2772,7 @@ Released 2018-09-13
[`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push
[`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some
[`self_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_assignment
[`self_named_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_constructor
[`semicolon_if_nothing_returned`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned
[`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse
[`shadow_reuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_reuse

View file

@ -330,6 +330,7 @@ mod regex;
mod repeat_once;
mod returns;
mod self_assignment;
mod self_named_constructor;
mod semicolon_if_nothing_returned;
mod serde_api;
mod shadow;
@ -900,6 +901,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
returns::LET_AND_RETURN,
returns::NEEDLESS_RETURN,
self_assignment::SELF_ASSIGNMENT,
self_named_constructor::SELF_NAMED_CONSTRUCTOR,
semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED,
serde_api::SERDE_API_MISUSE,
shadow::SHADOW_REUSE,
@ -1406,6 +1408,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(returns::LET_AND_RETURN),
LintId::of(returns::NEEDLESS_RETURN),
LintId::of(self_assignment::SELF_ASSIGNMENT),
LintId::of(self_named_constructor::SELF_NAMED_CONSTRUCTOR),
LintId::of(serde_api::SERDE_API_MISUSE),
LintId::of(single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
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:
LintId::of(redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES),
LintId::of(returns::LET_AND_RETURN),
LintId::of(returns::NEEDLESS_RETURN),
LintId::of(self_named_constructor::SELF_NAMED_CONSTRUCTOR),
LintId::of(single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
LintId::of(tabs_in_doc_comments::TABS_IN_DOC_COMMENTS),
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:
let scripts = conf.allowed_scripts.clone();
store.register_early_pass(move || box disallowed_script_idents::DisallowedScriptIdents::new(&scripts));
store.register_late_pass(|| box strlen_on_c_strings::StrlenOnCStrings);
store.register_late_pass(move || box self_named_constructor::SelfNamedConstructor);
}
#[rustfmt::skip]

View file

@ -0,0 +1,91 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::return_ty;
use clippy_utils::ty::{contains_adt_constructor, contains_ty};
use rustc_hir::{Impl, ImplItem, ImplItemKind, ItemKind, Node};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
declare_clippy_lint! {
/// **What it does:** Warns when constructors have the same name as their types.
///
/// **Why is this bad?** Repeating the name of the type is redundant.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust,ignore
/// struct Foo {}
///
/// impl Foo {
/// pub fn foo() -> Foo {
/// Foo {}
/// }
/// }
/// ```
/// Use instead:
/// ```rust,ignore
/// struct Foo {}
///
/// impl Foo {
/// pub fn new() -> Foo {
/// Foo {}
/// }
/// }
/// ```
pub SELF_NAMED_CONSTRUCTOR,
style,
"method should not have the same name as the type it is implemented for"
}
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(ref sig, _) => {
if sig.decl.implicit_self.has_implicit_self() {
return;
}
},
_ => 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());
// Do not check trait impls
if matches!(item.kind, ItemKind::Impl(Impl { of_trait: Some(_), .. })) {
return;
}
// Ensure method is constructor-like
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! {
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 || impl_item.ident.name.as_str().replace("_", "") == type_name;
then {
span_lint(
cx,
SELF_NAMED_CONSTRUCTOR,
impl_item.span,
&format!("constructor `{}` has the same name as the type", impl_item.ident.name),
);
}
}
}
}

View file

@ -7,7 +7,7 @@
struct Foo {}
impl Foo {
fn foo() -> Self {
fn new() -> Self {
impl Foo {
fn bar() {}
}

View file

@ -25,7 +25,9 @@ async fn all_to_one<'a>(a: &'a str, _b: &'a str) -> &'a str {
struct Foo;
impl Foo {
// ok
pub async fn foo(&mut self) {}
pub async fn new(&mut self) -> Self {
Foo {}
}
}
// rust-lang/rust#61115

View file

@ -59,7 +59,9 @@ pub trait E: Sized {
}
impl Foo {
pub fn foo() {}
pub fn new() -> Self {
Foo { a: 0, b: 0 }
}
fn bar() {}
}

View file

@ -78,23 +78,25 @@ LL | type AssociatedTypeDef = Self;
error: missing documentation for an associated function
--> $DIR/missing-doc-impl.rs:62:5
|
LL | pub fn foo() {}
| ^^^^^^^^^^^^^^^
LL | / pub fn new() -> Self {
LL | | Foo { a: 0, b: 0 }
LL | | }
| |_____^
error: missing documentation for an associated function
--> $DIR/missing-doc-impl.rs:63:5
--> $DIR/missing-doc-impl.rs:65:5
|
LL | fn bar() {}
| ^^^^^^^^^^^
error: missing documentation for an associated function
--> $DIR/missing-doc-impl.rs:67:5
--> $DIR/missing-doc-impl.rs:69:5
|
LL | pub fn foo() {}
| ^^^^^^^^^^^^^^^
error: missing documentation for an associated function
--> $DIR/missing-doc-impl.rs:71:5
--> $DIR/missing-doc-impl.rs:73:5
|
LL | / fn foo2() -> u32 {
LL | | 1

View file

@ -84,7 +84,7 @@ mod with_drop {
impl A {
// This can not be const because the type implements `Drop`.
pub fn a(self) -> B {
pub fn b(self) -> B {
B
}
}

View file

@ -6,7 +6,8 @@
dead_code,
clippy::no_effect,
clippy::if_same_then_else,
clippy::needless_return
clippy::needless_return,
clippy::self_named_constructor
)]
use std::cell::Cell;

View file

@ -6,7 +6,8 @@
dead_code,
clippy::no_effect,
clippy::if_same_then_else,
clippy::needless_return
clippy::needless_return,
clippy::self_named_constructor
)]
use std::cell::Cell;

View file

@ -1,5 +1,5 @@
error: this if-then-else expression returns a bool literal
--> $DIR/fixable.rs:39:5
--> $DIR/fixable.rs:40:5
|
LL | / if x {
LL | | true
@ -11,7 +11,7 @@ LL | | };
= note: `-D clippy::needless-bool` implied by `-D warnings`
error: this if-then-else expression returns a bool literal
--> $DIR/fixable.rs:44:5
--> $DIR/fixable.rs:45:5
|
LL | / if x {
LL | | false
@ -21,7 +21,7 @@ LL | | };
| |_____^ help: you can reduce it to: `!x`
error: this if-then-else expression returns a bool literal
--> $DIR/fixable.rs:49:5
--> $DIR/fixable.rs:50:5
|
LL | / if x && y {
LL | | false
@ -31,7 +31,7 @@ LL | | };
| |_____^ help: you can reduce it to: `!(x && y)`
error: this if-then-else expression returns a bool literal
--> $DIR/fixable.rs:69:5
--> $DIR/fixable.rs:70:5
|
LL | / if x {
LL | | return true;
@ -41,7 +41,7 @@ LL | | };
| |_____^ help: you can reduce it to: `return x`
error: this if-then-else expression returns a bool literal
--> $DIR/fixable.rs:77:5
--> $DIR/fixable.rs:78:5
|
LL | / if x {
LL | | return false;
@ -51,7 +51,7 @@ LL | | };
| |_____^ help: you can reduce it to: `return !x`
error: this if-then-else expression returns a bool literal
--> $DIR/fixable.rs:85:5
--> $DIR/fixable.rs:86:5
|
LL | / if x && y {
LL | | return true;
@ -61,7 +61,7 @@ LL | | };
| |_____^ help: you can reduce it to: `return x && y`
error: this if-then-else expression returns a bool literal
--> $DIR/fixable.rs:93:5
--> $DIR/fixable.rs:94:5
|
LL | / if x && y {
LL | | return false;
@ -71,7 +71,7 @@ LL | | };
| |_____^ help: you can reduce it to: `return !(x && y)`
error: equality checks against true are unnecessary
--> $DIR/fixable.rs:101:8
--> $DIR/fixable.rs:102:8
|
LL | if x == true {};
| ^^^^^^^^^ help: try simplifying it as shown: `x`
@ -79,25 +79,25 @@ LL | if x == true {};
= note: `-D clippy::bool-comparison` implied by `-D warnings`
error: equality checks against false can be replaced by a negation
--> $DIR/fixable.rs:105:8
--> $DIR/fixable.rs:106:8
|
LL | if x == false {};
| ^^^^^^^^^^ help: try simplifying it as shown: `!x`
error: equality checks against true are unnecessary
--> $DIR/fixable.rs:115:8
--> $DIR/fixable.rs:116:8
|
LL | if x == true {};
| ^^^^^^^^^ help: try simplifying it as shown: `x`
error: equality checks against false can be replaced by a negation
--> $DIR/fixable.rs:116:8
--> $DIR/fixable.rs:117:8
|
LL | if x == false {};
| ^^^^^^^^^^ help: try simplifying it as shown: `!x`
error: this if-then-else expression returns a bool literal
--> $DIR/fixable.rs:125:12
--> $DIR/fixable.rs:126:12
|
LL | } else if returns_bool() {
| ____________^

View file

@ -0,0 +1,59 @@
#![warn(clippy::self_named_constructor)]
struct ShouldSpawn;
struct ShouldNotSpawn;
impl ShouldSpawn {
pub fn should_spawn() -> ShouldSpawn {
ShouldSpawn
}
fn should_not_spawn() -> ShouldNotSpawn {
ShouldNotSpawn
}
}
impl ShouldNotSpawn {
pub fn new() -> ShouldNotSpawn {
ShouldNotSpawn
}
}
struct ShouldNotSpawnWithTrait;
trait ShouldNotSpawnTrait {
type Item;
}
impl ShouldNotSpawnTrait for ShouldNotSpawnWithTrait {
type Item = Self;
}
impl ShouldNotSpawnWithTrait {
pub fn should_not_spawn_with_trait() -> impl ShouldNotSpawnTrait<Item = Self> {
ShouldNotSpawnWithTrait
}
}
// Same trait name and same type name should not spawn the lint
#[derive(Default)]
pub struct Default;
trait TraitSameTypeName {
fn should_not_spawn() -> Self;
}
impl TraitSameTypeName for ShouldNotSpawn {
fn should_not_spawn() -> Self {
ShouldNotSpawn
}
}
struct SelfMethodShouldNotSpawn;
impl SelfMethodShouldNotSpawn {
fn self_method_should_not_spawn(self) -> Self {
SelfMethodShouldNotSpawn
}
}
fn main() {}

View file

@ -0,0 +1,12 @@
error: constructor `should_spawn` has the same name as the type
--> $DIR/self_named_constructor.rs:7:5
|
LL | / pub fn should_spawn() -> ShouldSpawn {
LL | | ShouldSpawn
LL | | }
| |_____^
|
= note: `-D clippy::self-named-constructor` implied by `-D warnings`
error: aborting due to previous error

View file

@ -6,7 +6,8 @@
clippy::unused_unit,
clippy::unnecessary_wraps,
clippy::or_fun_call,
clippy::needless_question_mark
clippy::needless_question_mark,
clippy::self_named_constructor
)]
use std::fmt::Debug;

View file

@ -1,5 +1,5 @@
error: passing a unit value to a function
--> $DIR/unit_arg.rs:55:5
--> $DIR/unit_arg.rs:56:5
|
LL | / foo({
LL | | 1;
@ -20,7 +20,7 @@ LL | foo(());
|
error: passing a unit value to a function
--> $DIR/unit_arg.rs:58:5
--> $DIR/unit_arg.rs:59:5
|
LL | foo(foo(1));
| ^^^^^^^^^^^
@ -32,7 +32,7 @@ LL | foo(());
|
error: passing a unit value to a function
--> $DIR/unit_arg.rs:59:5
--> $DIR/unit_arg.rs:60:5
|
LL | / foo({
LL | | foo(1);
@ -54,7 +54,7 @@ LL | foo(());
|
error: passing a unit value to a function
--> $DIR/unit_arg.rs:64:5
--> $DIR/unit_arg.rs:65:5
|
LL | / b.bar({
LL | | 1;
@ -74,7 +74,7 @@ LL | b.bar(());
|
error: passing unit values to a function
--> $DIR/unit_arg.rs:67:5
--> $DIR/unit_arg.rs:68:5
|
LL | taking_multiple_units(foo(0), foo(1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -87,7 +87,7 @@ LL | taking_multiple_units((), ());
|
error: passing unit values to a function
--> $DIR/unit_arg.rs:68:5
--> $DIR/unit_arg.rs:69:5
|
LL | / taking_multiple_units(foo(0), {
LL | | foo(1);
@ -110,7 +110,7 @@ LL | taking_multiple_units((), ());
|
error: passing unit values to a function
--> $DIR/unit_arg.rs:72:5
--> $DIR/unit_arg.rs:73:5
|
LL | / taking_multiple_units(
LL | | {
@ -140,7 +140,7 @@ LL | foo(2);
...
error: passing a unit value to a function
--> $DIR/unit_arg.rs:83:13
--> $DIR/unit_arg.rs:84:13
|
LL | None.or(Some(foo(2)));
| ^^^^^^^^^^^^
@ -154,7 +154,7 @@ LL | });
|
error: passing a unit value to a function
--> $DIR/unit_arg.rs:86:5
--> $DIR/unit_arg.rs:87:5
|
LL | foo(foo(()));
| ^^^^^^^^^^^^
@ -166,7 +166,7 @@ LL | foo(());
|
error: passing a unit value to a function
--> $DIR/unit_arg.rs:123:5
--> $DIR/unit_arg.rs:124:5
|
LL | Some(foo(1))
| ^^^^^^^^^^^^

View file

@ -4,7 +4,12 @@
#![warn(clippy::use_self)]
#![allow(dead_code)]
#![allow(clippy::should_implement_trait, clippy::upper_case_acronyms, clippy::from_over_into)]
#![allow(
clippy::should_implement_trait,
clippy::upper_case_acronyms,
clippy::from_over_into,
clippy::self_named_constructor
)]
#[macro_use]
extern crate proc_macro_derive;

View file

@ -4,7 +4,12 @@
#![warn(clippy::use_self)]
#![allow(dead_code)]
#![allow(clippy::should_implement_trait, clippy::upper_case_acronyms, clippy::from_over_into)]
#![allow(
clippy::should_implement_trait,
clippy::upper_case_acronyms,
clippy::from_over_into,
clippy::self_named_constructor
)]
#[macro_use]
extern crate proc_macro_derive;

View file

@ -1,5 +1,5 @@
error: unnecessary structure name repetition
--> $DIR/use_self.rs:18:21
--> $DIR/use_self.rs:23:21
|
LL | fn new() -> Foo {
| ^^^ help: use the applicable keyword: `Self`
@ -7,163 +7,163 @@ LL | fn new() -> Foo {
= note: `-D clippy::use-self` implied by `-D warnings`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:19:13
--> $DIR/use_self.rs:24:13
|
LL | Foo {}
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:21:22
--> $DIR/use_self.rs:26:22
|
LL | fn test() -> Foo {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:22:13
--> $DIR/use_self.rs:27:13
|
LL | Foo::new()
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:27:25
--> $DIR/use_self.rs:32:25
|
LL | fn default() -> Foo {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:28:13
--> $DIR/use_self.rs:33:13
|
LL | Foo::new()
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:93:24
--> $DIR/use_self.rs:98:24
|
LL | fn bad(foos: &[Foo]) -> impl Iterator<Item = &Foo> {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:93:55
--> $DIR/use_self.rs:98:55
|
LL | fn bad(foos: &[Foo]) -> impl Iterator<Item = &Foo> {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:108:13
--> $DIR/use_self.rs:113:13
|
LL | TS(0)
| ^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:143:29
--> $DIR/use_self.rs:148:29
|
LL | fn bar() -> Bar {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:144:21
--> $DIR/use_self.rs:149:21
|
LL | Bar { foo: Foo {} }
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:155:21
--> $DIR/use_self.rs:160:21
|
LL | fn baz() -> Foo {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:156:13
--> $DIR/use_self.rs:161:13
|
LL | Foo {}
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:173:21
--> $DIR/use_self.rs:178:21
|
LL | let _ = Enum::B(42);
| ^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:174:21
--> $DIR/use_self.rs:179:21
|
LL | let _ = Enum::C { field: true };
| ^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:175:21
--> $DIR/use_self.rs:180:21
|
LL | let _ = Enum::A;
| ^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:217:13
--> $DIR/use_self.rs:222:13
|
LL | nested::A::fun_1();
| ^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:218:13
--> $DIR/use_self.rs:223:13
|
LL | nested::A::A;
| ^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:220:13
--> $DIR/use_self.rs:225:13
|
LL | nested::A {};
| ^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:239:13
--> $DIR/use_self.rs:244:13
|
LL | TestStruct::from_something()
| ^^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:253:25
--> $DIR/use_self.rs:258:25
|
LL | async fn g() -> S {
| ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:254:13
--> $DIR/use_self.rs:259:13
|
LL | S {}
| ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:258:16
--> $DIR/use_self.rs:263:16
|
LL | &p[S::A..S::B]
| ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:258:22
--> $DIR/use_self.rs:263:22
|
LL | &p[S::A..S::B]
| ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:281:29
--> $DIR/use_self.rs:286:29
|
LL | fn foo(value: T) -> Foo<T> {
| ^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:282:13
--> $DIR/use_self.rs:287:13
|
LL | Foo::<T> { value }
| ^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:454:13
--> $DIR/use_self.rs:459:13
|
LL | A::new::<submod::B>(submod::B {})
| ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:491:13
--> $DIR/use_self.rs:496:13
|
LL | S2::new()
| ^^ help: use the applicable keyword: `Self`