From 052f5984e772513654be815a1eac08997db6839c Mon Sep 17 00:00:00 2001 From: mcarton Date: Thu, 3 Mar 2016 19:46:10 +0100 Subject: [PATCH] Fix types comparison --- src/methods.rs | 4 ++-- src/new_without_default.rs | 22 +++++++++++----------- src/utils/mod.rs | 8 ++++++++ tests/compile-fail/methods.rs | 19 +++++++++++++++++++ tests/compile-fail/new_without_default.rs | 9 +++++++++ 5 files changed, 49 insertions(+), 13 deletions(-) diff --git a/src/methods.rs b/src/methods.rs index 3ed75fdffeb..6d33f31d45c 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -10,7 +10,7 @@ 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 @@ fn check_item(&mut self, cx: &LateContext, item: &Item) { } 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, diff --git a/src/new_without_default.rs b/src/new_without_default.rs index 89467f1dc55..d341afb4d92 100644 --- a/src/new_without_default.rs +++ b/src/new_without_default.rs @@ -3,7 +3,8 @@ 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 @@ fn check_fn(&mut self, cx: &LateContext, kind: FnKind, decl: &hir::FnDecl, _: &h 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)); + }} } } } diff --git a/src/utils/mod.rs b/src/utils/mod.rs index bef4baea67e..c626fcb8930 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -740,3 +740,11 @@ pub fn return_ty(fun: ty::Ty) -> Option { 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() +} diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs index 0acab8be4fb..46f14d5d921 100644 --- a/tests/compile-fail/methods.rs +++ b/tests/compile-fail/methods.rs @@ -28,6 +28,25 @@ fn new(self) {} //~| 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; diff --git a/tests/compile-fail/new_without_default.rs b/tests/compile-fail/new_without_default.rs index 5f00179a9a2..cc033043bc5 100755 --- a/tests/compile-fail/new_without_default.rs +++ b/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() {}