Skip to content

Don’t warn for types used in trait implementation #615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 58 additions & 22 deletions src/types.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
use reexport::*;
use rustc::front::map::Node::NodeItem;
use rustc::lint::*;
use rustc::middle::ty;
use rustc_front::hir::*;
use reexport::*;
use rustc_front::util::{is_comparison_binop, binop_to_string};
use syntax::codemap::Span;
use rustc_front::intravisit::{FnKind, Visitor, walk_ty};
use rustc::middle::ty;
use rustc_front::util::{is_comparison_binop, binop_to_string};
use syntax::ast::FloatTy::*;
use syntax::ast::IntTy::*;
use syntax::ast::UintTy::*;
use syntax::ast::FloatTy::*;
use syntax::codemap::Span;

use utils::*;

Expand Down Expand Up @@ -47,27 +48,63 @@ impl LintPass for TypePass {
}

impl LateLintPass for TypePass {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we instead check the parent here using the node map?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that but while cx.tcx.map.get_parent(ast_ty.id) does not return DUMMY_NODE_ID, cx.tcx.map.find(cx.tcx.map.get_parent(ast_ty.id)) is always None here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want get_parent_node here; might need more than one traversal

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, it returns the same ID here.

fn check_ty(&mut self, cx: &LateContext, ast_ty: &Ty) {
if in_macro(cx, ast_ty.span) {
return;
fn check_fn(&mut self, cx: &LateContext, _: FnKind, decl: &FnDecl, _: &Block, _: Span, id: NodeId) {
// skip trait implementations, see #605
if let Some(NodeItem(ref item)) = cx.tcx.map.find(cx.tcx.map.get_parent(id)) {
if let ItemImpl(_, _, _, Some(..), _, _) = item.node {
return;
}
}
if let Some(ty) = cx.tcx.ast_ty_to_ty_cache.borrow().get(&ast_ty.id) {
if let ty::TyBox(ref inner) = ty.sty {
if match_type(cx, inner, &VEC_PATH) {
span_help_and_lint(cx,
BOX_VEC,
ast_ty.span,
"you seem to be trying to use `Box<Vec<T>>`. Consider using just `Vec<T>`",
"`Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation.");
}
} else if match_type(cx, ty, &LL_PATH) {

check_fn_decl(cx, decl);
}

fn check_struct_field(&mut self, cx: &LateContext, field: &StructField) {
check_ty(cx, &field.node.ty);
}

fn check_trait_item(&mut self, cx: &LateContext, item: &TraitItem) {
match item.node {
ConstTraitItem(ref ty, _) => check_ty(cx, ty),
MethodTraitItem(ref sig, _) => check_fn_decl(cx, &sig.decl),
TypeTraitItem(_, Some(ref ty)) => check_ty(cx, ty),
_ => (),
}
}
}

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;
}

if let Some(ty) = cx.tcx.ast_ty_to_ty_cache.borrow().get(&ast_ty.id) {
if let ty::TyBox(ref inner) = ty.sty {
if match_type(cx, inner, &VEC_PATH) {
span_help_and_lint(cx,
LINKEDLIST,
BOX_VEC,
ast_ty.span,
"I see you're using a LinkedList! Perhaps you meant some other data structure?",
"a VecDeque might work");
"you seem to be trying to use `Box<Vec<T>>`. Consider using just `Vec<T>`",
"`Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation.");
}
}
if ty.walk().any(|ty| match_type(cx, ty, &LL_PATH)) {
span_help_and_lint(cx,
LINKEDLIST,
ast_ty.span,
"I see you're using a LinkedList! Perhaps you meant some other data structure?",
"a VecDeque might work");
}
}
}

Expand Down Expand Up @@ -471,7 +508,6 @@ fn check_type(cx: &LateContext, ty: &Ty) {
visitor.visit_ty(ty);
visitor.score
};
// println!("{:?} --> {}", ty, score);
if score > 250 {
span_lint(cx,
TYPE_COMPLEXITY,
Expand Down
26 changes: 25 additions & 1 deletion tests/compile-fail/dlist.rs
Original file line number Diff line number Diff line change
@@ -1,15 +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(foo: LinkedList<u8>) { //~ ERROR I see you're using a LinkedList!
trait Foo {
type Baz = LinkedList<u8>; //~ ERROR I see you're using a LinkedList!
fn foo(LinkedList<u8>); //~ ERROR I see you're using a LinkedList!
const BAR : Option<LinkedList<u8>>; //~ ERROR I see you're using a LinkedList!
}

// ok, we don’t want to warn for implementations, see #605
impl Foo for LinkedList<u8> {
fn foo(_: LinkedList<u8>) {}
const BAR : Option<LinkedList<u8>> = None;
}

struct Bar;
impl Bar {
fn foo(_: LinkedList<u8>) {} //~ ERROR I see you're using a LinkedList!
}

pub fn test(foo: LinkedList<u8>) { //~ ERROR I see you're using a LinkedList!
println!("{:?}", foo)
}

pub fn test_ret() -> Option<LinkedList<u8>> { //~ ERROR I see you're using a LinkedList!
unimplemented!();
}

fn main(){
test(LinkedList::new());
}