Auto merge of #9726 - kraktus:fix_use_self, r=Alexendoo

[`use_self`] fix suggestion when full path to struct was given

Previously the following wrong suggestion was given

```rust
impl Error for std::fmt::Error {
    fn custom<T: std::fmt::Display>(_msg: T) -> Self {
-        std::fmt::Error // Should lint
+        Self::Error // Should lint
    }
}
```

Also remove known problem line related to #4140 since it's been closed, and refactor the lint

changelog: [`use_self`] fix suggestion when full path to struct was given

9 files changed, 87 insertions, 33 deletions

diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs
index acf27c34e94..464ffdc0a2a 100644
--- a/clippy_lints/src/use_self.rs
+++ b/clippy_lints/src/use_self.rs
@@ -30,7 +30,6 @@ declare_clippy_lint! {
     ///
     /// ### Known problems
     /// - Unaddressed false negative in fn bodies of trait implementations
-    /// - False positive with associated types in traits (#4140)
     ///
     /// ### Example
     /// ```rust
@@ -235,24 +234,13 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
             then {} else { return; }
         }
         match expr.kind {
-            ExprKind::Struct(QPath::Resolved(_, path), ..) => match path.res {
-                Res::SelfTyParam { .. } | Res::SelfTyAlias { .. } => (),
-                Res::Def(DefKind::Variant, _) => lint_path_to_variant(cx, path),
-                _ => span_lint(cx, path.span),
-            },
-            // tuple struct instantiation (`Foo(arg)` or `Enum::Foo(arg)`)
+            ExprKind::Struct(QPath::Resolved(_, path), ..) => check_path(cx, path),
             ExprKind::Call(fun, _) => {
                 if let ExprKind::Path(QPath::Resolved(_, path)) = fun.kind {
-                    if let Res::Def(DefKind::Ctor(ctor_of, _), ..) = path.res {
-                        match ctor_of {
-                            CtorOf::Variant => lint_path_to_variant(cx, path),
-                            CtorOf::Struct => span_lint(cx, path.span),
-                        }
-                    }
+                    check_path(cx, path);
                 }
             },
-            // unit enum variants (`Enum::A`)
-            ExprKind::Path(QPath::Resolved(_, path)) => lint_path_to_variant(cx, path),
+            ExprKind::Path(QPath::Resolved(_, path)) => check_path(cx, path),
             _ => (),
         }
     }
@@ -268,15 +256,7 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
                  | PatKind::Struct(QPath::Resolved(_, path), _, _) = pat.kind;
             if cx.typeck_results().pat_ty(pat) == cx.tcx.type_of(impl_id);
             then {
-                match path.res {
-                    Res::Def(DefKind::Ctor(ctor_of, _), ..) => match ctor_of {
-                            CtorOf::Variant => lint_path_to_variant(cx, path),
-                            CtorOf::Struct => span_lint(cx, path.span),
-                    },
-                    Res::Def(DefKind::Variant, ..) => lint_path_to_variant(cx, path),
-                    Res::Def(DefKind::Struct, ..) => span_lint(cx, path.span),
-                    _ => ()
-                }
+                check_path(cx, path);
             }
         }
     }
@@ -314,6 +294,16 @@ fn span_lint(cx: &LateContext<'_>, span: Span) {
     );
 }
 
