From 00a3bfb8eba20f3d3d5b9623e8737616dfdd4f1c Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Wed, 21 Dec 2016 15:42:20 +0100 Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20warn=20for=20types=20used=20in?= =?UTF-8?q?=20trait=20implementation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- clippy_lints/src/types.rs | 78 ++++++++++++++++++++++++++++++++++--- tests/compile-fail/dlist.rs | 29 +++++++++++++- 2 files changed, 100 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 89bc46074fd..5885d7863b7 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -70,11 +70,47 @@ impl LintPass for TypePass { } impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TypePass { - fn check_ty(&mut self, cx: &LateContext<'a, 'tcx>, ast_ty: &'tcx Ty) { - if in_macro(cx, ast_ty.span) { - return; + fn check_fn(&mut self, cx: &LateContext, _: FnKind, decl: &FnDecl, _: &Expr, _: Span, id: NodeId) { + // skip trait implementations, see #605 + if let Some(map::NodeItem(item)) = cx.tcx.map.find(cx.tcx.map.get_parent(id)) { + if let ItemImpl(_, _, _, Some(..), _, _) = item.node { + return; + } } - if let TyPath(ref qpath) = ast_ty.node { + + check_fn_decl(cx, decl); + } + + fn check_struct_field(&mut self, cx: &LateContext, field: &StructField) { + check_ty(cx, &field.ty); + } + + fn check_trait_item(&mut self, cx: &LateContext, item: &TraitItem) { + match item.node { + ConstTraitItem(ref ty, _) | + TypeTraitItem(_, Some(ref ty)) => check_ty(cx, ty), + MethodTraitItem(ref sig, _) => check_fn_decl(cx, &sig.decl), + _ => (), + } + } +} + +fn check_fn_decl(cx: &LateContext, decl: &FnDecl) { + for input in &decl.inputs { + check_ty(cx, &input.ty); + } + + if let FunctionRetTy::Return(ref ty) = decl.output { + check_ty(cx, ty); + } +} + +fn check_ty(cx: &LateContext, ast_ty: &Ty) { + if in_macro(cx, ast_ty.span) { + return; + } + match ast_ty.node { + TyPath(ref qpath) => { let def = cx.tcx.tables().qpath_def(qpath, ast_ty.id); if let Some(def_id) = opt_def_id(def) { if Some(def_id) == cx.tcx.lang_items.owned_box() { @@ -92,6 +128,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TypePass { ast_ty.span, "you seem to be trying to use `Box>`. Consider using just `Vec`", "`Vec` is already on the heap, `Box>` makes an extra allocation."); + return; // don't recurse into the type }} } else if match_def_path(cx, def_id, &paths::LINKED_LIST) { span_help_and_lint(cx, @@ -99,9 +136,40 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TypePass { ast_ty.span, "I see you're using a LinkedList! Perhaps you meant some other data structure?", "a VecDeque might work"); + return; // don't recurse into the type } } - } + match *qpath { + QPath::Resolved(Some(ref ty), ref p) => { + check_ty(cx, ty); + for ty in p.segments.iter().flat_map(|seg| seg.parameters.types()) { + check_ty(cx, ty); + } + }, + QPath::Resolved(None, ref p) => { + for ty in p.segments.iter().flat_map(|seg| seg.parameters.types()) { + check_ty(cx, ty); + } + }, + QPath::TypeRelative(ref ty, ref seg) => { + check_ty(cx, ty); + for ty in seg.parameters.types() { + check_ty(cx, ty); + } + }, + } + }, + // recurse + TySlice(ref ty) | + TyArray(ref ty, _) | + TyPtr(MutTy { ref ty, .. }) | + TyRptr(_, MutTy { ref ty, .. }) => check_ty(cx, ty), + TyTup(ref tys) => { + for ty in tys { + check_ty(cx, ty); + } + }, + _ => {}, } } diff --git a/tests/compile-fail/dlist.rs b/tests/compile-fail/dlist.rs index 525c166e62a..63c678eb69c 100644 --- a/tests/compile-fail/dlist.rs +++ b/tests/compile-fail/dlist.rs @@ -1,14 +1,39 @@ #![feature(plugin, collections)] +#![feature(associated_type_defaults)] +#![feature(associated_consts)] #![plugin(clippy)] #![deny(clippy)] +#![allow(dead_code)] extern crate collections; use collections::linked_list::LinkedList; -pub fn test(_: LinkedList) { //~ ERROR I see you're using a LinkedList! +trait Foo { + type Baz = LinkedList; //~ ERROR I see you're using a LinkedList! + fn foo(LinkedList); //~ ERROR I see you're using a LinkedList! + const BAR : Option>; //~ ERROR I see you're using a LinkedList! +} + +// ok, we don’t want to warn for implementations, see #605 +impl Foo for LinkedList { + fn foo(_: LinkedList) {} + const BAR : Option> = None; +} + +struct Bar; +impl Bar { + fn foo(_: LinkedList) {} //~ ERROR I see you're using a LinkedList! +} + +pub fn test(my_favourite_linked_list: LinkedList) { //~ ERROR I see you're using a LinkedList! + println!("{:?}", my_favourite_linked_list) +} + +pub fn test_ret() -> Option> { //~ ERROR I see you're using a LinkedList! + unimplemented!(); } fn main(){ - test(LinkedList::new()); //~ ERROR I see you're using a LinkedList! + test(LinkedList::new()); }