Rollup merge of #5869 - wiomoc:feature/implicit-self, r=ebroto,flip1995

New lint against `Self` as an arbitrary self type

Fixes #5861

changelog: * [`needless_arbitrary_self_type`] [#5869](https://github.com/rust-lang/rust-clippy/pull/5869)
This commit is contained in:
Philipp Krones 2020-08-10 14:56:26 +02:00 committed by GitHub
commit 8ee57eed79
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 373 additions and 58 deletions

View File

@ -1616,6 +1616,7 @@ Released 2018-09-13
[`mutex_atomic`]: https://rust-lang.github.io/rust-clippy/master/index.html#mutex_atomic
[`mutex_integer`]: https://rust-lang.github.io/rust-clippy/master/index.html#mutex_integer
[`naive_bytecount`]: https://rust-lang.github.io/rust-clippy/master/index.html#naive_bytecount
[`needless_arbitrary_self_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_arbitrary_self_type
[`needless_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_bool
[`needless_borrow`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
[`needless_borrowed_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrowed_reference

View File

@ -250,6 +250,7 @@ macro_rules! declare_clippy_lint {
mod mut_reference;
mod mutable_debug_assertion;
mod mutex_atomic;
mod needless_arbitrary_self_type;
mod needless_bool;
mod needless_borrow;
mod needless_borrowed_ref;
@ -718,6 +719,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL,
&mutex_atomic::MUTEX_ATOMIC,
&mutex_atomic::MUTEX_INTEGER,
&needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE,
&needless_bool::BOOL_COMPARISON,
&needless_bool::NEEDLESS_BOOL,
&needless_borrow::NEEDLESS_BORROW,
@ -1028,6 +1030,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_early_pass(|| box items_after_statements::ItemsAfterStatements);
store.register_early_pass(|| box precedence::Precedence);
store.register_early_pass(|| box needless_continue::NeedlessContinue);
store.register_early_pass(|| box needless_arbitrary_self_type::NeedlessArbitrarySelfType);
store.register_early_pass(|| box redundant_static_lifetimes::RedundantStaticLifetimes);
store.register_late_pass(|| box cargo_common_metadata::CargoCommonMetadata);
store.register_late_pass(|| box multiple_crate_versions::MultipleCrateVersions);
@ -1373,6 +1376,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&mut_key::MUTABLE_KEY_TYPE),
LintId::of(&mut_reference::UNNECESSARY_MUT_PASSED),
LintId::of(&mutex_atomic::MUTEX_ATOMIC),
LintId::of(&needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE),
LintId::of(&needless_bool::BOOL_COMPARISON),
LintId::of(&needless_bool::NEEDLESS_BOOL),
LintId::of(&needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),
@ -1605,6 +1609,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&misc::SHORT_CIRCUIT_STATEMENT),
LintId::of(&misc_early::UNNEEDED_WILDCARD_PATTERN),
LintId::of(&misc_early::ZERO_PREFIXED_LITERAL),
LintId::of(&needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE),
LintId::of(&needless_bool::BOOL_COMPARISON),
LintId::of(&needless_bool::NEEDLESS_BOOL),
LintId::of(&needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),

View File

@ -0,0 +1,118 @@
use crate::utils::span_lint_and_sugg;
use if_chain::if_chain;
use rustc_ast::ast::{BindingMode, Lifetime, Mutability, Param, PatKind, Path, TyKind};
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::kw;
use rustc_span::Span;
declare_clippy_lint! {
/// **What it does:** The lint checks for `self` in fn parameters that
/// specify the `Self`-type explicitly
/// **Why is this bad?** Increases the amount and decreases the readability of code
///
/// **Known problems:** None
///
/// **Example:**
/// ```rust
/// enum ValType {
/// I32,
/// I64,
/// F32,
/// F64,
/// }
///
/// impl ValType {
/// pub fn bytes(self: Self) -> usize {
/// match self {
/// Self::I32 | Self::F32 => 4,
/// Self::I64 | Self::F64 => 8,
/// }
/// }
/// }
/// ```
///
/// Could be rewritten as
///
/// ```rust
/// enum ValType {
/// I32,
/// I64,
/// F32,
/// F64,
/// }
///
/// impl ValType {
/// pub fn bytes(self) -> usize {
/// match self {
/// Self::I32 | Self::F32 => 4,
/// Self::I64 | Self::F64 => 8,
/// }
/// }
/// }
/// ```
pub NEEDLESS_ARBITRARY_SELF_TYPE,
complexity,
"type of `self` parameter is already by default `Self`"
}
declare_lint_pass!(NeedlessArbitrarySelfType => [NEEDLESS_ARBITRARY_SELF_TYPE]);
enum Mode {
Ref(Option<Lifetime>),
Value,
}
fn check_param_inner(cx: &EarlyContext<'_>, path: &Path, span: Span, binding_mode: &Mode, mutbl: Mutability) {
if_chain! {
if let [segment] = &path.segments[..];
if segment.ident.name == kw::SelfUpper;
then {
let self_param = match (binding_mode, mutbl) {
(Mode::Ref(None), Mutability::Mut) => "&mut self".to_string(),
(Mode::Ref(Some(lifetime)), Mutability::Mut) => format!("&{} mut self", &lifetime.ident.name),
(Mode::Ref(None), Mutability::Not) => "&self".to_string(),
(Mode::Ref(Some(lifetime)), Mutability::Not) => format!("&{} self", &lifetime.ident.name),
(Mode::Value, Mutability::Mut) => "mut self".to_string(),
(Mode::Value, Mutability::Not) => "self".to_string(),
};
span_lint_and_sugg(
cx,
NEEDLESS_ARBITRARY_SELF_TYPE,
span,
"the type of the `self` parameter does not need to be arbitrary",
"consider to change this parameter to",
self_param,
Applicability::MachineApplicable,
)
}
}
}
impl EarlyLintPass for NeedlessArbitrarySelfType {
fn check_param(&mut self, cx: &EarlyContext<'_>, p: &Param) {
if !p.is_self() {
return;
}
match &p.ty.kind {
TyKind::Path(None, path) => {
if let PatKind::Ident(BindingMode::ByValue(mutbl), _, _) = p.pat.kind {
check_param_inner(cx, path, p.span.to(p.ty.span), &Mode::Value, mutbl)
}
},
TyKind::Rptr(lifetime, mut_ty) => {
if_chain! {
if let TyKind::Path(None, path) = &mut_ty.ty.kind;
if let PatKind::Ident(BindingMode::ByValue(Mutability::Not), _, _) = p.pat.kind;
then {
check_param_inner(cx, path, p.span.to(p.ty.span), &Mode::Ref(*lifetime), mut_ty.mutbl)
}
}
},
_ => {},
}
}
}

View File

@ -50,7 +50,7 @@
/// fn func<T: Clone + Default>(arg: T) {}
/// ```
/// or
/// ///
///
/// ```rust
/// fn func<T>(arg: T) where T: Clone + Default {}
/// ```

View File

@ -1459,6 +1459,13 @@
deprecation: None,
module: "bytecount",
},
Lint {
name: "needless_arbitrary_self_type",
group: "complexity",
desc: "type of `self` parameter is already by default `Self`",
deprecation: None,
module: "needless_arbitrary_self_type",
},
Lint {
name: "needless_bool",
group: "complexity",

View File

@ -1,4 +1,10 @@
#![allow(unused, dead_code, clippy::needless_lifetimes, clippy::needless_pass_by_value)]
#![allow(
unused,
dead_code,
clippy::needless_lifetimes,
clippy::needless_pass_by_value,
clippy::needless_arbitrary_self_type
)]
#![warn(clippy::extra_unused_lifetimes)]
fn empty() {}

View File

@ -1,5 +1,5 @@
error: this lifetime isn't used in the function definition
--> $DIR/extra_unused_lifetimes.rs:8:14
--> $DIR/extra_unused_lifetimes.rs:14:14
|
LL | fn unused_lt<'a>(x: u8) {}
| ^^
@ -7,19 +7,19 @@ LL | fn unused_lt<'a>(x: u8) {}
= note: `-D clippy::extra-unused-lifetimes` implied by `-D warnings`
error: this lifetime isn't used in the function definition
--> $DIR/extra_unused_lifetimes.rs:10:25
--> $DIR/extra_unused_lifetimes.rs:16:25
|
LL | fn unused_lt_transitive<'a, 'b: 'a>(x: &'b u8) {
| ^^
error: this lifetime isn't used in the function definition
--> $DIR/extra_unused_lifetimes.rs:35:10
--> $DIR/extra_unused_lifetimes.rs:41:10
|
LL | fn x<'a>(&self) {}
| ^^
error: this lifetime isn't used in the function definition
--> $DIR/extra_unused_lifetimes.rs:61:22
--> $DIR/extra_unused_lifetimes.rs:67:22
|
LL | fn unused_lt<'a>(x: u8) {}
| ^^

View File

@ -4,14 +4,14 @@
pub struct PubOne;
impl PubOne {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
}
impl PubOne {
// A second impl for this struct -- the error span shouldn't mention this.
pub fn irrelevant(self: &Self) -> bool {
pub fn irrelevant(&self) -> bool {
false
}
}
@ -21,7 +21,7 @@ pub fn irrelevant(self: &Self) -> bool {
#[allow(clippy::len_without_is_empty)]
impl PubAllowed {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
}
@ -29,17 +29,17 @@ pub fn len(self: &Self) -> isize {
// No `allow` attribute on this impl block, but that doesn't matter -- we only require one on the
// impl containing `len`.
impl PubAllowed {
pub fn irrelevant(self: &Self) -> bool {
pub fn irrelevant(&self) -> bool {
false
}
}
pub trait PubTraitsToo {
fn len(self: &Self) -> isize;
fn len(&self) -> isize;
}
impl PubTraitsToo for One {
fn len(self: &Self) -> isize {
fn len(&self) -> isize {
0
}
}
@ -47,11 +47,11 @@ fn len(self: &Self) -> isize {
pub struct HasIsEmpty;
impl HasIsEmpty {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
fn is_empty(self: &Self) -> bool {
fn is_empty(&self) -> bool {
false
}
}
@ -59,11 +59,11 @@ fn is_empty(self: &Self) -> bool {
pub struct HasWrongIsEmpty;
impl HasWrongIsEmpty {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
pub fn is_empty(self: &Self, x: u32) -> bool {
pub fn is_empty(&self, x: u32) -> bool {
false
}
}
@ -71,7 +71,7 @@ pub fn is_empty(self: &Self, x: u32) -> bool {
struct NotPubOne;
impl NotPubOne {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
// No error; `len` is pub but `NotPubOne` is not exported anyway.
1
}
@ -80,19 +80,19 @@ pub fn len(self: &Self) -> isize {
struct One;
impl One {
fn len(self: &Self) -> isize {
fn len(&self) -> isize {
// No error; `len` is private; see issue #1085.
1
}
}
trait TraitsToo {
fn len(self: &Self) -> isize;
fn len(&self) -> isize;
// No error; `len` is private; see issue #1085.
}
impl TraitsToo for One {
fn len(self: &Self) -> isize {
fn len(&self) -> isize {
0
}
}
@ -100,11 +100,11 @@ fn len(self: &Self) -> isize {
struct HasPrivateIsEmpty;
impl HasPrivateIsEmpty {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
fn is_empty(self: &Self) -> bool {
fn is_empty(&self) -> bool {
false
}
}
@ -112,16 +112,16 @@ fn is_empty(self: &Self) -> bool {
struct Wither;
pub trait WithIsEmpty {
fn len(self: &Self) -> isize;
fn is_empty(self: &Self) -> bool;
fn len(&self) -> isize;
fn is_empty(&self) -> bool;
}
impl WithIsEmpty for Wither {
fn len(self: &Self) -> isize {
fn len(&self) -> isize {
1
}
fn is_empty(self: &Self) -> bool {
fn is_empty(&self) -> bool {
false
}
}

View File

@ -2,7 +2,7 @@ error: item `PubOne` has a public `len` method but no corresponding `is_empty` m
--> $DIR/len_without_is_empty.rs:6:1
|
LL | / impl PubOne {
LL | | pub fn len(self: &Self) -> isize {
LL | | pub fn len(&self) -> isize {
LL | | 1
LL | | }
LL | | }
@ -14,7 +14,7 @@ error: trait `PubTraitsToo` has a `len` method but no (possibly inherited) `is_e
--> $DIR/len_without_is_empty.rs:37:1
|
LL | / pub trait PubTraitsToo {
LL | | fn len(self: &Self) -> isize;
LL | | fn len(&self) -> isize;
LL | | }
| |_^
@ -22,7 +22,7 @@ error: item `HasIsEmpty` has a public `len` method but a private `is_empty` meth
--> $DIR/len_without_is_empty.rs:49:1
|
LL | / impl HasIsEmpty {
LL | | pub fn len(self: &Self) -> isize {
LL | | pub fn len(&self) -> isize {
LL | | 1
LL | | }
... |
@ -34,7 +34,7 @@ error: item `HasWrongIsEmpty` has a public `len` method but no corresponding `is
--> $DIR/len_without_is_empty.rs:61:1
|
LL | / impl HasWrongIsEmpty {
LL | | pub fn len(self: &Self) -> isize {
LL | | pub fn len(&self) -> isize {
LL | | 1
LL | | }
... |

View File

@ -7,12 +7,12 @@ pub struct One;
struct Wither;
trait TraitsToo {
fn len(self: &Self) -> isize;
fn len(&self) -> isize;
// No error; `len` is private; see issue #1085.
}
impl TraitsToo for One {
fn len(self: &Self) -> isize {
fn len(&self) -> isize {
0
}
}
@ -20,11 +20,11 @@ impl TraitsToo for One {
pub struct HasIsEmpty;
impl HasIsEmpty {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
fn is_empty(self: &Self) -> bool {
fn is_empty(&self) -> bool {
false
}
}
@ -32,26 +32,26 @@ impl HasIsEmpty {
pub struct HasWrongIsEmpty;
impl HasWrongIsEmpty {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
pub fn is_empty(self: &Self, x: u32) -> bool {
pub fn is_empty(&self, x: u32) -> bool {
false
}
}
pub trait WithIsEmpty {
fn len(self: &Self) -> isize;
fn is_empty(self: &Self) -> bool;
fn len(&self) -> isize;
fn is_empty(&self) -> bool;
}
impl WithIsEmpty for Wither {
fn len(self: &Self) -> isize {
fn len(&self) -> isize {
1
}
fn is_empty(self: &Self) -> bool {
fn is_empty(&self) -> bool {
false
}
}

View File

@ -7,12 +7,12 @@
struct Wither;
trait TraitsToo {
fn len(self: &Self) -> isize;
fn len(&self) -> isize;
// No error; `len` is private; see issue #1085.
}
impl TraitsToo for One {
fn len(self: &Self) -> isize {
fn len(&self) -> isize {
0
}
}
@ -20,11 +20,11 @@ fn len(self: &Self) -> isize {
pub struct HasIsEmpty;
impl HasIsEmpty {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
fn is_empty(self: &Self) -> bool {
fn is_empty(&self) -> bool {
false
}
}
@ -32,26 +32,26 @@ fn is_empty(self: &Self) -> bool {
pub struct HasWrongIsEmpty;
impl HasWrongIsEmpty {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
pub fn is_empty(self: &Self, x: u32) -> bool {
pub fn is_empty(&self, x: u32) -> bool {
false
}
}
pub trait WithIsEmpty {
fn len(self: &Self) -> isize;
fn is_empty(self: &Self) -> bool;
fn len(&self) -> isize;
fn is_empty(&self) -> bool;
}
impl WithIsEmpty for Wither {
fn len(self: &Self) -> isize {
fn len(&self) -> isize {
1
}
fn is_empty(self: &Self) -> bool {
fn is_empty(&self) -> bool {
false
}
}

View File

@ -0,0 +1,69 @@
// run-rustfix
#![warn(clippy::needless_arbitrary_self_type)]
#![allow(unused_mut, clippy::needless_lifetimes)]
pub enum ValType {
A,
B,
}
impl ValType {
pub fn bad(self) {
unimplemented!();
}
pub fn good(self) {
unimplemented!();
}
pub fn mut_bad(mut self) {
unimplemented!();
}
pub fn mut_good(mut self) {
unimplemented!();
}
pub fn ref_bad(&self) {
unimplemented!();
}
pub fn ref_good(&self) {
unimplemented!();
}
pub fn ref_bad_with_lifetime<'a>(&'a self) {
unimplemented!();
}
pub fn ref_good_with_lifetime<'a>(&'a self) {
unimplemented!();
}
pub fn mut_ref_bad(&mut self) {
unimplemented!();
}
pub fn mut_ref_good(&mut self) {
unimplemented!();
}
pub fn mut_ref_bad_with_lifetime<'a>(&'a mut self) {
unimplemented!();
}
pub fn mut_ref_good_with_lifetime<'a>(&'a mut self) {
unimplemented!();
}
pub fn mut_ref_mut_good(mut self: &mut Self) {
unimplemented!();
}
pub fn mut_ref_mut_ref_good(self: &&mut &mut Self) {
unimplemented!();
}
}
fn main() {}

View File

@ -0,0 +1,69 @@
// run-rustfix
#![warn(clippy::needless_arbitrary_self_type)]
#![allow(unused_mut, clippy::needless_lifetimes)]
pub enum ValType {
A,
B,
}
impl ValType {
pub fn bad(self: Self) {
unimplemented!();
}
pub fn good(self) {
unimplemented!();
}
pub fn mut_bad(mut self: Self) {
unimplemented!();
}
pub fn mut_good(mut self) {
unimplemented!();
}
pub fn ref_bad(self: &Self) {
unimplemented!();
}
pub fn ref_good(&self) {
unimplemented!();
}
pub fn ref_bad_with_lifetime<'a>(self: &'a Self) {
unimplemented!();
}
pub fn ref_good_with_lifetime<'a>(&'a self) {
unimplemented!();
}
pub fn mut_ref_bad(self: &mut Self) {
unimplemented!();
}
pub fn mut_ref_good(&mut self) {
unimplemented!();
}
pub fn mut_ref_bad_with_lifetime<'a>(self: &'a mut Self) {
unimplemented!();
}
pub fn mut_ref_good_with_lifetime<'a>(&'a mut self) {
unimplemented!();
}
pub fn mut_ref_mut_good(mut self: &mut Self) {
unimplemented!();
}
pub fn mut_ref_mut_ref_good(self: &&mut &mut Self) {
unimplemented!();
}
}
fn main() {}

View File

@ -0,0 +1,40 @@
error: the type of the `self` parameter does not need to be arbitrary
--> $DIR/needless_arbitrary_self_type.rs:12:16
|
LL | pub fn bad(self: Self) {
| ^^^^^^^^^^ help: consider to change this parameter to: `self`
|
= note: `-D clippy::needless-arbitrary-self-type` implied by `-D warnings`
error: the type of the `self` parameter does not need to be arbitrary
--> $DIR/needless_arbitrary_self_type.rs:20:20
|
LL | pub fn mut_bad(mut self: Self) {
| ^^^^^^^^^^^^^^ help: consider to change this parameter to: `mut self`
error: the type of the `self` parameter does not need to be arbitrary
--> $DIR/needless_arbitrary_self_type.rs:28:20
|
LL | pub fn ref_bad(self: &Self) {
| ^^^^^^^^^^^ help: consider to change this parameter to: `&self`
error: the type of the `self` parameter does not need to be arbitrary
--> $DIR/needless_arbitrary_self_type.rs:36:38
|
LL | pub fn ref_bad_with_lifetime<'a>(self: &'a Self) {
| ^^^^^^^^^^^^^^ help: consider to change this parameter to: `&'a self`
error: the type of the `self` parameter does not need to be arbitrary
--> $DIR/needless_arbitrary_self_type.rs:44:24
|
LL | pub fn mut_ref_bad(self: &mut Self) {
| ^^^^^^^^^^^^^^^ help: consider to change this parameter to: `&mut self`
error: the type of the `self` parameter does not need to be arbitrary
--> $DIR/needless_arbitrary_self_type.rs:52:42
|
LL | pub fn mut_ref_bad_with_lifetime<'a>(self: &'a mut Self) {
| ^^^^^^^^^^^^^^^^^^ help: consider to change this parameter to: `&'a mut self`
error: aborting due to 6 previous errors

View File

@ -22,9 +22,9 @@ struct HasOption {
}
impl HasOption {
fn do_option_nothing(self: &Self, value: usize) {}
fn do_option_nothing(&self, value: usize) {}
fn do_option_plus_one(self: &Self, value: usize) -> usize {
fn do_option_plus_one(&self, value: usize) -> usize {
value + 1
}
}

View File

@ -22,9 +22,9 @@ struct HasOption {
}
impl HasOption {
fn do_option_nothing(self: &Self, value: usize) {}
fn do_option_nothing(&self, value: usize) {}
fn do_option_plus_one(self: &Self, value: usize) -> usize {
fn do_option_plus_one(&self, value: usize) -> usize {
value + 1
}
}

View File

@ -18,9 +18,9 @@ struct HasResult {
}
impl HasResult {
fn do_result_nothing(self: &Self, value: usize) {}
fn do_result_nothing(&self, value: usize) {}
fn do_result_plus_one(self: &Self, value: usize) -> usize {
fn do_result_plus_one(&self, value: usize) -> usize {
value + 1
}
}

View File

@ -18,9 +18,9 @@ struct HasResult {
}
impl HasResult {
fn do_result_nothing(self: &Self, value: usize) {}
fn do_result_nothing(&self, value: usize) {}
fn do_result_plus_one(self: &Self, value: usize) -> usize {
fn do_result_plus_one(&self, value: usize) -> usize {
value + 1
}
}