use_self - fix issue with hir_ty_to_ty

clippy_lints/src/use_self.rs

@@ -57,7 +57,7 @@ declare_lint_pass!(UseSelf => [USE_SELF]);
 
 const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element";
 
-fn span_lint<'tcx>(cx: &LateContext<'tcx>, span: Span) {
+fn span_lint(cx: &LateContext<'_>, span: Span) {
     span_lint_and_sugg(
         cx,
         USE_SELF,
@@ -99,12 +99,12 @@ fn span_lint_on_qpath_resolved<'tcx>(cx: &LateContext<'tcx>, qpath: &'tcx QPath<
     }
 }
 
-struct ImplVisitor<'a, 'tcx> {
+struct BodyVisitor<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
     self_ty: Ty<'tcx>,
 }
 
-impl<'a, 'tcx> ImplVisitor<'a, 'tcx> {
+impl<'a, 'tcx> BodyVisitor<'a, 'tcx> {
     fn check_trait_method_impl_decl(
         &mut self,
         impl_item: &ImplItem<'tcx>,
@@ -151,46 +151,13 @@ impl<'a, 'tcx> ImplVisitor<'a, 'tcx> {
     }
 }
 
-impl<'a, 'tcx> Visitor<'tcx> for ImplVisitor<'a, 'tcx> {
+impl<'a, 'tcx> Visitor<'tcx> for BodyVisitor<'a, 'tcx> {
     type Map = Map<'tcx>;
 
     fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
         NestedVisitorMap::OnlyBodies(self.cx.tcx.hir())
     }
 
-    fn visit_ty(&mut self, hir_ty: &'tcx hir::Ty<'tcx>) {
-        if let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind {
-            match path.res {
-                def::Res::SelfTy(..) => {},
-                _ => {
-                    match self.cx.tcx.hir().find(self.cx.tcx.hir().get_parent_node(hir_ty.hir_id)) {
-                        Some(Node::Expr(Expr {
-                            kind: ExprKind::Path(QPath::TypeRelative(_, _segment)),
-                            ..
-                        })) => {
-                            // The following block correctly identifies applicable lint locations
-                            // but `hir_ty_to_ty` calls cause odd ICEs.
-                            //
-                            // if hir_ty_to_ty(self.cx.tcx, hir_ty) == self.self_ty {
-                            //     // FIXME: this span manipulation should not be necessary
-                            //     // @flip1995 found an ast lowering issue in
-                            //     // https://github.com/rust-lang/rust/blob/master/src/librustc_ast_lowering/path.rs#L142-L162
-                            //     span_lint_until_last_segment(self.cx, hir_ty.span, segment);
-                            // }
-                        },
-                        _ => {
-                            if hir_ty_to_ty(self.cx.tcx, hir_ty) == self.self_ty {
-                                span_lint(self.cx, hir_ty.span)
-                            }
-                        },
-                    }
-                },
-            }
-        }
-
-        walk_ty(self, hir_ty);
-    }
-
     fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
         fn expr_ty_matches<'tcx>(expr: &'tcx Expr<'tcx>, self_ty: Ty<'tcx>, cx: &LateContext<'tcx>) -> bool {
             let def_id = expr.hir_id.owner;
@@ -247,6 +214,52 @@ impl<'a, 'tcx> Visitor<'tcx> for ImplVisitor<'a, 'tcx> {
     }
 }
 
