Extend the `use_self` lint to suggest uses of `Self::Variant`.

4 files changed, 76 insertions, 24 deletions

diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs
index b60b9ccab32..3f93e019c66 100644
--- a/clippy_lints/src/use_self.rs
+++ b/clippy_lints/src/use_self.rs
@@ -51,9 +51,11 @@ declare_lint_pass!(UseSelf => [USE_SELF]);
 
 const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element";
 
-fn span_use_self_lint(cx: &LateContext<'_, '_>, path: &Path) {
+fn span_use_self_lint(cx: &LateContext<'_, '_>, path: &Path, last_segment: Option<&PathSegment>) {
+    let last_segment = last_segment.unwrap_or_else(|| path.segments.last().expect(SEGMENTS_MSG));
+
     // Path segments only include actual path, no methods or fields.
-    let last_path_span = path.segments.last().expect(SEGMENTS_MSG).ident.span;
+    let last_path_span = last_segment.ident.span;
     // Only take path up to the end of last_path_span.
     let span = path.span.with_hi(last_path_span.hi());
 
@@ -80,22 +82,18 @@ impl<'a, 'tcx> Visitor<'tcx> for TraitImplTyVisitor<'a, 'tcx> {
         let trait_ty = self.trait_type_walker.next();
         let impl_ty = self.impl_type_walker.next();
 
-        if let TyKind::Path(QPath::Resolved(_, path)) = &t.node {
+        if_chain! {
+            if let TyKind::Path(QPath::Resolved(_, path)) = &t.node;
+
             // The implementation and trait types don't match which means that
             // the concrete type was specified by the implementation
-            if impl_ty != trait_ty {
-                if let Some(impl_ty) = impl_ty {
-                    if self.item_type == impl_ty {
-                        let is_self_ty = if let def::Res::SelfTy(..) = path.res {
-                            true
-                        } else {
-                            false
-                        };
-
-                        if !is_self_ty {
-                            span_use_self_lint(self.cx, path);
-                        }
-                    }
+            if impl_ty != trait_ty;
+            if let Some(impl_ty) = impl_ty;
+            if self.item_type == impl_ty;
+            then {
+                match path.res {
+                    def::Res::SelfTy(..) => {},
+                    _ => span_use_self_lint(self.cx, path, None)
                 }
             }
         }
@@ -220,15 +218,35 @@ struct UseSelfVisitor<'a, 'tcx> {
 
 impl<'a, 'tcx> Visitor<'tcx> for UseSelfVisitor<'a, 'tcx> {
     fn visit_path(&mut self, path: &'tcx Path, _id: HirId) {
+        if path.segments.len() >= 2 {
+            let last_but_one = &path.segments[path.segments.len() - 2];
+            if last_but_one.ident.name != kw::SelfUpper {
+                let enum_def_id = match path.res {
+                    Res::Def(DefKind::Variant, variant_def_id) =>
+                        self.cx.tcx.parent(variant_def_id),
+                    Res::Def(DefKind::Ctor(def::CtorOf::Variant, _), ctor_def_id) => {
+                        let variant_def_id = self.cx.tcx.parent(ctor_def_id);
+                        variant_def_id.and_then(|def_id| self.cx.tcx.parent(def_id))
+                    }
+                    _ => None
+                };
+
+                if self.item_path.res.opt_def_id() == enum_def_id {
+                    span_use_self_lint(self.cx, path, Some(last_but_one));
+                }
+            }
+        }
+
         if path.segments.last().expect(SEGMENTS_MSG).ident.name != kw::SelfUpper {
             if self.item_path.res == path.res {
-                span_use_self_lint(self.cx, path);
-            } else if let Res::Def(DefKind::Ctor(def::CtorOf::Struct, CtorKind::Fn), ctor_did) = path.res {
-                if self.item_path.res.opt_def_id() == self.cx.tcx.parent(ctor_did) {
-                    span_use_self_lint(self.cx, path);
+                span_use_self_lint(self.cx, path, None);
+            } else if let Res::Def(DefKind::Ctor(def::CtorOf::Struct, CtorKind::Fn), ctor_def_id) = path.res {
+                if self.item_path.res.opt_def_id() == self.cx.tcx.parent(ctor_def_id) {
+                    span_use_self_lint(self.cx, path, None);
                 }
             }
         }
+
         walk_path(self, path);
     }
 
diff --git a/tests/ui/use_self.fixed b/tests/ui/use_self.fixed
index 68af85030ab..ac2a1708b65 100644
--- a/tests/ui/use_self.fixed
+++ b/tests/ui/use_self.fixed
@@ -265,6 +265,8 @@ mod nesting {
 
     enum Enum {
         A,
+        B(u64),
+        C { field: bool }
     }
     impl Enum {
         fn method() {
@@ -272,6 +274,12 @@ mod nesting {
             use self::Enum::*; // Issue 3425
             static STATIC: Enum = Enum::A; // Can't use Self as type
         }
+
+        fn method2() {
+            let _ = Self::B(42);
+            let _ = Self::C { field: true };
+            let _ = Self::A;
+        }
     }
 }
 
diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs
index 7a6d415528a..21b5833e56e 100644
--- a/tests/ui/use_self.rs
+++ b/tests/ui/use_self.rs
@@ -265,6 +265,8 @@ mod nesting {
 
     enum Enum {
         A,
+        B(u64),
+        C { field: bool }
     }
     impl Enum {
         fn method() {
@@ -272,6 +274,12 @@ mod nesting {
             use self::Enum::*; // Issue 3425
             static STATIC: Enum = Enum::A; // Can't use Self as type
         }
+
+        fn method2() {
+            let _ = Enum::B(42);
+            let _ = Enum::C { field: true };
+            let _ = Enum::A;
+        }
     }
 }
 
diff --git a/tests/ui/use_self.stderr b/tests/ui/use_self.stderr
index bf1f41fd64e..12dd672e3f4 100644
--- a/tests/ui/use_self.stderr
+++ b/tests/ui/use_self.stderr
@@ -175,22 +175,40 @@ LL |                     Bar { foo: Foo {} }
    |                     ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:304:13
+  --> $DIR/use_self.rs:279:21
+   |
+LL |             let _ = Enum::B(42);
+   |                     ^^^^ help: use the applicable keyword: `Self`
+
+error: unnecessary structure name repetition
+  --> $DIR/use_self.rs:280:21
+   |
+LL |             let _ = Enum::C { field: true };
+   |                     ^^^^ help: use the applicable keyword: `Self`
+
+error: unnecessary structure name repetition
+  --> $DIR/use_self.rs:281:21
+   |
+LL |             let _ = Enum::A;
+   |                     ^^^^ help: use the applicable keyword: `Self`
+
+error: unnecessary structure name repetition
+  --> $DIR/use_self.rs:312:13
    |
 LL |             nested::A::fun_1();
    |             ^^^^^^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:305:13
+  --> $DIR/use_self.rs:313:13
    |
 LL |             nested::A::A;
    |             ^^^^^^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:307:13
+  --> $DIR/use_self.rs:315:13
    |
 LL |             nested::A {};
    |             ^^^^^^^^^ help: use the applicable keyword: `Self`
 
-error: aborting due to 31 previous errors
+error: aborting due to 34 previous errors