adopt comments from review

9 files changed, 231 insertions, 128 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 179ecee03da..3f470f601ef 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1616,13 +1616,13 @@ 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
 [`needless_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_collect
 [`needless_continue`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_continue
 [`needless_doctest_main`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_doctest_main
-[`needless_fn_self_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_fn_self_type
 [`needless_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
 [`needless_pass_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value
 [`needless_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 80c85e70e89..3c39de1abf1 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -250,11 +250,11 @@ mod mut_mut;
 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;
 mod needless_continue;
-mod needless_fn_self_type;
 mod needless_pass_by_value;
 mod needless_update;
 mod neg_cmp_op_on_partial_ord;
@@ -718,12 +718,12 @@ 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,
         &needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE,
         &needless_continue::NEEDLESS_CONTINUE,
-        &needless_fn_self_type::NEEDLESS_FN_SELF_TYPE,
         &needless_pass_by_value::NEEDLESS_PASS_BY_VALUE,
         &needless_update::NEEDLESS_UPDATE,
         &neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD,
@@ -1029,7 +1029,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_fn_self_type::NeedlessFnSelfType);
+    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);
@@ -1374,10 +1374,10 @@ 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),
-        LintId::of(&needless_fn_self_type::NEEDLESS_FN_SELF_TYPE),
         LintId::of(&needless_update::NEEDLESS_UPDATE),
         LintId::of(&neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD),
         LintId::of(&neg_multiply::NEG_MULTIPLY),
@@ -1538,7 +1538,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&misc_early::MIXED_CASE_HEX_LITERALS),
         LintId::of(&misc_early::REDUNDANT_PATTERN),
         LintId::of(&mut_reference::UNNECESSARY_MUT_PASSED),
-        LintId::of(&needless_fn_self_type::NEEDLESS_FN_SELF_TYPE),
         LintId::of(&neg_multiply::NEG_MULTIPLY),
         LintId::of(&new_without_default::NEW_WITHOUT_DEFAULT),
         LintId::of(&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
@@ -1607,6 +1606,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),
diff --git a/clippy_lints/src/needless_arbitrary_self_type.rs b/clippy_lints/src/needless_arbitrary_self_type.rs
new file mode 100644
index 00000000000..1b23ecd9ad2
--- /dev/null
+++ b/clippy_lints/src/needless_arbitrary_self_type.rs
@@ -0,0 +1,114 @@
+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 is 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 let TyKind::Path(None, path) = &mut_ty.ty.kind {
+                    check_param_inner(cx, path, p.span.to(p.ty.span), &Mode::Ref(*lifetime), mut_ty.mutbl)
+                }
+            },
+            _ => {},
+        }
+    }
+}
diff --git a/clippy_lints/src/needless_fn_self_type.rs b/clippy_lints/src/needless_fn_self_type.rs
deleted file mode 100644
index b71f2fbbd46..00000000000
--- a/clippy_lints/src/needless_fn_self_type.rs
+++ /dev/null
@@ -1,78 +0,0 @@
-use crate::utils::span_lint_and_help;
-use if_chain::if_chain;
-use rustc_ast::ast::{Param, TyKind};
-use rustc_lint::{EarlyContext, EarlyLintPass};
-use rustc_session::{declare_lint_pass, declare_tool_lint};
-
-declare_clippy_lint! {
-    /// **What it does:** The lint checks for `self` fn fn parameters that explicitly
-    /// 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_FN_SELF_TYPE,
-    style,
-    "type of `self` parameter is already by default `Self`"
-}
-
-declare_lint_pass!(NeedlessFnSelfType => [NEEDLESS_FN_SELF_TYPE]);
-
-impl EarlyLintPass for NeedlessFnSelfType {
-    fn check_param(&mut self, cx: &EarlyContext<'_>, p: &Param) {
-        if_chain! {
-            if p.is_self();
-            if let TyKind::Path(None, path) = &p.ty.kind;
-            if let Some(segment) = path.segments.first();
-            if segment.ident.as_str() == sym!(Self).as_str();
-            then {
-                span_lint_and_help(
-                    cx,
-                    NEEDLESS_FN_SELF_TYPE,
-                    p.ty.span,
-                    "the type of the `self` parameter is already by default `Self`",
-                    None,
-                    "consider removing the type specification",
-                );
-            }
-        }
-    }
-}
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index a20e410f79b..91761e8a86d 100644
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -1460,6 +1460,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         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",
         desc: "if-statements with plain booleans in the then- and else-clause, e.g., `if p { true } else { false }`",
@@ -1502,13 +1509,6 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         module: "doc",
     },
     Lint {
-        name: "needless_fn_self_type",
-        group: "style",
-        desc: "type of `self` parameter is already by default `Self`",
-        deprecation: None,
-        module: "needless_fn_self_type",
-    },
-    Lint {
         name: "needless_lifetimes",
         group: "complexity",
         desc: "using explicit lifetimes for references in function arguments when elision rules would allow omitting them",
diff --git a/tests/ui/needless_arbitrary_self_type.rs b/tests/ui/needless_arbitrary_self_type.rs
new file mode 100644
index 00000000000..da4bbcf4a7d
--- /dev/null
+++ b/tests/ui/needless_arbitrary_self_type.rs
@@ -0,0 +1,58 @@
+#![warn(clippy::needless_arbitrary_self_type)]
+
+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_bad_with_lifetime<'a>(self: &'a Self) {
+        unimplemented!();
+    }
+
+    pub fn ref_good(&self) {
+        unimplemented!();
+    }
+
+    pub fn mut_ref_bad(self: &mut Self) {
+        unimplemented!();
+    }
+
+    pub fn mut_ref_bad_with_lifetime<'a>(self: &'a mut Self) {
+        unimplemented!();
+    }
+
+    pub fn mut_ref_good(&mut self) {
+        unimplemented!();
+    }
+
+    pub fn mut_ref_mut_bad(mut self: &mut Self) {
+        unimplemented!();
+    }
+
+    pub fn mut_ref_mut_ref_good(self: &&mut &mut Self) {
+        unimplemented!();
+    }
+}
+
+fn main() {}
diff --git a/tests/ui/needless_arbitrary_self_type.stderr b/tests/ui/needless_arbitrary_self_type.stderr
new file mode 100644
index 00000000000..ee803b24071
--- /dev/null
+++ b/tests/ui/needless_arbitrary_self_type.stderr
@@ -0,0 +1,46 @@
+error: the type of the `self` parameter is arbitrary
+  --> $DIR/needless_arbitrary_self_type.rs:9: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 is arbitrary
+  --> $DIR/needless_arbitrary_self_type.rs:17: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 is arbitrary
+  --> $DIR/needless_arbitrary_self_type.rs:25:20
+   |
+LL |     pub fn ref_bad(self: &Self) {
+   |                    ^^^^^^^^^^^ help: consider to change this parameter to: `&self`
+
+error: the type of the `self` parameter is arbitrary
+  --> $DIR/needless_arbitrary_self_type.rs:29: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 is arbitrary
+  --> $DIR/needless_arbitrary_self_type.rs:37: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 is arbitrary
+  --> $DIR/needless_arbitrary_self_type.rs:41: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: the type of the `self` parameter is arbitrary
+  --> $DIR/needless_arbitrary_self_type.rs:49:28
+   |
+LL |     pub fn mut_ref_mut_bad(mut self: &mut Self) {
+   |                            ^^^^^^^^^^^^^^^^^^^ help: consider to change this parameter to: `&mut self`
+
+error: aborting due to 7 previous errors
+
diff --git a/tests/ui/needless_fn_self_type.rs b/tests/ui/needless_fn_self_type.rs
deleted file mode 100644
index 12bb84582ff..00000000000
--- a/tests/ui/needless_fn_self_type.rs
+++ /dev/null
@@ -1,26 +0,0 @@
-#![warn(clippy::style, clippy::needless_fn_self_type)]
-
-pub enum ValType {
-    I32,
-    I64,
-    F32,
-    F64,
-}
-
-impl ValType {
-    pub fn bytes_bad(self: Self) -> usize {
-        match self {
-            Self::I32 | Self::F32 => 4,
-            Self::I64 | Self::F64 => 8,
-        }
-    }
-
-    pub fn bytes_good(self) -> usize {
-        match self {
-            Self::I32 | Self::F32 => 4,
-            Self::I64 | Self::F64 => 8,
-        }
-    }
-}
-
-fn main() {}
diff --git a/tests/ui/needless_fn_self_type.stderr b/tests/ui/needless_fn_self_type.stderr
deleted file mode 100644
index 703705c7842..00000000000
--- a/tests/ui/needless_fn_self_type.stderr
+++ /dev/null
@@ -1,11 +0,0 @@
-error: the type of the `self` parameter is already by default `Self`
-  --> $DIR/needless_fn_self_type.rs:11:28
-   |
-LL |     pub fn bytes_bad(self: Self) -> usize {
-   |                            ^^^^
-   |
-   = note: `-D clippy::needless-fn-self-type` implied by `-D warnings`
-   = help: consider removing the type specification
-
-error: aborting due to previous error
-