From eb854b233c353441f86c4a346a941c5965a2333a Mon Sep 17 00:00:00 2001
From: Josh Mcguigan <joshmcg88@gmail.com>
Date: Tue, 2 Oct 2018 20:11:56 -0700
Subject: [PATCH] new_ret_no_self added positive test cases

---
 clippy_lints/src/methods/mod.rs | 22 ++++++------
 tests/ui/new_ret_no_self.rs     | 63 +++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 11 deletions(-)
 create mode 100644 tests/ui/new_ret_no_self.rs

diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 7c15eb677cc..d11dbf0e773 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -878,6 +878,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
         let name = implitem.ident.name;
         let parent = cx.tcx.hir.get_parent(implitem.id);
         let item = cx.tcx.hir.expect_item(parent);
+        let def_id = cx.tcx.hir.local_def_id(item.id);
+        let ty = cx.tcx.type_of(def_id);
         if_chain! {
             if let hir::ImplItemKind::Method(ref sig, id) = implitem.node;
             if let Some(first_arg_ty) = sig.decl.inputs.get(0);
@@ -899,8 +901,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
                 }
 
                 // check conventions w.r.t. conversion method names and predicates
-                let def_id = cx.tcx.hir.local_def_id(item.id);
-                let ty = cx.tcx.type_of(def_id);
                 let is_copy = is_copy(cx, ty);
                 for &(ref conv, self_kinds) in &CONVENTIONS {
                     if conv.check(&name.as_str()) {
@@ -928,17 +928,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
                         break;
                     }
                 }
-
-                let ret_ty = return_ty(cx, implitem.id);
-                if name == "new" &&
-                   !ret_ty.walk().any(|t| same_tys(cx, t, ty)) {
-                    span_lint(cx,
-                              NEW_RET_NO_SELF,
-                              implitem.span,
-                              "methods called `new` usually return `Self`");
-                }
             }
         }
+
+        let ret_ty = return_ty(cx, implitem.id);
+        if name == "new" &&
+            !ret_ty.walk().any(|t| same_tys(cx, t, ty)) {
+            span_lint(cx,
+                      NEW_RET_NO_SELF,
+                      implitem.span,
+                      "methods called `new` usually return `Self`");
+        }
     }
 }
 
diff --git a/tests/ui/new_ret_no_self.rs b/tests/ui/new_ret_no_self.rs
new file mode 100644
index 00000000000..67933f00262
--- /dev/null
+++ b/tests/ui/new_ret_no_self.rs
@@ -0,0 +1,63 @@
+#![feature(tool_lints)]
+
+#![warn(clippy::new_ret_no_self)]
+#![allow(dead_code, clippy::trivially_copy_pass_by_ref)]
+
+fn main(){}
+
+//trait R {
+//    type Item;
+//}
+//
+//struct S;
+//
+//impl R for S {
+//    type Item = Self;
+//}
+//
+//impl S {
+//    // should not trigger the lint
+//    pub fn new() -> impl R<Item = Self> {
+//        S
+//    }
+//}
+//
+//struct S2;
+//
+//impl R for S2 {
+//    type Item = Self;
+//}
+//
+//impl S2 {
+//    // should not trigger the lint
+//    pub fn new(_: String) -> impl R<Item = Self> {
+//        S2
+//    }
+//}
+//
+//struct T;
+//
+//impl T {
+//    // should not trigger lint
+//    pub fn new() -> Self {
+//        unimplemented!();
+//    }
+//}
+
+struct U;
+
+impl U {
+    // should trigger lint
+    pub fn new() -> u32 {
+        unimplemented!();
+    }
+}
+
+struct V;
+
+impl V {
+    // should trigger lint
+    pub fn new(_: String) -> u32 {
+        unimplemented!();
+    }
+}