+fn check_path(cx: &LateContext<'_>, path: &Path<'_>) {
+    match path.res {
+        Res::Def(DefKind::Ctor(CtorOf::Variant, _) | DefKind::Variant, ..) => {
+            lint_path_to_variant(cx, path);
+        },
+        Res::Def(DefKind::Ctor(CtorOf::Struct, _) | DefKind::Struct, ..) => span_lint(cx, path.span),
+        _ => (),
+    }
+}
+
 fn lint_path_to_variant(cx: &LateContext<'_>, path: &Path<'_>) {
     if let [.., self_seg, _variant] = path.segments {
         let span = path
diff --git a/tests/ui-cargo/cargo_rust_version/fail_both_diff/src/main.stderr b/tests/ui-cargo/cargo_rust_version/fail_both_diff/src/main.stderr
index 9a7d802dc6d..163f8bb35e7 100644
--- a/tests/ui-cargo/cargo_rust_version/fail_both_diff/src/main.stderr
+++ b/tests/ui-cargo/cargo_rust_version/fail_both_diff/src/main.stderr
@@ -12,5 +12,11 @@ note: the lint level is defined here
 LL | #![deny(clippy::use_self)]
    |         ^^^^^^^^^^^^^^^^
 
-error: aborting due to previous error; 1 warning emitted
+error: unnecessary structure name repetition
+  --> $DIR/main.rs:7:9
+   |
+LL |         Foo
+   |         ^^^ help: use the applicable keyword: `Self`
+
+error: aborting due to 2 previous errors; 1 warning emitted
 
diff --git a/tests/ui-cargo/cargo_rust_version/fail_both_same/src/main.stderr b/tests/ui-cargo/cargo_rust_version/fail_both_same/src/main.stderr
index a280e1bacdf..259d39b1252 100644
--- a/tests/ui-cargo/cargo_rust_version/fail_both_same/src/main.stderr
+++ b/tests/ui-cargo/cargo_rust_version/fail_both_same/src/main.stderr
@@ -10,5 +10,11 @@ note: the lint level is defined here
 LL | #![deny(clippy::use_self)]
    |         ^^^^^^^^^^^^^^^^
 
-error: aborting due to previous error
+error: unnecessary structure name repetition
+  --> $DIR/main.rs:7:9
+   |
+LL |         Foo
+   |         ^^^ help: use the applicable keyword: `Self`
+
+error: aborting due to 2 previous errors
 
diff --git a/tests/ui-cargo/cargo_rust_version/fail_cargo/src/main.stderr b/tests/ui-cargo/cargo_rust_version/fail_cargo/src/main.stderr
index a280e1bacdf..259d39b1252 100644
--- a/tests/ui-cargo/cargo_rust_version/fail_cargo/src/main.stderr
+++ b/tests/ui-cargo/cargo_rust_version/fail_cargo/src/main.stderr
@@ -10,5 +10,11 @@ note: the lint level is defined here
 LL | #![deny(clippy::use_self)]
    |         ^^^^^^^^^^^^^^^^
 
-error: aborting due to previous error
+error: unnecessary structure name repetition
+  --> $DIR/main.rs:7:9
+   |
+LL |         Foo
+   |         ^^^ help: use the applicable keyword: `Self`
+
+error: aborting due to 2 previous errors
 
diff --git a/tests/ui-cargo/cargo_rust_version/fail_clippy/src/main.stderr b/tests/ui-cargo/cargo_rust_version/fail_clippy/src/main.stderr
index a280e1bacdf..259d39b1252 100644
--- a/tests/ui-cargo/cargo_rust_version/fail_clippy/src/main.stderr
+++ b/tests/ui-cargo/cargo_rust_version/fail_clippy/src/main.stderr
@@ -10,5 +10,11 @@ note: the lint level is defined here
 LL | #![deny(clippy::use_self)]
    |         ^^^^^^^^^^^^^^^^
 
-error: aborting due to previous error
+error: unnecessary structure name repetition
+  --> $DIR/main.rs:7:9
+   |
+LL |         Foo
+   |         ^^^ help: use the applicable keyword: `Self`
+
+error: aborting due to 2 previous errors
 
diff --git a/tests/ui-cargo/cargo_rust_version/fail_file_attr/src/main.stderr b/tests/ui-cargo/cargo_rust_version/fail_file_attr/src/main.stderr
index 88f6e00922b..97e6c3d5a55 100644
--- a/tests/ui-cargo/cargo_rust_version/fail_file_attr/src/main.stderr
+++ b/tests/ui-cargo/cargo_rust_version/fail_file_attr/src/main.stderr
@@ -10,5 +10,11 @@ note: the lint level is defined here
 LL | #![deny(clippy::use_self)]
    |         ^^^^^^^^^^^^^^^^
 
-error: aborting due to previous error
+error: unnecessary structure name repetition
+  --> $DIR/main.rs:12:9
+   |
+LL |         Foo
+   |         ^^^ help: use the applicable keyword: `Self`
+
+error: aborting due to 2 previous errors
 
diff --git a/tests/ui/use_self_trait.fixed b/tests/ui/use_self_trait.fixed
index 12adf1e41a6..4e779308d02 100644
--- a/tests/ui/use_self_trait.fixed
+++ b/tests/ui/use_self_trait.fixed
@@ -47,8 +47,7 @@ impl Mul for Bad {
 
 impl Clone for Bad {
     fn clone(&self) -> Self {
-        // FIXME: applicable here
-        Bad
+        Self
     }
 }
 
@@ -138,4 +137,16 @@ mod impl_in_macro {
     parse_ip_impl!(Foo); // Should not lint
 }
 
+mod full_path_replacement {
+    trait Error {
+        fn custom<T: std::fmt::Display>(_msg: T) -> Self;
+    }
+
+    impl Error for std::fmt::Error {
+        fn custom<T: std::fmt::Display>(_msg: T) -> Self {
+            Self // Should lint
+        }
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/use_self_trait.rs b/tests/ui/use_self_trait.rs
index 49dbcddc1d8..325dc73b21e 100644
--- a/tests/ui/use_self_trait.rs
+++ b/tests/ui/use_self_trait.rs
@@ -47,7 +47,6 @@ impl Mul for Bad {
 
 impl Clone for Bad {
     fn clone(&self) -> Self {
-        // FIXME: applicable here
         Bad
     }
 }
@@ -138,4 +137,16 @@ mod impl_in_macro {
     parse_ip_impl!(Foo); // Should not lint
 }
 
+mod full_path_replacement {
+    trait Error {
+        fn custom<T: std::fmt::Display>(_msg: T) -> Self;
+    }
+
+    impl Error for std::fmt::Error {
+        fn custom<T: std::fmt::Display>(_msg: T) -> Self {
+            std::fmt::Error // Should lint
+        }
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/use_self_trait.stderr b/tests/ui/use_self_trait.stderr
index 55af3ff2a93..090729b9c3d 100644
--- a/tests/ui/use_self_trait.stderr
+++ b/tests/ui/use_self_trait.stderr
@@ -84,5 +84,17 @@ error: unnecessary structure name repetition
 LL |     fn mul(self, rhs: Bad) -> Bad {
    |                               ^^^ help: use the applicable keyword: `Self`
 
-error: aborting due to 14 previous errors
+error: unnecessary structure name repetition
+  --> $DIR/use_self_trait.rs:50:9
+   |
+LL |         Bad
+   |         ^^^ help: use the applicable keyword: `Self`
+
+error: unnecessary structure name repetition
+  --> $DIR/use_self_trait.rs:147:13
+   |
+LL |             std::fmt::Error // Should lint
+   |             ^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self`
+
+error: aborting due to 16 previous errors