diff --git a/clippy_lints/src/enum_glob_use.rs b/clippy_lints/src/enum_glob_use.rs index f4f8d6f378b7..7992c4966b2a 100644 --- a/clippy_lints/src/enum_glob_use.rs +++ b/clippy_lints/src/enum_glob_use.rs @@ -48,6 +48,8 @@ impl EnumGlobUse { return; // re-exports are fine } if let ItemUse(ref path, UseKind::Glob) = item.node { + // FIXME: ask jseyfried why the qpath.def for `use std::cmp::Ordering::*;` + // extracted through `ItemUse(ref qpath, UseKind::Glob)` is a `Mod` and not an `Enum` if let Def::Enum(_) = path.def { span_lint(cx, ENUM_GLOB_USE, item.span, "don't use glob imports for enum variants"); } diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index 236919bfc253..6d100317fd88 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/clippy_lints/src/lifetimes.rs @@ -5,7 +5,7 @@ use rustc::hir::*; use rustc::hir::intravisit::{Visitor, walk_ty, walk_ty_param_bound, walk_fn_decl, walk_generics}; use std::collections::{HashSet, HashMap}; use syntax::codemap::Span; -use utils::{in_external_macro, span_lint}; +use utils::{in_external_macro, span_lint, last_path_segment}; /// **What it does:** Checks for lifetime annotations which can be removed by /// relying on lifetime elision. @@ -240,26 +240,24 @@ impl<'v, 't> RefVisitor<'v, 't> { } fn collect_anonymous_lifetimes(&mut self, qpath: &QPath, ty: &Ty) { - if let QPath::Resolved(_, ref path) = *qpath { - let last_path_segment = path.segments.last().map(|s| &s.parameters); - if let Some(&AngleBracketedParameters(ref params)) = last_path_segment { - if params.lifetimes.is_empty() { - match self.cx.tcx.tables().qpath_def(qpath, ty.id) { - Def::TyAlias(def_id) | - Def::Struct(def_id) => { - let generics = self.cx.tcx.item_generics(def_id); - for _ in generics.regions.as_slice() { - self.record(&None); - } + let last_path_segment = &last_path_segment(qpath).parameters; + if let &AngleBracketedParameters(ref params) = last_path_segment { + if params.lifetimes.is_empty() { + match self.cx.tcx.tables().qpath_def(qpath, ty.id) { + Def::TyAlias(def_id) | + Def::Struct(def_id) => { + let generics = self.cx.tcx.item_generics(def_id); + for _ in generics.regions.as_slice() { + self.record(&None); } - Def::Trait(def_id) => { - let trait_def = self.cx.tcx.trait_defs.borrow()[&def_id]; - for _ in &trait_def.generics.regions { - self.record(&None); - } - } - _ => (), } + Def::Trait(def_id) => { + let trait_def = self.cx.tcx.trait_defs.borrow()[&def_id]; + for _ in &trait_def.generics.regions { + self.record(&None); + } + } + _ => (), } } } diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index d752e78cddf8..0d862b76d9bb 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -16,7 +16,7 @@ use utils::sugg; use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, multispan_sugg, in_external_macro, is_refutable, span_help_and_lint, is_integer_literal, - get_enclosing_block, span_lint_and_then, higher, walk_ptrs_ty}; + get_enclosing_block, span_lint_and_then, higher, walk_ptrs_ty, last_path_segment}; use utils::paths; /// **What it does:** Checks for looping over the range of `0..len` of some @@ -369,26 +369,23 @@ impl LateLintPass for Pass { if let (&PatKind::TupleStruct(ref qpath, ref pat_args, _), &ExprMethodCall(method_name, _, ref method_args)) = (pat, &match_expr.node) { let iter_expr = &method_args[0]; - if let QPath::Resolved(_, ref path) = *qpath { - if let Some(lhs_constructor) = path.segments.last() { - if &*method_name.node.as_str() == "next" && - match_trait_method(cx, match_expr, &paths::ITERATOR) && - &*lhs_constructor.name.as_str() == "Some" && - !is_refutable(cx, &pat_args[0]) && - !is_iterator_used_after_while_let(cx, iter_expr) { - let iterator = snippet(cx, method_args[0].span, "_"); - let loop_var = snippet(cx, pat_args[0].span, "_"); - span_lint_and_then(cx, - WHILE_LET_ON_ITERATOR, - expr.span, - "this loop could be written as a `for` loop", - |db| { - db.span_suggestion(expr.span, - "try", - format!("for {} in {} {{ .. }}", loop_var, iterator)); - }); - } - } + let lhs_constructor = last_path_segment(qpath); + if &*method_name.node.as_str() == "next" && + match_trait_method(cx, match_expr, &paths::ITERATOR) && + &*lhs_constructor.name.as_str() == "Some" && + !is_refutable(cx, &pat_args[0]) && + !is_iterator_used_after_while_let(cx, iter_expr) { + let iterator = snippet(cx, method_args[0].span, "_"); + let loop_var = snippet(cx, pat_args[0].span, "_"); + span_lint_and_then(cx, + WHILE_LET_ON_ITERATOR, + expr.span, + "this loop could be written as a `for` loop", + |db| { + db.span_suggestion(expr.span, + "try", + format!("for {} in {} {{ .. }}", loop_var, iterator)); + }); } } } diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs index 1a824e467c49..edcdc74013b0 100644 --- a/clippy_lints/src/methods.rs +++ b/clippy_lints/src/methods.rs @@ -3,6 +3,7 @@ use rustc::lint::*; use rustc::middle::const_val::ConstVal; use rustc::middle::const_qualif::ConstQualif; use rustc::ty; +use rustc::hir::def::Def; use rustc_const_eval::EvalHint::ExprTypeChecked; use rustc_const_eval::eval_const_expr_partial; use std::borrow::Cow; @@ -10,7 +11,8 @@ use std::fmt; use syntax::codemap::Span; use utils::{get_trait_def_id, implements_trait, in_external_macro, in_macro, is_copy, match_path, match_trait_method, match_type, method_chain_args, return_ty, same_tys, snippet, - span_lint, span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth}; + span_lint, span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth, + last_path_segment, single_segment_path, match_def_path}; use utils::paths; use utils::sugg; @@ -701,12 +703,8 @@ fn lint_or_fun_call(cx: &LateContext, expr: &hir::Expr, name: &str, args: &[hir: } if name == "unwrap_or" { - if let hir::ExprPath(hir::QPath::Resolved(_, ref path)) = fun.node { - let path: &str = &path.segments - .last() - .expect("A path must have at least one segment") - .name - .as_str(); + if let hir::ExprPath(ref qpath) = fun.node { + let path: &str = &*last_path_segment(qpath).name.as_str(); if ["default", "new"].contains(&path) { let arg_ty = cx.tcx.tables().expr_ty(arg); @@ -878,7 +876,8 @@ fn lint_cstring_as_ptr(cx: &LateContext, expr: &hir::Expr, new: &hir::Expr, unwr let hir::ExprCall(ref fun, ref args) = new.node, args.len() == 1, let hir::ExprPath(ref path) = fun.node, - match_path(path, &paths::CSTRING_NEW), + let Def::Method(did) = cx.tcx.tables().qpath_def(path, fun.id), + match_def_path(cx, did, &paths::CSTRING_NEW) ], { span_lint_and_then(cx, TEMPORARY_CSTRING_AS_PTR, expr.span, "you are getting the inner pointer of a temporary `CString`", @@ -1188,8 +1187,9 @@ fn lint_chars_next(cx: &LateContext, expr: &hir::Expr, chain: &hir::Expr, other: let Some(args) = method_chain_args(chain, &["chars", "next"]), let hir::ExprCall(ref fun, ref arg_char) = other.node, arg_char.len() == 1, - let hir::ExprPath(hir::QPath::Resolved(None, ref path)) = fun.node, - path.segments.len() == 1 && &*path.segments[0].name.as_str() == "Some" + let hir::ExprPath(ref qpath) = fun.node, + let Some(segment) = single_segment_path(qpath), + &*segment.name.as_str() == "Some" ], { let self_ty = walk_ptrs_ty(cx.tcx.tables().expr_ty_adjusted(&args[0][0])); diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 854a8d144d68..5752e95d0acc 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -10,7 +10,7 @@ use rustc_const_math::ConstFloat; use syntax::codemap::{Span, Spanned, ExpnFormat}; use utils::{ get_item_name, get_parent_expr, implements_trait, in_macro, is_integer_literal, match_path, - snippet, span_lint, span_lint_and_then, walk_ptrs_ty + snippet, span_lint, span_lint_and_then, walk_ptrs_ty, last_path_segment }; use utils::sugg::Sugg; @@ -263,22 +263,14 @@ impl LateLintPass for Pass { } let binding = match expr.node { ExprPath(ref qpath) => { - if let QPath::Resolved(_, ref path) = *qpath { - let binding = path.segments - .last() - .expect("path should always have at least one segment") - .name - .as_str(); - if binding.starts_with('_') && - !binding.starts_with("__") && - &*binding != "_result" && // FIXME: #944 - is_used(cx, expr) && - // don't lint if the declaration is in a macro - non_macro_local(cx, &cx.tcx.tables().qpath_def(qpath, expr.id)) { - Some(binding) - } else { - None - } + let binding = last_path_segment(qpath).name.as_str(); + if binding.starts_with('_') && + !binding.starts_with("__") && + &*binding != "_result" && // FIXME: #944 + is_used(cx, expr) && + // don't lint if the declaration is in a macro + non_macro_local(cx, &cx.tcx.tables().qpath_def(qpath, expr.id)) { + Some(binding) } else { None } diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index 791843ee567b..b3d03f5dc18f 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -75,17 +75,13 @@ impl LateLintPass for StepByZero { // .iter() and .len() called on same Path let ExprPath(QPath::Resolved(_, ref iter_path)) = iter_args[0].node, let ExprPath(QPath::Resolved(_, ref len_path)) = len_args[0].node, - iter_path == len_path + iter_path.segments == len_path.segments ], { - let Path { segments: ref iter_path, .. } = **iter_path; - let Path { segments: ref len_path, .. } = **len_path; - if iter_path == len_path { - span_lint(cx, - RANGE_ZIP_WITH_LEN, - expr.span, - &format!("It is more idiomatic to use {}.iter().enumerate()", - snippet(cx, iter_args[0].span, "_"))); - } + span_lint(cx, + RANGE_ZIP_WITH_LEN, + expr.span, + &format!("It is more idiomatic to use {}.iter().enumerate()", + snippet(cx, iter_args[0].span, "_"))); }} } } diff --git a/clippy_lints/src/transmute.rs b/clippy_lints/src/transmute.rs index 3a8680b1cf77..2263390840c1 100644 --- a/clippy_lints/src/transmute.rs +++ b/clippy_lints/src/transmute.rs @@ -2,7 +2,7 @@ use rustc::lint::*; use rustc::ty::TypeVariants::{TyRawPtr, TyRef}; use rustc::ty; use rustc::hir::*; -use utils::{match_def_path, paths, span_lint, span_lint_and_then, snippet}; +use utils::{match_def_path, paths, span_lint, span_lint_and_then, snippet, last_path_segment}; use utils::sugg; /// **What it does:** Checks for transmutes that can't ever be correct on any @@ -191,9 +191,8 @@ impl LateLintPass for Transmute { /// the type's `ToString` implementation. In weird cases it could lead to types with invalid `'_` /// lifetime, but it should be rare. fn get_type_snippet(cx: &LateContext, path: &QPath, to_rty: ty::Ty) -> String { + let seg = last_path_segment(path); if_let_chain!{[ - let QPath::Resolved(_, ref path) = *path, - let Some(seg) = path.segments.last(), let PathParameters::AngleBracketedParameters(ref ang) = seg.parameters, let Some(to_ty) = ang.types.get(1), let TyRptr(_, ref to_ty) = to_ty.node, diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 008a1fbc6ce4..15e5f3f58db7 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -7,7 +7,7 @@ use std::cmp::Ordering; use syntax::ast::{IntTy, UintTy, FloatTy}; use syntax::codemap::Span; use utils::{comparisons, higher, in_external_macro, in_macro, match_def_path, snippet, - span_help_and_lint, span_lint, opt_def_id}; + span_help_and_lint, span_lint, opt_def_id, last_path_segment}; use utils::paths; /// Handles all the linting of funky types @@ -78,9 +78,8 @@ impl LateLintPass for TypePass { let def = cx.tcx.tables().qpath_def(qpath, ast_ty.id); if let Some(def_id) = opt_def_id(def) { if def_id == cx.tcx.lang_items.owned_box().unwrap() { + let last = last_path_segment(qpath); if_let_chain! {[ - let QPath::Resolved(_, ref path) = *qpath, - let Some(ref last) = path.segments.last(), let PathParameters::AngleBracketedParameters(ref ag) = last.parameters, let Some(ref vec) = ag.types.get(0), let TyPath(ref qpath) = vec.node, diff --git a/clippy_lints/src/utils/higher.rs b/clippy_lints/src/utils/higher.rs index a2e8bc891b4c..6ae4743a368c 100644 --- a/clippy_lints/src/utils/higher.rs +++ b/clippy_lints/src/utils/higher.rs @@ -163,7 +163,7 @@ pub fn vec_macro<'e>(cx: &LateContext, expr: &'e hir::Expr) -> Option SpanlessHash<'a, 'tcx> { ExprPath(ref qpath) => { let c: fn(_) -> _ = ExprPath; c.hash(&mut self.s); - self.hash_qpath(qpath); + self.hash_qpath(qpath, e.id); } ExprStruct(ref path, ref fields, ref expr) => { let c: fn(_, _, _) -> _ = ExprStruct; c.hash(&mut self.s); - self.hash_qpath(path); + self.hash_qpath(path, e.id); for f in fields { self.hash_name(&f.name.node); @@ -528,21 +528,8 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> { n.as_str().hash(&mut self.s); } - pub fn hash_qpath(&mut self, p: &QPath) { - match *p { - QPath::Resolved(ref _ty, ref path) => { - let c: fn(_, _) -> _ = QPath::Resolved; - c.hash(&mut self.s); - // self.hash_ty(ty); FIXME - self.hash_path(path); - }, - QPath::TypeRelative(ref _ty, ref seg) => { - let c: fn(_, _) -> _ = QPath::TypeRelative; - c.hash(&mut self.s); - // self.hash_ty(ty); FIXME - self.hash_name(&seg.name); - }, - } + pub fn hash_qpath(&mut self, p: &QPath, id: NodeId) { + self.cx.tcx.tables().qpath_def(p, id).hash(&mut self.s); } pub fn hash_path(&mut self, p: &Path) { diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 68d91301b2ca..cb25759b4ae0 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -95,7 +95,16 @@ pub fn differing_macro_contexts(lhs: Span, rhs: Span) -> bool { } /// Returns true if this `expn_info` was expanded by any macro. pub fn in_macro(cx: &T, span: Span) -> bool { - cx.sess().codemap().with_expn_info(span.expn_id, |info| info.is_some()) + cx.sess().codemap().with_expn_info(span.expn_id, |info| match info { + Some(info) => { + match info.callee.format { + // don't treat range expressions desugared to structs as "in_macro" + ExpnFormat::CompilerDesugaring(name) => name != "...", + _ => true, + } + }, + None => false, + }) } /// Returns true if the macro that expanded the crate was outside of the current crate or was a @@ -150,6 +159,9 @@ pub fn match_def_path(cx: &LateContext, def_id: DefId, path: &[&str]) -> bool { let mut apb = AbsolutePathBuffer { names: vec![] }; cx.tcx.push_item_path(&mut apb, def_id); + if path == paths::VEC_FROM_ELEM { + println!("{:#?} == {:#?}", apb.names, path); + } apb.names.len() == path.len() && apb.names.iter().zip(path.iter()).all(|(a, &b)| &**a == b) @@ -197,6 +209,23 @@ pub fn match_trait_method(cx: &LateContext, expr: &Expr, path: &[&str]) -> bool } } +pub fn last_path_segment(path: &QPath) -> &PathSegment { + match *path { + QPath::Resolved(_, ref path) => path.segments + .last() + .expect("A path must have at least one segment"), + QPath::TypeRelative(_, ref seg) => &seg, + } +} + +pub fn single_segment_path(path: &QPath) -> Option<&PathSegment> { + match *path { + QPath::Resolved(_, ref path) if path.segments.len() == 1 => Some(&path.segments[0]), + QPath::Resolved(..) => None, + QPath::TypeRelative(_, ref seg) => Some(&seg), + } +} + /// Match a `Path` against a slice of segment string literals. /// /// # Examples @@ -204,10 +233,16 @@ pub fn match_trait_method(cx: &LateContext, expr: &Expr, path: &[&str]) -> bool /// match_path(path, &["std", "rt", "begin_unwind"]) /// ``` pub fn match_path(path: &QPath, segments: &[&str]) -> bool { - if let QPath::Resolved(_, ref path) = *path { - match_path_old(path, segments) - } else { - false + match *path { + QPath::Resolved(_, ref path) => match_path_old(path, segments), + QPath::TypeRelative(ref ty, ref segment) => match ty.node { + TyPath(ref inner_path) => { + segments.len() > 0 && + match_path(inner_path, &segments[..(segments.len() - 1)]) && + segment.name == segments[segments.len() - 1] + }, + _ => false, + }, } } diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index eefc2e5193a0..402985613aeb 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -12,7 +12,7 @@ pub const CLONE_TRAIT: [&'static str; 3] = ["core", "clone", "Clone"]; 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 CSTRING_NEW: [&'static str; 5] = ["std", "ffi", "c_str", "CString", "new"]; 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] = ["core", "fmt", "Display", "fmt"]; @@ -64,6 +64,7 @@ pub const RESULT: [&'static str; 3] = ["core", "result", "Result"]; pub const RESULT_ERR: [&'static str; 4] = ["core", "result", "Result", "Err"]; pub const RESULT_OK: [&'static str; 4] = ["core", "result", "Result", "Ok"]; pub const SERDE_DE_VISITOR: [&'static str; 3] = ["serde", "de", "Visitor"]; +pub const SLICE_INTO_VEC: [&'static str; 4] = ["collections", "slice", "", "into_vec"]; 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"]; diff --git a/tests/compile-fail/copies.rs b/tests/compile-fail/copies.rs index 3e65d1aeec47..005129f1b5da 100644 --- a/tests/compile-fail/copies.rs +++ b/tests/compile-fail/copies.rs @@ -9,6 +9,7 @@ #![allow(blacklisted_name)] #![allow(collapsible_if)] #![allow(zero_divided_by_zero, eq_op)] +#![allow(path_statements)] fn bar(_: T) {} fn foo() -> bool { unimplemented!() } diff --git a/tests/compile-fail/dlist.rs b/tests/compile-fail/dlist.rs index e79196191212..525c166e62af 100644 --- a/tests/compile-fail/dlist.rs +++ b/tests/compile-fail/dlist.rs @@ -10,5 +10,5 @@ pub fn test(_: LinkedList) { //~ ERROR I see you're using a LinkedList! } fn main(){ - test(LinkedList::new()); + test(LinkedList::new()); //~ ERROR I see you're using a LinkedList! } diff --git a/tests/compile-test.rs b/tests/compile-test.rs index deadd499192d..50701ce0ec99 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -21,6 +21,7 @@ fn run_mode(dir: &'static str, mode: &'static str) { fn prepare_env() { set_var("CLIPPY_DISABLE_WIKI_LINKS", "true"); + set_var("RUST_BACKTRACE", "0"); // these are riddicously slow right now } #[test]