From 317e97bae79331cba506da9615850405fa82d937 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Wed, 29 Nov 2017 17:06:27 +0100 Subject: [PATCH] Fix #2196 --- clippy_lints/src/new_without_default.rs | 123 ++++++++++++------------ tests/ui/new_without_default.rs | 6 ++ 2 files changed, 67 insertions(+), 62 deletions(-) diff --git a/clippy_lints/src/new_without_default.rs b/clippy_lints/src/new_without_default.rs index d56833eb4574..b037ef6c42b7 100644 --- a/clippy_lints/src/new_without_default.rs +++ b/clippy_lints/src/new_without_default.rs @@ -1,9 +1,7 @@ -use rustc::hir::intravisit::FnKind; use rustc::hir::def_id::DefId; use rustc::hir; use rustc::lint::*; use rustc::ty::{self, Ty}; -use syntax::ast; use syntax::codemap::Span; use utils::paths; use utils::{get_trait_def_id, implements_trait, in_external_macro, return_ty, same_tys, span_lint_and_then}; @@ -90,66 +88,67 @@ impl LintPass for NewWithoutDefault { } impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault { - fn check_fn( - &mut self, - cx: &LateContext<'a, 'tcx>, - kind: FnKind<'tcx>, - decl: &'tcx hir::FnDecl, - _: &'tcx hir::Body, - span: Span, - id: ast::NodeId, - ) { - if in_external_macro(cx, span) { - return; - } - - if let FnKind::Method(name, sig, _, _) = kind { - if sig.constness == hir::Constness::Const { - // can't be implemented by default - return; - } - if !cx.generics - .expect("method must have generics") - .ty_params - .is_empty() - { - // when the result of `new()` depends on a type parameter we should not require - // an - // impl of `Default` - return; - } - if decl.inputs.is_empty() && name == "new" && cx.access_levels.is_reachable(id) { - let self_ty = cx.tcx - .type_of(cx.tcx.hir.local_def_id(cx.tcx.hir.get_parent(id))); - if_chain! { - if same_tys(cx, self_ty, return_ty(cx, id)); - if let Some(default_trait_id) = get_trait_def_id(cx, &paths::DEFAULT_TRAIT); - if !implements_trait(cx, self_ty, default_trait_id, &[]); - then { - if let Some(sp) = can_derive_default(self_ty, cx, default_trait_id) { - span_lint_and_then( - cx, - NEW_WITHOUT_DEFAULT_DERIVE, - span, - &format!("you should consider deriving a `Default` implementation for `{}`", self_ty), - |db| { - db.suggest_item_with_attr(cx, sp, "try this", "#[derive(Default)]"); - }); - } else { - span_lint_and_then( - cx, - NEW_WITHOUT_DEFAULT, - span, - &format!("you should consider adding a `Default` implementation for `{}`", self_ty), - |db| { - db.suggest_prepend_item( - cx, - span, - "try this", - &create_new_without_default_suggest_msg(self_ty), - ); - }, - ); + fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item) { + if let hir::ItemImpl(_, _, _, _, None, _, ref items) = item.node { + for assoc_item in items { + if let hir::AssociatedItemKind::Method { has_self: false } = assoc_item.kind { + let impl_item = cx.tcx.hir.impl_item(assoc_item.id); + if in_external_macro(cx, impl_item.span) { + return; + } + if let hir::ImplItemKind::Method(ref sig, _) = impl_item.node { + let name = impl_item.name; + let span = impl_item.span; + let id = impl_item.id; + let decl = &sig.decl; + if sig.constness == hir::Constness::Const { + // can't be implemented by default + return; + } + if !impl_item.generics + .ty_params + .is_empty() + { + // when the result of `new()` depends on a type parameter we should not require + // an + // impl of `Default` + return; + } + if decl.inputs.is_empty() && name == "new" && cx.access_levels.is_reachable(id) { + let self_ty = cx.tcx + .type_of(cx.tcx.hir.local_def_id(cx.tcx.hir.get_parent(id))); + if_chain! { + if same_tys(cx, self_ty, return_ty(cx, id)); + if let Some(default_trait_id) = get_trait_def_id(cx, &paths::DEFAULT_TRAIT); + if !implements_trait(cx, self_ty, default_trait_id, &[]); + then { + if let Some(sp) = can_derive_default(self_ty, cx, default_trait_id) { + span_lint_and_then( + cx, + NEW_WITHOUT_DEFAULT_DERIVE, + span, + &format!("you should consider deriving a `Default` implementation for `{}`", self_ty), + |db| { + db.suggest_item_with_attr(cx, sp, "try this", "#[derive(Default)]"); + }); + } else { + span_lint_and_then( + cx, + NEW_WITHOUT_DEFAULT, + span, + &format!("you should consider adding a `Default` implementation for `{}`", self_ty), + |db| { + db.suggest_prepend_item( + cx, + span, + "try this", + &create_new_without_default_suggest_msg(self_ty), + ); + }, + ); + } + } + } } } } diff --git a/tests/ui/new_without_default.rs b/tests/ui/new_without_default.rs index 9fd0fea137c8..e618bf1c231a 100644 --- a/tests/ui/new_without_default.rs +++ b/tests/ui/new_without_default.rs @@ -83,4 +83,10 @@ impl IgnoreGenericNew { pub fn new() -> Self { IgnoreGenericNew } // the derived Default does not make sense here as the result depends on T } +pub trait TraitWithNew: Sized { + fn new() -> Self { + panic!() + } +} + fn main() {}