Fix types comparison

Author: mcarton

src/methods.rs

@@ -10,7 +10,7 @@ use std::{fmt, iter};
 use syntax::codemap::Span;
 use syntax::ptr::P;
 use utils::{get_trait_def_id, implements_trait, in_external_macro, in_macro, match_path, match_trait_method,
-            match_type, method_chain_args, return_ty, snippet, snippet_opt, span_lint,
+            match_type, method_chain_args, return_ty, same_tys, snippet, snippet_opt, span_lint,
             span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth};
 use utils::{BTREEMAP_ENTRY_PATH, DEFAULT_TRAIT_PATH, HASHMAP_ENTRY_PATH, OPTION_PATH, RESULT_PATH, STRING_PATH,
             VEC_PATH};
@@ -432,7 +432,7 @@ impl LateLintPass for MethodsPass {
                     }
 
                     let ret_ty = return_ty(cx.tcx.node_id_to_type(implitem.id));
-                    if &name.as_str() == &"new" && !ret_ty.map_or(false, |ret_ty| ret_ty.walk().any(|t| t == ty)) {
+                    if &name.as_str() == &"new" && !ret_ty.map_or(false, |ret_ty| ret_ty.walk().any(|t| same_tys(cx, t, ty))) {
                         span_lint(cx,
                                   NEW_RET_NO_SELF,
                                   sig.explicit_self.span,

src/new_without_default.rs

@@ -3,7 +3,8 @@ use rustc_front::hir;
 use rustc_front::intravisit::FnKind;
 use syntax::ast;
 use syntax::codemap::Span;
-use utils::{get_trait_def_id, implements_trait, in_external_macro, return_ty, span_lint, DEFAULT_TRAIT_PATH};
+use utils::{get_trait_def_id, implements_trait, in_external_macro, return_ty, same_tys, span_lint,
+            DEFAULT_TRAIT_PATH};
 
 /// **What it does:** This lints about type with a `fn new() -> Self` method and no `Default`
 /// implementation.
@@ -49,16 +50,15 @@ impl LateLintPass for NewWithoutDefault {
             if decl.inputs.is_empty() && name.as_str() == "new" {
                 let self_ty = cx.tcx.lookup_item_type(cx.tcx.map.local_def_id(cx.tcx.map.get_parent(id))).ty;
 
-                let ret_ty = return_ty(cx.tcx.node_id_to_type(id));
-
-                if Some(self_ty) == ret_ty {
-                    if let Some(default_trait_id) = get_trait_def_id(cx, &DEFAULT_TRAIT_PATH) {
-                        if !implements_trait(cx, self_ty, default_trait_id, Vec::new()) {
-                            span_lint(cx, NEW_WITHOUT_DEFAULT, span,
-                                      &format!("you should consider adding a `Default` implementation for `{}`", self_ty));
-                        }
-                    }
-                }
+                if_let_chain!{[
+                    let Some(ret_ty) = return_ty(cx.tcx.node_id_to_type(id)),
+                    same_tys(cx, self_ty, ret_ty),
+                    let Some(default_trait_id) = get_trait_def_id(cx, &DEFAULT_TRAIT_PATH),
+                    !implements_trait(cx, self_ty, default_trait_id, Vec::new())
+                ], {
+                    span_lint(cx, NEW_WITHOUT_DEFAULT, span,
+                              &format!("you should consider adding a `Default` implementation for `{}`", self_ty));
+                }}
             }
         }
     }

src/utils/mod.rs

@@ -740,3 +740,11 @@ pub fn return_ty(fun: ty::Ty) -> Option<ty::Ty> {
         None
     }
 }
+
+/// Check if two types are the same.
+// FIXME: this works correctly for lifetimes bounds (`for <'a> Foo<'a>` == `for <'b> Foo<'b>` but
+// not for type parameters.
+pub fn same_tys<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, a: ty::Ty<'tcx>, b: ty::Ty<'tcx>) -> bool {
+    let infcx = infer::new_infer_ctxt(cx.tcx, &cx.tcx.tables, None);
+    infcx.can_equate(&cx.tcx.erase_regions(&a), &cx.tcx.erase_regions(&b)).is_ok()
+}

tests/compile-fail/methods.rs

@@ -28,6 +28,25 @@ impl T {
     //~| ERROR methods called `new` usually return `Self`
 }
 
+struct Lt<'a> {
+    foo: &'a u32,
+}
+
+impl<'a> Lt<'a> {
+    // The lifetime is different, but that’s irrelevant, see #734
+    #[allow(needless_lifetimes)]
+    pub fn new<'b>(s: &'b str) -> Lt<'b> { unimplemented!() }
+}
+
+struct Lt2<'a> {
+    foo: &'a u32,
+}
+
+impl<'a> Lt2<'a> {
+    // The lifetime is different, but that’s irrelevant, see #734
+    pub fn new(s: &str) -> Lt2 { unimplemented!() }
+}
+
 #[derive(Clone,Copy)]
 struct U;
 

tests/compile-fail/new_without_default.rs

@@ -32,4 +32,13 @@ impl Params {
     fn new(_: u32) -> Self { Params }
 }
 
+struct Generics<'a, T> {
+    foo: &'a bool,
+    bar: T,
+}
+
+impl<'c, V> Generics<'c, V> {
+    fn new<'b>() -> Generics<'b, V> { unimplemented!() } //~ERROR: you should consider adding a `Default` implementation for
+}
+
 fn main() {}