diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 7a3604917..fbbbf497d 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -6,8 +6,7 @@ use rustc::hir::*; use semver::Version; use syntax::ast::{Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem, NestedMetaItemKind}; use syntax::codemap::Span; -use utils::{in_macro, match_path, span_lint, span_lint_and_then, snippet_opt}; -use utils::paths; +use utils::{in_macro, match_def_path, resolve_node, paths, span_lint, span_lint_and_then, snippet_opt}; /// **What it does:** Checks for items annotated with `#[inline(always)]`, /// unless the annotated function is empty or simply panics. @@ -101,7 +100,7 @@ impl LateLintPass for AttrPass { } fn check_item(&mut self, cx: &LateContext, item: &Item) { - if is_relevant_item(item) { + if is_relevant_item(cx, item) { check_attrs(cx, item.span, &item.name, &item.attrs) } match item.node { @@ -140,62 +139,63 @@ impl LateLintPass for AttrPass { } fn check_impl_item(&mut self, cx: &LateContext, item: &ImplItem) { - if is_relevant_impl(item) { + if is_relevant_impl(cx, item) { check_attrs(cx, item.span, &item.name, &item.attrs) } } fn check_trait_item(&mut self, cx: &LateContext, item: &TraitItem) { - if is_relevant_trait(item) { + if is_relevant_trait(cx, item) { check_attrs(cx, item.span, &item.name, &item.attrs) } } } -fn is_relevant_item(item: &Item) -> bool { +fn is_relevant_item(cx: &LateContext, item: &Item) -> bool { if let ItemFn(_, _, _, _, _, ref block) = item.node { - is_relevant_block(block) + is_relevant_block(cx, block) } else { false } } -fn is_relevant_impl(item: &ImplItem) -> bool { +fn is_relevant_impl(cx: &LateContext, item: &ImplItem) -> bool { match item.node { - ImplItemKind::Method(_, ref block) => is_relevant_block(block), + ImplItemKind::Method(_, ref block) => is_relevant_block(cx, block), _ => false, } } -fn is_relevant_trait(item: &TraitItem) -> bool { +fn is_relevant_trait(cx: &LateContext, item: &TraitItem) -> bool { match item.node { MethodTraitItem(_, None) => true, - MethodTraitItem(_, Some(ref block)) => is_relevant_block(block), + MethodTraitItem(_, Some(ref block)) => is_relevant_block(cx, block), _ => false, } } -fn is_relevant_block(block: &Block) -> bool { +fn is_relevant_block(cx: &LateContext, block: &Block) -> bool { for stmt in &block.stmts { match stmt.node { StmtDecl(_, _) => return true, StmtExpr(ref expr, _) | StmtSemi(ref expr, _) => { - return is_relevant_expr(expr); + return is_relevant_expr(cx, expr); } } } - block.expr.as_ref().map_or(false, |e| is_relevant_expr(e)) + block.expr.as_ref().map_or(false, |e| is_relevant_expr(cx, e)) } -fn is_relevant_expr(expr: &Expr) -> bool { +fn is_relevant_expr(cx: &LateContext, expr: &Expr) -> bool { match expr.node { - ExprBlock(ref block) => is_relevant_block(block), - ExprRet(Some(ref e)) => is_relevant_expr(e), + ExprBlock(ref block) => is_relevant_block(cx, block), + ExprRet(Some(ref e)) => is_relevant_expr(cx, e), ExprRet(None) | ExprBreak(_) => false, ExprCall(ref path_expr, _) => { - if let ExprPath(_, ref path) = path_expr.node { - !match_path(path, &paths::BEGIN_PANIC) + if let ExprPath(..) = path_expr.node { + let fun_id = resolve_node(cx, path_expr.id).expect("function should be resolved").def_id(); + !match_def_path(cx, fun_id, &paths::BEGIN_PANIC) } else { true } diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index 0cae23dd6..6ed2d8190 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -4,7 +4,7 @@ use rustc::lint::*; use rustc::ty::TypeVariants; use syntax::ast::LitKind; use utils::paths; -use utils::{is_expn_of, match_path, match_type, span_lint, walk_ptrs_ty}; +use utils::{is_expn_of, match_def_path, match_path, match_type, resolve_node, span_lint, walk_ptrs_ty}; /// **What it does:** Checks for the use of `format!("string literal with no /// argument")` and `format!("{}", foo)` where `foo` is a string. @@ -44,9 +44,10 @@ impl LateLintPass for Pass { // `format!("{}", foo)` expansion ExprCall(ref fun, ref args) => { if_let_chain!{[ - let ExprPath(_, ref path) = fun.node, + let ExprPath(..) = fun.node, args.len() == 2, - match_path(path, &paths::FMT_ARGUMENTS_NEWV1), + let Some(fun) = resolve_node(cx, fun.id), + match_def_path(cx, fun.def_id(), &paths::FMT_ARGUMENTS_NEWV1), // ensure the format string is `"{..}"` with only one argument and no text check_static_str(cx, &args[0]), // ensure the format argument is `{}` ie. Display with no fancy option @@ -127,8 +128,9 @@ fn check_arg_is_display(cx: &LateContext, expr: &Expr) -> bool { exprs.len() == 1, let ExprCall(_, ref args) = exprs[0].node, args.len() == 2, - let ExprPath(None, ref path) = args[1].node, - match_path(path, &paths::DISPLAY_FMT_METHOD) + let ExprPath(None, _) = args[1].node, + let Some(fun) = resolve_node(cx, args[1].id), + match_def_path(cx, fun.def_id(), &paths::DISPLAY_FMT_METHOD), ], { let ty = walk_ptrs_ty(cx.tcx.pat_ty(&pat[0])); diff --git a/clippy_lints/src/panic.rs b/clippy_lints/src/panic.rs index d4b7c0fb7..14fe343d5 100644 --- a/clippy_lints/src/panic.rs +++ b/clippy_lints/src/panic.rs @@ -1,7 +1,7 @@ use rustc::hir::*; use rustc::lint::*; use syntax::ast::LitKind; -use utils::{is_direct_expn_of, match_path, paths, span_lint}; +use utils::{is_direct_expn_of, match_def_path, resolve_node, paths, span_lint}; /// **What it does:** Checks for missing parameters in `panic!`. /// @@ -39,8 +39,9 @@ impl LateLintPass for Pass { let Some(ref ex) = block.expr, let ExprCall(ref fun, ref params) = ex.node, params.len() == 2, - let ExprPath(None, ref path) = fun.node, - match_path(path, &paths::BEGIN_PANIC), + let ExprPath(None, _) = fun.node, + let Some(fun) = resolve_node(cx, fun.id), + match_def_path(cx, fun.def_id(), &paths::BEGIN_PANIC), let ExprLit(ref lit) = params[0].node, is_direct_expn_of(cx, params[0].span, "panic").is_some(), let LitKind::Str(ref string, _) = lit.node, diff --git a/clippy_lints/src/print.rs b/clippy_lints/src/print.rs index da41ca849..46d38a67a 100644 --- a/clippy_lints/src/print.rs +++ b/clippy_lints/src/print.rs @@ -2,7 +2,7 @@ use rustc::hir::*; use rustc::hir::map::Node::{NodeItem, NodeImplItem}; use rustc::lint::*; use utils::paths; -use utils::{is_expn_of, match_path, span_lint}; +use utils::{is_expn_of, match_path, match_def_path, resolve_node, span_lint}; use format::get_argument_fmtstr_parts; /// **What it does:** This lint warns when you using `print!()` with a format string that @@ -67,63 +67,69 @@ impl LintPass for Pass { impl LateLintPass for Pass { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { - if let ExprCall(ref fun, ref args) = expr.node { - if let ExprPath(_, ref path) = fun.node { - // Search for `std::io::_print(..)` which is unique in a - // `print!` expansion. - if match_path(path, &paths::IO_PRINT) { - if let Some(span) = is_expn_of(cx, expr.span, "print") { - // `println!` uses `print!`. - let (span, name) = match is_expn_of(cx, span, "println") { - Some(span) => (span, "println"), - None => (span, "print"), - }; + if_let_chain! {[ + let ExprCall(ref fun, ref args) = expr.node, + let ExprPath(..) = fun.node, + let Some(fun) = resolve_node(cx, fun.id), + ], { + let fun_id = fun.def_id(); - span_lint(cx, PRINT_STDOUT, span, &format!("use of `{}!`", name)); + // Search for `std::io::_print(..)` which is unique in a + // `print!` expansion. + if match_def_path(cx, fun_id, &paths::IO_PRINT) { + if let Some(span) = is_expn_of(cx, expr.span, "print") { + // `println!` uses `print!`. + let (span, name) = match is_expn_of(cx, span, "println") { + Some(span) => (span, "println"), + None => (span, "print"), + }; - // Check print! with format string ending in "\n". - if_let_chain!{[ - name == "print", + span_lint(cx, PRINT_STDOUT, span, &format!("use of `{}!`", name)); - // ensure we're calling Arguments::new_v1 - args.len() == 1, - let ExprCall(ref args_fun, ref args_args) = args[0].node, - let ExprPath(_, ref args_path) = args_fun.node, - match_path(args_path, &paths::FMT_ARGUMENTS_NEWV1), - args_args.len() == 2, - let ExprAddrOf(_, ref match_expr) = args_args[1].node, - let ExprMatch(ref args, _, _) = match_expr.node, - let ExprTup(ref args) = args.node, + // Check print! with format string ending in "\n". + if_let_chain!{[ + name == "print", - // collect the format string parts and check the last one - let Some(fmtstrs) = get_argument_fmtstr_parts(cx, &args_args[0]), - let Some(last_str) = fmtstrs.last(), - let Some('\n') = last_str.chars().last(), + // ensure we're calling Arguments::new_v1 + args.len() == 1, + let ExprCall(ref args_fun, ref args_args) = args[0].node, + let ExprPath(..) = args_fun.node, + let Some(def) = resolve_node(cx, args_fun.id), + match_def_path(cx, def.def_id(), &paths::FMT_ARGUMENTS_NEWV1), + args_args.len() == 2, + let ExprAddrOf(_, ref match_expr) = args_args[1].node, + let ExprMatch(ref args, _, _) = match_expr.node, + let ExprTup(ref args) = args.node, - // "foo{}bar" is made into two strings + one argument, - // if the format string starts with `{}` (eg. "{}foo"), - // the string array is prepended an empty string "". - // We only want to check the last string after any `{}`: - args.len() < fmtstrs.len(), - ], { - span_lint(cx, PRINT_WITH_NEWLINE, span, - "using `print!()` with a format string that ends in a \ - newline, consider using `println!()` instead"); - }} - } + // collect the format string parts and check the last one + let Some(fmtstrs) = get_argument_fmtstr_parts(cx, &args_args[0]), + let Some(last_str) = fmtstrs.last(), + let Some('\n') = last_str.chars().last(), + + // "foo{}bar" is made into two strings + one argument, + // if the format string starts with `{}` (eg. "{}foo"), + // the string array is prepended an empty string "". + // We only want to check the last string after any `{}`: + args.len() < fmtstrs.len(), + ], { + span_lint(cx, PRINT_WITH_NEWLINE, span, + "using `print!()` with a format string that ends in a \ + newline, consider using `println!()` instead"); + }} } - // Search for something like - // `::std::fmt::ArgumentV1::new(__arg0, ::std::fmt::Debug::fmt)` - else if args.len() == 2 && match_path(path, &paths::FMT_ARGUMENTV1_NEW) { - if let ExprPath(None, ref path) = args[1].node { - if match_path(path, &paths::DEBUG_FMT_METHOD) && !is_in_debug_impl(cx, expr) && - is_expn_of(cx, expr.span, "panic").is_none() { - span_lint(cx, USE_DEBUG, args[0].span, "use of `Debug`-based formatting"); - } + } + // Search for something like + // `::std::fmt::ArgumentV1::new(__arg0, ::std::fmt::Debug::fmt)` + else if args.len() == 2 && match_def_path(cx, fun_id, &paths::FMT_ARGUMENTV1_NEW) { + if let ExprPath(None, _) = args[1].node { + let def_id = resolve_node(cx, args[1].id).unwrap().def_id(); + if match_def_path(cx, def_id, &paths::DEBUG_FMT_METHOD) && !is_in_debug_impl(cx, expr) && + is_expn_of(cx, expr.span, "panic").is_none() { + span_lint(cx, USE_DEBUG, args[0].span, "use of `Debug`-based formatting"); } } } - } + }} } } diff --git a/clippy_lints/src/utils/higher.rs b/clippy_lints/src/utils/higher.rs index f9c94029b..d49cb7826 100644 --- a/clippy_lints/src/utils/higher.rs +++ b/clippy_lints/src/utils/higher.rs @@ -6,7 +6,7 @@ use rustc::hir; use rustc::lint::LateContext; use syntax::ast; use syntax::ptr::P; -use utils::{is_expn_of, match_path, paths}; +use utils::{is_expn_of, match_path, match_def_path, resolve_node, paths}; /// Convert a hir binary operator to the corresponding `ast` type. pub fn binop(op: hir::BinOp_) -> ast::BinOpKind { @@ -170,9 +170,10 @@ pub fn vec_macro<'e>(cx: &LateContext, expr: &'e hir::Expr) -> Option(cx: &LateContext<'a, 'tcx>, ty: ty::Ty<'tcx>, }) } +/// Resolve the definition of a node from its `NodeId`. +pub fn resolve_node(cx: &LateContext, id: NodeId) -> Option { + cx.tcx.def_map.borrow().get(&id).map(|d| d.full_def()) +} + /// Match an `Expr` against a chain of methods, and return the matched `Expr`s. /// /// For example, if `expr` represents the `.baz()` in `foo.bar().baz()`, diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 2f05ce37d..f182ed161 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -1,6 +1,6 @@ //! This module contains paths to types and functions Clippy needs to know about. -pub const BEGIN_PANIC: [&'static str; 3] = ["std", "rt", "begin_panic"]; +pub const BEGIN_PANIC: [&'static str; 3] = ["std", "panicking", "begin_panic"]; pub const BINARY_HEAP: [&'static str; 3] = ["collections", "binary_heap", "BinaryHeap"]; pub const BOX: [&'static str; 3] = ["std", "boxed", "Box"]; pub const BOX_NEW: [&'static str; 4] = ["std", "boxed", "Box", "new"]; @@ -13,18 +13,18 @@ pub const CMP_MAX: [&'static str; 3] = ["core", "cmp", "max"]; pub const CMP_MIN: [&'static str; 3] = ["core", "cmp", "min"]; pub const COW: [&'static str; 3] = ["collections", "borrow", "Cow"]; pub const CSTRING_NEW: [&'static str; 4] = ["std", "ffi", "CString", "new"]; -pub const DEBUG_FMT_METHOD: [&'static str; 4] = ["std", "fmt", "Debug", "fmt"]; +pub const DEBUG_FMT_METHOD: [&'static str; 4] = ["core", "fmt", "Debug", "fmt"]; pub const DEFAULT_TRAIT: [&'static str; 3] = ["core", "default", "Default"]; -pub const DISPLAY_FMT_METHOD: [&'static str; 4] = ["std", "fmt", "Display", "fmt"]; +pub const DISPLAY_FMT_METHOD: [&'static str; 4] = ["core", "fmt", "Display", "fmt"]; pub const DROP: [&'static str; 3] = ["core", "mem", "drop"]; -pub const FMT_ARGUMENTS_NEWV1: [&'static str; 4] = ["std", "fmt", "Arguments", "new_v1"]; -pub const FMT_ARGUMENTV1_NEW: [&'static str; 4] = ["std", "fmt", "ArgumentV1", "new"]; +pub const FMT_ARGUMENTS_NEWV1: [&'static str; 4] = ["core", "fmt", "Arguments", "new_v1"]; +pub const FMT_ARGUMENTV1_NEW: [&'static str; 4] = ["core", "fmt", "ArgumentV1", "new"]; pub const HASH: [&'static str; 2] = ["hash", "Hash"]; pub const HASHMAP: [&'static str; 5] = ["std", "collections", "hash", "map", "HashMap"]; pub const HASHMAP_ENTRY: [&'static str; 5] = ["std", "collections", "hash", "map", "Entry"]; pub const HASHSET: [&'static str; 5] = ["std", "collections", "hash", "set", "HashSet"]; pub const INTO_ITERATOR: [&'static str; 4] = ["core", "iter", "traits", "IntoIterator"]; -pub const IO_PRINT: [&'static str; 3] = ["std", "io", "_print"]; +pub const IO_PRINT: [&'static str; 4] = ["std", "io", "stdio", "_print"]; pub const ITERATOR: [&'static str; 4] = ["core", "iter", "iterator", "Iterator"]; pub const LINKED_LIST: [&'static str; 3] = ["collections", "linked_list", "LinkedList"]; pub const LINT: [&'static str; 3] = ["rustc", "lint", "Lint"]; @@ -64,4 +64,4 @@ pub const STRING: [&'static str; 3] = ["collections", "string", "String"]; pub const TRANSMUTE: [&'static str; 4] = ["core", "intrinsics", "", "transmute"]; pub const VEC: [&'static str; 3] = ["collections", "vec", "Vec"]; pub const VEC_DEQUE: [&'static str; 3] = ["collections", "vec_deque", "VecDeque"]; -pub const VEC_FROM_ELEM: [&'static str; 3] = ["std", "vec", "from_elem"]; +pub const VEC_FROM_ELEM: [&'static str; 3] = ["collections", "vec", "from_elem"];