+struct FnSigVisitor<'a, 'tcx> {
+    cx: &'a LateContext<'tcx>,
+    self_ty: Ty<'tcx>,
+}
+
+impl<'a, 'tcx> Visitor<'tcx> for FnSigVisitor<'a, 'tcx> {
+    type Map = Map<'tcx>;
+
+    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
+        NestedVisitorMap::None
+    }
+
+    fn visit_ty(&mut self, hir_ty: &'tcx hir::Ty<'tcx>) {
+        if let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind {
+            match path.res {
+                def::Res::SelfTy(..) => {},
+                _ => {
+                    match self.cx.tcx.hir().find(self.cx.tcx.hir().get_parent_node(hir_ty.hir_id)) {
+                        Some(Node::Expr(Expr {
+                            kind: ExprKind::Path(QPath::TypeRelative(_, segment)),
+                            ..
+                        })) => {
+                            // The following block correctly identifies applicable lint locations
+                            // but `hir_ty_to_ty` calls cause odd ICEs.
+                            //
+                            if hir_ty_to_ty(self.cx.tcx, hir_ty) == self.self_ty {
+                                // fixme: this span manipulation should not be necessary
+                                // @flip1995 found an ast lowering issue in
+                                // https://github.com/rust-lang/rust/blob/master/src/librustc_ast_lowering/path.rs#l142-l162
+                                span_lint_until_last_segment(self.cx, hir_ty.span, segment);
+                            }
+                        },
+                        _ => {
+                            if hir_ty_to_ty(self.cx.tcx, hir_ty) == self.self_ty {
+                                span_lint(self.cx, hir_ty.span)
+                            }
+                        },
+                    }
+                },
+            }
+        }
+
+        walk_ty(self, hir_ty);
+    }
+}
+
 impl<'tcx> LateLintPass<'tcx> for UseSelf {
     fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) {
         if in_external_macro(cx.sess(), impl_item.span) {
@@ -270,7 +283,8 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
                 // TODO: don't short-circuit upon lifetime parameters
                 if should_check {
                     let self_ty = hir_ty_to_ty(cx.tcx, hir_self_ty);
-                    let visitor = &mut ImplVisitor { cx, self_ty };
+                    let body_visitor = &mut BodyVisitor { cx, self_ty };
+                    let fn_sig_visitor = &mut FnSigVisitor { cx, self_ty };
 
                     let tcx = cx.tcx;
                     let impl_def_id = tcx.hir().local_def_id(imp.hir_id);
@@ -279,11 +293,12 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
                         if let Some(impl_trait_ref) = impl_trait_ref;
                         if let ImplItemKind::Fn(FnSig { decl: impl_decl, .. }, impl_body_id) = &impl_item.kind;
                         then {
-                            visitor.check_trait_method_impl_decl(impl_item, impl_decl, impl_trait_ref);
+                            body_visitor.check_trait_method_impl_decl(impl_item, impl_decl, impl_trait_ref);
                             let body = tcx.hir().body(*impl_body_id);
-                            visitor.visit_body(body);
+                            body_visitor.visit_body(body);
                         } else {
-                            walk_impl_item(visitor, impl_item)
+                            walk_impl_item(body_visitor, impl_item);
+                            walk_impl_item(fn_sig_visitor, impl_item);
                         }
                     }
                 }

tests/ui/use_self.fixed

@@ -15,12 +15,14 @@ mod use_self {
             Self {}
         }
         fn test() -> Self {
+            // FIXME: applicable here
             Foo::new()
         }
     }
 
     impl Default for Foo {
-        fn default() -> Self {
+        // FIXME: applicable here
+        fn default() -> Foo {
             // FIXME: applicable here
             Foo::new()
         }
@@ -213,7 +215,9 @@ mod rustfix {
         fn fun_1() {}
 
         fn fun_2() {
+            // FIXME: applicable here
             nested::A::fun_1();
+            // FIXME: applicable here
             nested::A::A;
 
             Self {};

tests/ui/use_self.rs

@@ -15,11 +15,13 @@ mod use_self {
             Foo {}
         }
         fn test() -> Foo {
+            // FIXME: applicable here
             Foo::new()
         }
     }
 
     impl Default for Foo {
+        // FIXME: applicable here
         fn default() -> Foo {
             // FIXME: applicable here
             Foo::new()
@@ -213,7 +215,9 @@ mod rustfix {
         fn fun_1() {}
 
         fn fun_2() {
+            // FIXME: applicable here
             nested::A::fun_1();
+            // FIXME: applicable here
             nested::A::A;
 
             nested::A {};

tests/ui/use_self.stderr

@@ -1,15 +1,15 @@
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:14:21
+  --> $DIR/use_self.rs:15:13
    |
-LL |         fn new() -> Foo {
+LL |             Foo {}
    |             ^^^ help: use the applicable keyword: `Self`
    |
    = note: `-D clippy::use-self` implied by `-D warnings`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:15:13
+  --> $DIR/use_self.rs:14:21
    |
-LL |             Foo {}
+LL |         fn new() -> Foo {
    |                     ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
@@ -19,25 +19,30 @@ LL |         fn test() -> Foo {
    |                      ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:23:25
+  --> $DIR/use_self.rs:96:24
-   |
-LL |         fn default() -> Foo {
-   |                         ^^^ help: use the applicable keyword: `Self`
-
-error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:94:24
    |
 LL |         fn bad(foos: &[Foo]) -> impl Iterator<Item = &Foo> {
    |                        ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:109:13
+  --> $DIR/use_self.rs:111:13
    |
 LL |             TS(0)
    |             ^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:117:25
+  --> $DIR/use_self.rs:120:17
+   |
+LL |                 Foo {}
+   |                 ^^^ help: use the applicable keyword: `Self`
+...
+LL |         use_self_expand!(); // Should lint in local macros
+   |         ------------------- in this macro invocation
+   |
+   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: unnecessary structure name repetition
+  --> $DIR/use_self.rs:119:25
    |
 LL |             fn new() -> Foo {
    |                         ^^^ help: use the applicable keyword: `Self`
@@ -48,93 +53,82 @@ LL |         use_self_expand!(); // Should lint in local macros
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:118:17
+  --> $DIR/use_self.rs:144:21
-   |
-LL |                 Foo {}
-   |                 ^^^ help: use the applicable keyword: `Self`
-...
-LL |         use_self_expand!(); // Should lint in local macros
-   |         ------------------- in this macro invocation
-   |
-   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
-
-error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:141:29
-   |
-LL |                 fn bar() -> Bar {
-   |                             ^^^ help: use the applicable keyword: `Self`
-
-error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:142:21
    |
 LL |                     Bar { foo: Foo {} }
    |                     ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:153:21
+  --> $DIR/use_self.rs:143:29
    |
-LL |         fn baz() -> Foo {
+LL |                 fn bar() -> Bar {
    |                             ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:154:13
+  --> $DIR/use_self.rs:156:13
    |
 LL |             Foo {}
    |             ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:171:21
+  --> $DIR/use_self.rs:155:21
+   |
+LL |         fn baz() -> Foo {
+   |                     ^^^ help: use the applicable keyword: `Self`
+
+error: unnecessary structure name repetition
+  --> $DIR/use_self.rs:173:21
    |
 LL |             let _ = Enum::B(42);
    |                     ^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:172:21
+  --> $DIR/use_self.rs:174:21
    |
 LL |             let _ = Enum::C { field: true };
    |                     ^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:173:21
+  --> $DIR/use_self.rs:175:21
    |
 LL |             let _ = Enum::A;
    |                     ^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:218:13
+  --> $DIR/use_self.rs:222:13
    |
 LL |             nested::A {};
    |             ^^^^^^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:254:13
+  --> $DIR/use_self.rs:258:13
    |
 LL |             S {}
    |             ^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:282:29
+  --> $DIR/use_self.rs:287:13
-   |
-LL |         fn foo(value: T) -> Foo<T> {
-   |                             ^^^^^^ help: use the applicable keyword: `Self`
-
-error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:283:13
    |
 LL |             Foo { value }
    |             ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:320:21
+  --> $DIR/use_self.rs:286:29
+   |
+LL |         fn foo(value: T) -> Foo<T> {
+   |                             ^^^^^^ help: use the applicable keyword: `Self`
+
+error: unnecessary structure name repetition
+  --> $DIR/use_self.rs:324:21
    |
 LL |         type From = T::From;
    |                     ^^^^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:321:19
+  --> $DIR/use_self.rs:325:19
    |
 LL |         type To = T::To;
    |                   ^^^^^ help: use the applicable keyword: `Self`
 
-error: aborting due to 21 previous errors
+error: aborting due to 20 previous errors
 

tests/ui/use_self_trait.fixed

@@ -18,21 +18,21 @@ trait SelfTrait {
 struct Bad;
 
 impl SelfTrait for Bad {
-    fn refs(p1: &Self) -> &Self {
+    fn refs(p1: &Bad) -> &Bad {
         p1
     }
 
-    fn ref_refs<'a>(p1: &'a &'a Self) -> &'a &'a Self {
+    fn ref_refs<'a>(p1: &'a &'a Bad) -> &'a &'a Bad {
         p1
     }
 
-    fn mut_refs(p1: &mut Self) -> &mut Self {
+    fn mut_refs(p1: &mut Bad) -> &mut Bad {
         p1
     }
 
-    fn nested(_p1: Box<Self>, _p2: (&u8, &Self)) {}
+    fn nested(_p1: Box<Bad>, _p2: (&u8, &Bad)) {}
 
-    fn vals(_: Self) -> Self {
+    fn vals(_: Bad) -> Bad {
         Bad::default()
     }
 }
@@ -40,7 +40,7 @@ impl SelfTrait for Bad {
 impl Mul for Bad {
     type Output = Self;
 
-    fn mul(self, rhs: Self) -> Self {
+    fn mul(self, rhs: Bad) -> Bad {
         rhs
     }
 }

tests/ui/use_self_trait.stderr

@@ -1,82 +1,10 @@
-error: unnecessary structure name repetition
-  --> $DIR/use_self_trait.rs:21:18
-   |
-LL |     fn refs(p1: &Bad) -> &Bad {
-   |                  ^^^ help: use the applicable keyword: `Self`
-   |
-   = note: `-D clippy::use-self` implied by `-D warnings`
-
-error: unnecessary structure name repetition
-  --> $DIR/use_self_trait.rs:21:27
-   |
-LL |     fn refs(p1: &Bad) -> &Bad {
-   |                           ^^^ help: use the applicable keyword: `Self`
-
-error: unnecessary structure name repetition
-  --> $DIR/use_self_trait.rs:25:33
-   |
-LL |     fn ref_refs<'a>(p1: &'a &'a Bad) -> &'a &'a Bad {
-   |                                 ^^^ help: use the applicable keyword: `Self`
-
-error: unnecessary structure name repetition
-  --> $DIR/use_self_trait.rs:25:49
-   |
-LL |     fn ref_refs<'a>(p1: &'a &'a Bad) -> &'a &'a Bad {
-   |                                                 ^^^ help: use the applicable keyword: `Self`
-
-error: unnecessary structure name repetition
-  --> $DIR/use_self_trait.rs:29:26
-   |
-LL |     fn mut_refs(p1: &mut Bad) -> &mut Bad {
-   |                          ^^^ help: use the applicable keyword: `Self`
-
-error: unnecessary structure name repetition
-  --> $DIR/use_self_trait.rs:29:39
-   |
-LL |     fn mut_refs(p1: &mut Bad) -> &mut Bad {
-   |                                       ^^^ help: use the applicable keyword: `Self`
-
-error: unnecessary structure name repetition
-  --> $DIR/use_self_trait.rs:33:24
-   |
-LL |     fn nested(_p1: Box<Bad>, _p2: (&u8, &Bad)) {}
-   |                        ^^^ help: use the applicable keyword: `Self`
-
-error: unnecessary structure name repetition
-  --> $DIR/use_self_trait.rs:33:42
-   |
-LL |     fn nested(_p1: Box<Bad>, _p2: (&u8, &Bad)) {}
-   |                                          ^^^ help: use the applicable keyword: `Self`
-
-error: unnecessary structure name repetition
-  --> $DIR/use_self_trait.rs:35:16
-   |
-LL |     fn vals(_: Bad) -> Bad {
-   |                ^^^ help: use the applicable keyword: `Self`
-
-error: unnecessary structure name repetition
-  --> $DIR/use_self_trait.rs:35:24
-   |
-LL |     fn vals(_: Bad) -> Bad {
-   |                        ^^^ help: use the applicable keyword: `Self`
-
 error: unnecessary structure name repetition
   --> $DIR/use_self_trait.rs:41:19
    |
 LL |     type Output = Bad;
    |                   ^^^ help: use the applicable keyword: `Self`
-
-error: unnecessary structure name repetition
-  --> $DIR/use_self_trait.rs:43:23
    |
-LL |     fn mul(self, rhs: Bad) -> Bad {
+   = note: `-D clippy::use-self` implied by `-D warnings`
-   |                       ^^^ help: use the applicable keyword: `Self`
 
-error: unnecessary structure name repetition
+error: aborting due to previous error
-  --> $DIR/use_self_trait.rs:43:31
-   |
-LL |     fn mul(self, rhs: Bad) -> Bad {
-   |                               ^^^ help: use the applicable keyword: `Self`
-
-error: aborting due to 13 previous errors