[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 rust-lang#4140 since it's been closed, and refactor the lint
flip1995 · Oct 26, 2022 · 1909a6a · 1909a6a
1 parent e86e810
commit 1909a6a
Show file tree

Hide file tree

Showing 9 changed files with 87 additions and 33 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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