diff --git a/src/methods.rs b/src/methods.rs index 663c7dd1bc4b..3ed75fdffeb7 100644 --- a/src/methods.rs +++ b/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, returns_self, snippet, snippet_opt, span_lint, + match_type, method_chain_args, return_ty, 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}; @@ -431,7 +431,8 @@ impl LateLintPass for MethodsPass { } } - if &name.as_str() == &"new" && !returns_self(cx, &sig.decl.output, ty) { + 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)) { 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 4666336495c6..89467f1dc556 100644 --- a/src/new_without_default.rs +++ b/src/new_without_default.rs @@ -3,7 +3,7 @@ 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, returns_self, span_lint, DEFAULT_TRAIT_PATH}; +use utils::{get_trait_def_id, implements_trait, in_external_macro, return_ty, span_lint, DEFAULT_TRAIT_PATH}; /// **What it does:** This lints about type with a `fn new() -> Self` method and no `Default` /// implementation. @@ -47,13 +47,15 @@ impl LateLintPass for NewWithoutDefault { if let FnKind::Method(name, _, _) = kind { if decl.inputs.is_empty() && name.as_str() == "new" { - let ty = cx.tcx.lookup_item_type(cx.tcx.map.local_def_id(cx.tcx.map.get_parent(id))).ty; + let self_ty = cx.tcx.lookup_item_type(cx.tcx.map.local_def_id(cx.tcx.map.get_parent(id))).ty; - if returns_self(cx, &decl.output, 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, ty, default_trait_id, Vec::new()) { + 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 `{}`", ty)); + &format!("you should consider adding a `Default` implementation for `{}`", self_ty)); } } } diff --git a/src/ptr_arg.rs b/src/ptr_arg.rs index c02e5609b8c7..85baf3310c3d 100644 --- a/src/ptr_arg.rs +++ b/src/ptr_arg.rs @@ -6,6 +6,7 @@ use rustc::front::map::NodeItem; use rustc::lint::*; use rustc::middle::ty; use rustc_front::hir::*; +use syntax::ast::NodeId; use utils::{STRING_PATH, VEC_PATH}; use utils::{span_lint, match_type}; @@ -35,7 +36,7 @@ impl LintPass for PtrArg { impl LateLintPass for PtrArg { fn check_item(&mut self, cx: &LateContext, item: &Item) { if let ItemFn(ref decl, _, _, _, _, _) = item.node { - check_fn(cx, decl); + check_fn(cx, decl, item.id); } } @@ -46,34 +47,34 @@ impl LateLintPass for PtrArg { return; // ignore trait impls } } - check_fn(cx, &sig.decl); + check_fn(cx, &sig.decl, item.id); } } fn check_trait_item(&mut self, cx: &LateContext, item: &TraitItem) { if let MethodTraitItem(ref sig, _) = item.node { - check_fn(cx, &sig.decl); + check_fn(cx, &sig.decl, item.id); } } } -fn check_fn(cx: &LateContext, decl: &FnDecl) { - for arg in &decl.inputs { - if let Some(ty) = cx.tcx.ast_ty_to_ty_cache.borrow().get(&arg.ty.id) { - if let ty::TyRef(_, ty::TypeAndMut { ty, mutbl: MutImmutable }) = ty.sty { - if match_type(cx, ty, &VEC_PATH) { - span_lint(cx, - PTR_ARG, - arg.ty.span, - "writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used \ - with non-Vec-based slices. Consider changing the type to `&[...]`"); - } else if match_type(cx, ty, &STRING_PATH) { - span_lint(cx, - PTR_ARG, - arg.ty.span, - "writing `&String` instead of `&str` involves a new object where a slice will do. \ - Consider changing the type to `&str`"); - } +fn check_fn(cx: &LateContext, decl: &FnDecl, fn_id: NodeId) { + let fn_ty = cx.tcx.node_id_to_type(fn_id).fn_sig().skip_binder(); + + for (arg, ty) in decl.inputs.iter().zip(&fn_ty.inputs) { + if let ty::TyRef(_, ty::TypeAndMut { ty, mutbl: MutImmutable }) = ty.sty { + if match_type(cx, ty, &VEC_PATH) { + span_lint(cx, + PTR_ARG, + arg.ty.span, + "writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used \ + with non-Vec-based slices. Consider changing the type to `&[...]`"); + } else if match_type(cx, ty, &STRING_PATH) { + span_lint(cx, + PTR_ARG, + arg.ty.span, + "writing `&String` instead of `&str` involves a new object where a slice will do. \ + Consider changing the type to `&str`"); } } } diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 46feafb1de40..bef4baea67e1 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -732,18 +732,11 @@ pub fn unsugar_range(expr: &Expr) -> Option { } } -/// Return whether a method returns `Self`. -pub fn returns_self(cx: &LateContext, ret: &FunctionRetTy, ty: ty::Ty) -> bool { - if let FunctionRetTy::Return(ref ret_ty) = *ret { - let ast_ty_to_ty_cache = cx.tcx.ast_ty_to_ty_cache.borrow(); - let ret_ty = ast_ty_to_ty_cache.get(&ret_ty.id); - - if let Some(&ret_ty) = ret_ty { - ret_ty.walk().any(|t| t == ty) - } else { - false - } +/// Convenience function to get the return type of a function or `None` if the function diverges. +pub fn return_ty(fun: ty::Ty) -> Option { + if let ty::FnConverging(ret_ty) = fun.fn_sig().skip_binder().output { + Some(ret_ty) } else { - false + None } }