Fix use_self regressions

diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs
index 82dc0fd2b8ad0d4ab1bdcf41046a3a3327f3b867..a4450acea0913863df78853560c55821f0e82cf0 100644 (file)
--- a/clippy_lints/src/use_self.rs
+++ b/clippy_lints/src/use_self.rs
@@ -4,7 +4,6 @@
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use rustc::ty;
 use syntax::ast::NodeId;
-use syntax::symbol::keywords;
 use syntax_pos::symbol::keywords::SelfType;
 
 /// **What it does:** Checks for unnecessary repetition of structure name when a
@@ -58,26 +57,29 @@ fn span_use_self_lint(cx: &LateContext, path: &Path) {
 }
 
 struct TraitImplTyVisitor<'a, 'tcx: 'a> {
+    item_path: &'a Path,
     cx: &'a LateContext<'a, 'tcx>,
-    type_walker: ty::walk::TypeWalker<'tcx>,
+    trait_type_walker: ty::walk::TypeWalker<'tcx>,
+    impl_type_walker: ty::walk::TypeWalker<'tcx>,
 }
 
 impl<'a, 'tcx> Visitor<'tcx> for TraitImplTyVisitor<'a, 'tcx> {
     fn visit_ty(&mut self, t: &'tcx Ty) {
-        let trait_ty = self.type_walker.next();
+        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 {
-            let impl_is_self_ty = if let def::Def::SelfTy(..) = path.def {
-                true
-            } else {
-                false
-            };
-            if !impl_is_self_ty {
-                let trait_is_self_ty = if let Some(ty::TyParam(ty::ParamTy { name, .. })) = trait_ty.map(|ty| &ty.sty) {
-                    *name == keywords::SelfType.name().as_str()
+            if self.item_path.def == path.def {
+                let is_self_ty = if let def::Def::SelfTy(..) = path.def {
+                    true
                 } else {
                     false
                 };
-                if trait_is_self_ty {
+
+                if !is_self_ty && impl_ty != trait_ty {
+                    // The implementation and trait types don't match which means that
+                    // the concrete type was specified by the implementation but
+                    // it didn't use `Self`
                     span_use_self_lint(self.cx, path);
                 }
             }
@@ -92,6 +94,7 @@ fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
 
 fn check_trait_method_impl_decl<'a, 'tcx: 'a>(
     cx: &'a LateContext<'a, 'tcx>,
+    item_path: &'a Path,
     impl_item: &ImplItem,
     impl_decl: &'tcx FnDecl,
     impl_trait_ref: &ty::TraitRef,
@@ -110,24 +113,30 @@ fn check_trait_method_impl_decl<'a, 'tcx: 'a>(
     let trait_method_sig = cx.tcx.fn_sig(trait_method.def_id);
     let trait_method_sig = cx.tcx.erase_late_bound_regions(&trait_method_sig);
 
+    let impl_method_def_id = cx.tcx.hir.local_def_id(impl_item.id);
+    let impl_method_sig = cx.tcx.fn_sig(impl_method_def_id);
+    let impl_method_sig = cx.tcx.erase_late_bound_regions(&impl_method_sig);
+
     let output_ty = if let FunctionRetTy::Return(ty) = &impl_decl.output {
         Some(&**ty)
     } else {
         None
     };
 
-    for (impl_ty, trait_ty) in impl_decl
-        .inputs
-        .iter()
-        .chain(output_ty)
-        .zip(trait_method_sig.inputs_and_output)
-    {
+    for (impl_decl_ty, (impl_ty, trait_ty)) in impl_decl.inputs.iter().chain(output_ty).zip(
+        impl_method_sig
+            .inputs_and_output
+            .iter()
+            .zip(trait_method_sig.inputs_and_output),
+    ) {
         let mut visitor = TraitImplTyVisitor {
             cx,
-            type_walker: trait_ty.walk(),
+            item_path,
+            trait_type_walker: trait_ty.walk(),
+            impl_type_walker: impl_ty.walk(),
         };
 
-        visitor.visit_ty(&impl_ty);
+        visitor.visit_ty(&impl_decl_ty);
     }
 }
 
@@ -163,7 +172,7 @@ fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) {
                             let impl_item = cx.tcx.hir.impl_item(impl_item_ref.id);
                             if let ImplItemKind::Method(MethodSig{ decl: impl_decl, .. }, impl_body_id)
                                     = &impl_item.node {
-                                check_trait_method_impl_decl(cx, impl_item, impl_decl, &impl_trait_ref);
+                                check_trait_method_impl_decl(cx, item_path, impl_item, impl_decl, &impl_trait_ref);
                                 let body = cx.tcx.hir.body(*impl_body_id);
                                 visitor.visit_body(body);
                             } else {

diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs
index e3133b0a7a1f078233349b0471753bd6d59f2643..689c9d68d1208aa7504f6f985f32ad4a7f0bc074 100644 (file)
--- a/tests/ui/use_self.rs
+++ b/tests/ui/use_self.rs
@@ -1,10 +1,6 @@
-
-
 #![warn(use_self)]
 #![allow(dead_code)]
 #![allow(should_implement_trait)]
-#![allow(boxed_local)]
-
 
 fn main() {}
 
@@ -68,9 +64,10 @@ fn clone(&self) -> Foo<'a> {
     }
 }
 
+#[allow(boxed_local)]
 mod traits {
 
-    #![cfg_attr(feature = "cargo-clippy", allow(boxed_local))]
+    use std::ops::Mul;
 
     trait SelfTrait {
         fn refs(p1: &Self) -> &Self;
@@ -104,6 +101,14 @@ fn vals(_: Bad) -> Bad {
         }
     }
 
+    impl Mul for Bad {
+        type Output = Bad;
+
+        fn mul(self, rhs: Bad) -> Bad {
+            rhs
+        }
+    }
+
     #[derive(Default)]
     struct Good;
 
@@ -128,6 +133,14 @@ fn vals(_: Self) -> Self {
         }
     }
 
+    impl Mul for Good {
+        type Output = Self;
+
+        fn mul(self, rhs: Self) -> Self {
+            rhs
+        }
+    }
+
     trait NameTrait {
         fn refs(p1: &u8) -> &u8;
         fn ref_refs<'a>(p1: &'a &'a u8) -> &'a &'a u8;
@@ -162,7 +175,7 @@ fn vals(_: Self) -> Self {
     impl Clone for Good {
         fn clone(&self) -> Self {
             // Note: Not linted and it wouldn't be valid
-            // because "can't use `Self` as a constructor`
+            // because "can't use `Self` as a constructor`"
             Good
         }
     }

diff --git a/tests/ui/use_self.stderr b/tests/ui/use_self.stderr
index ede95126f860d7a4e15b4cf6d28da4f697ab2746..899361012524f94e5550ad082c2ab033276b6c08 100644 (file)
--- a/tests/ui/use_self.stderr
+++ b/tests/ui/use_self.stderr
@@ -1,106 +1,124 @@
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:15:21
+  --> $DIR/use_self.rs:11:21
    |
-15 |         fn new() -> Foo {
+11 |         fn new() -> Foo {
    |                     ^^^ help: use the applicable keyword: `Self`
    |
    = note: `-D use-self` implied by `-D warnings`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:16:13
+  --> $DIR/use_self.rs:12:13
    |
-16 |             Foo {}
+12 |             Foo {}
    |             ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:18:22
+  --> $DIR/use_self.rs:14:22
    |
-18 |         fn test() -> Foo {
+14 |         fn test() -> Foo {
    |                      ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:19:13
+  --> $DIR/use_self.rs:15:13
    |
-19 |             Foo::new()
+15 |             Foo::new()
    |             ^^^^^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:24:25
+  --> $DIR/use_self.rs:20:25
    |
-24 |         fn default() -> Foo {
+20 |         fn default() -> Foo {
    |                         ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:25:13
+  --> $DIR/use_self.rs:21:13
    |
-25 |             Foo::new()
+21 |             Foo::new()
    |             ^^^^^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:87:22
+  --> $DIR/use_self.rs:84:22
    |
-87 |         fn refs(p1: &Bad) -> &Bad {
+84 |         fn refs(p1: &Bad) -> &Bad {
    |                      ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:87:31
+  --> $DIR/use_self.rs:84:31
    |
-87 |         fn refs(p1: &Bad) -> &Bad {
+84 |         fn refs(p1: &Bad) -> &Bad {
    |                               ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:91:37
+  --> $DIR/use_self.rs:88:37
    |
-91 |         fn ref_refs<'a>(p1: &'a &'a Bad) -> &'a &'a Bad {
+88 |         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.rs:91:53
+  --> $DIR/use_self.rs:88:53
    |
-91 |         fn ref_refs<'a>(p1: &'a &'a Bad) -> &'a &'a Bad {
+88 |         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.rs:95:30
+  --> $DIR/use_self.rs:92:30
    |
-95 |         fn mut_refs(p1: &mut Bad) -> &mut Bad {
+92 |         fn mut_refs(p1: &mut Bad) -> &mut Bad {
    |                              ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:95:43
+  --> $DIR/use_self.rs:92:43
    |
-95 |         fn mut_refs(p1: &mut Bad) -> &mut Bad {
+92 |         fn mut_refs(p1: &mut Bad) -> &mut Bad {
    |                                           ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:99:28
+  --> $DIR/use_self.rs:96:28
    |
-99 |         fn nested(_p1: Box<Bad>, _p2: (&u8, &Bad)) {
+96 |         fn nested(_p1: Box<Bad>, _p2: (&u8, &Bad)) {
    |                            ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:99:46
+  --> $DIR/use_self.rs:96:46
    |
-99 |         fn nested(_p1: Box<Bad>, _p2: (&u8, &Bad)) {
+96 |         fn nested(_p1: Box<Bad>, _p2: (&u8, &Bad)) {
    |                                              ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-   --> $DIR/use_self.rs:102:20
+  --> $DIR/use_self.rs:99:20
+   |
+99 |         fn vals(_: Bad) -> Bad {
+   |                    ^^^ help: use the applicable keyword: `Self`
+
+error: unnecessary structure name repetition
+  --> $DIR/use_self.rs:99:28
+   |
+99 |         fn vals(_: Bad) -> Bad {
+   |                            ^^^ help: use the applicable keyword: `Self`
+
+error: unnecessary structure name repetition
+   --> $DIR/use_self.rs:100:13
     |
-102 |         fn vals(_: Bad) -> Bad {
-    |                    ^^^ help: use the applicable keyword: `Self`
+100 |             Bad::default()
+    |             ^^^^^^^^^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-   --> $DIR/use_self.rs:102:28
+   --> $DIR/use_self.rs:105:23
     |
-102 |         fn vals(_: Bad) -> Bad {
-    |                            ^^^ help: use the applicable keyword: `Self`
+105 |         type Output = Bad;
+    |                       ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-   --> $DIR/use_self.rs:103:13
+   --> $DIR/use_self.rs:107:27
     |
-103 |             Bad::default()
-    |             ^^^^^^^^^^^^ help: use the applicable keyword: `Self`
+107 |         fn mul(self, rhs: Bad) -> Bad {
+    |                           ^^^ help: use the applicable keyword: `Self`
+
+error: unnecessary structure name repetition
+   --> $DIR/use_self.rs:107:35
+    |
+107 |         fn mul(self, rhs: Bad) -> Bad {
+    |                                   ^^^ help: use the applicable keyword: `Self`
 
-error: aborting due to 17 previous errors
+error: aborting due to 20 previous errors