From: Josh Mcguigan <joshmcg88@gmail.com>
Date: Fri, 5 Oct 2018 02:01:04 +0000 (-0700)
Subject: new_ret_no_self correctly lint impl return
X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=a5e4805ecf1ff5b38f0d467ba90530e43bfd0d9c;p=rust.git

new_ret_no_self correctly lint impl return
---

diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index c78bb48db2a..f9c010beea7 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -11,7 +11,7 @@
 use crate::rustc::hir;
 use crate::rustc::hir::def::Def;
 use crate::rustc::lint::{in_external_macro, LateContext, LateLintPass, Lint, LintArray, LintContext, LintPass};
-use crate::rustc::ty::{self, Ty};
+use crate::rustc::ty::{self, Ty, TyKind, Predicate};
 use crate::rustc::{declare_tool_lint, lint_array};
 use crate::rustc_errors::Applicability;
 use crate::syntax::ast;
@@ -933,9 +933,27 @@ fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, implitem: &'tcx hir::I
 
         if let hir::ImplItemKind::Method(_, _) = implitem.node {
             let ret_ty = return_ty(cx, implitem.id);
-            if name == "new" &&
-                !same_tys(cx, ret_ty, ty) &&
-                !ret_ty.is_impl_trait() {
+
+            // if return type is impl trait
+            if let TyKind::Opaque(def_id, _) = ret_ty.sty {
+
+                // then one of the associated types must be Self
+                for predicate in cx.tcx.predicates_of(def_id).predicates.iter() {
+                    match predicate {
+                        (Predicate::Projection(poly_projection_predicate), _) => {
+                            let binder = poly_projection_predicate.ty();
+                            let associated_type = binder.skip_binder();
+                            let associated_type_is_self_type = same_tys(cx, ty, associated_type);
+
+                            // if the associated type is self, early return and do not trigger lint
+                            if associated_type_is_self_type { return; }
+                        },
+                        (_, _) => {},
+                    }
+                }
+            }
+
+            if name == "new" && !same_tys(cx, ret_ty, ty) {
                 span_lint(cx,
                           NEW_RET_NO_SELF,
                           implitem.span,
diff --git a/tests/ui/new_ret_no_self.rs b/tests/ui/new_ret_no_self.rs
index 3b7ff7780ef..e9f41d34133 100644
--- a/tests/ui/new_ret_no_self.rs
+++ b/tests/ui/new_ret_no_self.rs
@@ -9,6 +9,11 @@ trait R {
     type Item;
 }
 
+trait Q {
+    type Item;
+    type Item2;
+}
+
 struct S;
 
 impl R for S {
@@ -42,12 +47,26 @@ impl R for S3 {
 }
 
 impl S3 {
-    // should trigger the lint, but currently does not
+    // should trigger the lint
     pub fn new(_: String) -> impl R<Item = u32> {
         S3
     }
 }
 
+struct S4;
+
+impl Q for S4 {
+    type Item = u32;
+    type Item2 = Self;
+}
+
+impl S4 {
+    // should not trigger the lint
+    pub fn new(_: String) -> impl Q<Item = u32, Item2 = Self> {
+        S4
+    }
+}
+
 struct T;
 
 impl T {
diff --git a/tests/ui/new_ret_no_self.stderr b/tests/ui/new_ret_no_self.stderr
index cab5fa55cb6..aa3a633c418 100644
--- a/tests/ui/new_ret_no_self.stderr
+++ b/tests/ui/new_ret_no_self.stderr
@@ -1,20 +1,28 @@
 error: methods called `new` usually return `Self`
-  --> $DIR/new_ret_no_self.rs:64:5
+  --> $DIR/new_ret_no_self.rs:51:5
    |
-64 | /     pub fn new() -> u32 {
-65 | |         unimplemented!();
-66 | |     }
+51 | /     pub fn new(_: String) -> impl R<Item = u32> {
+52 | |         S3
+53 | |     }
    | |_____^
    |
    = note: `-D clippy::new-ret-no-self` implied by `-D warnings`
 
 error: methods called `new` usually return `Self`
-  --> $DIR/new_ret_no_self.rs:73:5
+  --> $DIR/new_ret_no_self.rs:83:5
    |
-73 | /     pub fn new(_: String) -> u32 {
-74 | |         unimplemented!();
-75 | |     }
+83 | /     pub fn new() -> u32 {
+84 | |         unimplemented!();
+85 | |     }
    | |_____^
 
-error: aborting due to 2 previous errors
+error: methods called `new` usually return `Self`
+  --> $DIR/new_ret_no_self.rs:92:5
+   |
+92 | /     pub fn new(_: String) -> u32 {
+93 | |         unimplemented!();
+94 | |     }
+   | |_____^
+
+error: aborting due to 3 previous errors