diff --git a/src/loops.rs b/src/loops.rs index 0b341f645df3..70abb7a1aac7 100644 --- a/src/loops.rs +++ b/src/loops.rs @@ -285,7 +285,7 @@ impl LateLintPass for LoopsPass { let iter_expr = &method_args[0]; if let Some(lhs_constructor) = path.segments.last() { if method_name.node.as_str() == "next" && - match_trait_method(cx, match_expr, &["core", "iter", "Iterator"]) && + match_trait_method(cx, match_expr, &paths::ITERATOR) && lhs_constructor.identifier.name.as_str() == "Some" && !is_iterator_used_after_while_let(cx, iter_expr) { let iterator = snippet(cx, method_args[0].span, "_"); @@ -305,7 +305,7 @@ impl LateLintPass for LoopsPass { if let StmtSemi(ref expr, _) = stmt.node { if let ExprMethodCall(ref method, _, ref args) = expr.node { if args.len() == 1 && method.node.as_str() == "collect" && - match_trait_method(cx, expr, &["core", "iter", "Iterator"]) { + match_trait_method(cx, expr, &paths::ITERATOR) { span_lint(cx, UNUSED_COLLECT, expr.span, @@ -488,7 +488,7 @@ fn check_for_loop_arg(cx: &LateContext, pat: &Pat, arg: &Expr, expr: &Expr) { object, method_name)); } - } else if method_name.as_str() == "next" && match_trait_method(cx, arg, &["core", "iter", "Iterator"]) { + } else if method_name.as_str() == "next" && match_trait_method(cx, arg, &paths::ITERATOR) { span_lint(cx, ITER_NEXT_LOOP, expr.span, @@ -739,11 +739,11 @@ fn is_ref_iterable_type(cx: &LateContext, e: &Expr) -> bool { match_type(cx, ty, &paths::VEC) || match_type(cx, ty, &paths::LINKED_LIST) || match_type(cx, ty, &paths::HASHMAP) || - match_type(cx, ty, &["std", "collections", "hash", "set", "HashSet"]) || - match_type(cx, ty, &["collections", "vec_deque", "VecDeque"]) || - match_type(cx, ty, &["collections", "binary_heap", "BinaryHeap"]) || + match_type(cx, ty, &paths::HASHSET) || + match_type(cx, ty, &paths::VEC_DEQUE) || + match_type(cx, ty, &paths::BINARY_HEAP) || match_type(cx, ty, &paths::BTREEMAP) || - match_type(cx, ty, &["collections", "btree", "set", "BTreeSet"]) + match_type(cx, ty, &paths::BTREESET) } fn is_iterable_array(ty: ty::Ty) -> bool { diff --git a/src/map_clone.rs b/src/map_clone.rs index 1cfa339d4a06..1a0620d88345 100644 --- a/src/map_clone.rs +++ b/src/map_clone.rs @@ -96,7 +96,7 @@ fn expr_eq_ident(expr: &Expr, id: Ident) -> bool { } fn get_type_name(cx: &LateContext, expr: &Expr, arg: &Expr) -> Option<&'static str> { - if match_trait_method(cx, expr, &["core", "iter", "Iterator"]) { + if match_trait_method(cx, expr, &paths::ITERATOR) { Some("iterator") } else if match_type(cx, walk_ptrs_ty(cx.tcx.expr_ty(arg)), &paths::OPTION) { Some("Option") diff --git a/src/methods.rs b/src/methods.rs index 73f6a7aa4b02..e25154bd38b6 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -752,7 +752,7 @@ fn lint_map_unwrap_or_else(cx: &LateContext, expr: &Expr, map_args: &MethodArgs, /// lint use of `filter().next() for Iterators` fn lint_filter_next(cx: &LateContext, expr: &Expr, filter_args: &MethodArgs) { // lint if caller of `.filter().next()` is an Iterator - if match_trait_method(cx, expr, &["core", "iter", "Iterator"]) { + if match_trait_method(cx, expr, &paths::ITERATOR) { let msg = "called `filter(p).next()` on an Iterator. This is more succinctly expressed by calling `.find(p)` \ instead."; let filter_snippet = snippet(cx, filter_args[1].span, ".."); @@ -776,7 +776,7 @@ fn lint_filter_next(cx: &LateContext, expr: &Expr, filter_args: &MethodArgs) { fn lint_search_is_some(cx: &LateContext, expr: &Expr, search_method: &str, search_args: &MethodArgs, is_some_args: &MethodArgs) { // lint if caller of search is an Iterator - if match_trait_method(cx, &*is_some_args[0], &["core", "iter", "Iterator"]) { + if match_trait_method(cx, &*is_some_args[0], &paths::ITERATOR) { let msg = format!("called `is_some()` after searching an iterator with {}. This is more succinctly expressed \ by calling `any()`.", search_method); diff --git a/src/minmax.rs b/src/minmax.rs index 7cd2d33cab94..eaba19b08e41 100644 --- a/src/minmax.rs +++ b/src/minmax.rs @@ -3,7 +3,7 @@ use rustc::lint::*; use rustc::hir::*; use std::cmp::{PartialOrd, Ordering}; use syntax::ptr::P; -use utils::{match_def_path, span_lint}; +use utils::{match_def_path, paths, span_lint}; /// **What it does:** This lint checks for expressions where `std::cmp::min` and `max` are used to clamp values, but switched so that the result is constant. /// @@ -57,9 +57,9 @@ fn min_max<'a>(cx: &LateContext, expr: &'a Expr) -> Option<(MinMax, Constant, &' if let ExprPath(None, _) = path.node { let def_id = cx.tcx.def_map.borrow()[&path.id].def_id(); - if match_def_path(cx, def_id, &["core", "cmp", "min"]) { + if match_def_path(cx, def_id, &paths::CMP_MIN) { fetch_const(args, MinMax::Min) - } else if match_def_path(cx, def_id, &["core", "cmp", "max"]) { + } else if match_def_path(cx, def_id, &paths::CMP_MAX) { fetch_const(args, MinMax::Max) } else { None diff --git a/src/ranges.rs b/src/ranges.rs index c2555da1d0b8..e96212a9cefa 100644 --- a/src/ranges.rs +++ b/src/ranges.rs @@ -1,7 +1,7 @@ use rustc::lint::*; use rustc::hir::*; use syntax::codemap::Spanned; -use utils::{is_integer_literal, match_type, snippet, span_lint, unsugar_range, UnsugaredRange}; +use utils::{is_integer_literal, match_type, paths, snippet, span_lint, unsugar_range, UnsugaredRange}; /// **What it does:** This lint checks for iterating over ranges with a `.step_by(0)`, which never terminates. /// @@ -39,7 +39,7 @@ impl LateLintPass for StepByZero { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { if let ExprMethodCall(Spanned { node: ref name, .. }, _, ref args) = expr.node { // Range with step_by(0). - if name.as_str() == "step_by" && args.len() == 2 && is_range(cx, &args[0]) && + if name.as_str() == "step_by" && args.len() == 2 && has_step_by(cx, &args[0]) && is_integer_literal(&args[1], 0) { span_lint(cx, RANGE_STEP_BY_ZERO, @@ -77,10 +77,13 @@ impl LateLintPass for StepByZero { } } -fn is_range(cx: &LateContext, expr: &Expr) -> bool { +fn has_step_by(cx: &LateContext, expr: &Expr) -> bool { // No need for walk_ptrs_ty here because step_by moves self, so it // can't be called on a borrowed range. let ty = cx.tcx.expr_ty(expr); - // Note: RangeTo and RangeFull don't have step_by - match_type(cx, ty, &["core", "ops", "Range"]) || match_type(cx, ty, &["core", "ops", "RangeFrom"]) + + // Note: `RangeTo`, `RangeToInclusive` and `RangeFull` don't have step_by + match_type(cx, ty, &paths::RANGE) + || match_type(cx, ty, &paths::RANGE_FROM) + || match_type(cx, ty, &paths::RANGE_INCLUSIVE) } diff --git a/src/utils/mod.rs b/src/utils/mod.rs index d8f5a0757b79..83247a591749 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -122,23 +122,33 @@ pub fn in_external_macro(cx: &T, span: Span) -> bool { /// ``` /// match_def_path(cx, id, &["core", "option", "Option"]) /// ``` +/// +/// See also the `paths` module. pub fn match_def_path(cx: &LateContext, def_id: DefId, path: &[&str]) -> bool { - let krate = &cx.tcx.crate_name(def_id.krate); - if krate != &path[0] { - return false; + use syntax::parse::token; + + struct AbsolutePathBuffer { + names: Vec, } - let path = &path[1..]; - let other = cx.tcx.def_path(def_id).data; + impl ty::item_path::ItemPathBuffer for AbsolutePathBuffer { + fn root_mode(&self) -> &ty::item_path::RootMode { + const ABSOLUTE: &'static ty::item_path::RootMode = &ty::item_path::RootMode::Absolute; + ABSOLUTE + } - if other.len() != path.len() { - return false; + fn push(&mut self, text: &str) { + self.names.push(token::intern(text).as_str()); + } } - other.into_iter() - .map(|e| e.data) - .zip(path) - .all(|(nm, p)| nm.as_interned_str() == *p) + let mut apb = AbsolutePathBuffer { + names: vec![], + }; + + cx.tcx.push_item_path(&mut apb, def_id); + + apb.names == path } /// Check if type is struct or enum type with given def path. @@ -730,9 +740,12 @@ pub fn unsugar_range(expr: &Expr) -> Option { Some(unwrap_unstable(expr)) } - match unwrap_unstable(&expr).node { + // The range syntax is expanded to literal paths starting with `core` or `std` depending on + // `#[no_std]`. Testing both instead of resolving the paths. + + match unwrap_unstable(expr).node { ExprPath(None, ref path) => { - if match_path(path, &paths::RANGE_FULL) { + if match_path(path, &paths::RANGE_FULL_STD) || match_path(path, &paths::RANGE_FULL) { Some(UnsugaredRange { start: None, end: None, @@ -743,31 +756,31 @@ pub fn unsugar_range(expr: &Expr) -> Option { } } ExprStruct(ref path, ref fields, None) => { - if match_path(path, &paths::RANGE_FROM) { + if match_path(path, &paths::RANGE_FROM_STD) || match_path(path, &paths::RANGE_FROM) { Some(UnsugaredRange { start: get_field("start", fields), end: None, limits: RangeLimits::HalfOpen, }) - } else if match_path(path, &paths::RANGE_INCLUSIVE_NON_EMPTY) { + } else if match_path(path, &paths::RANGE_INCLUSIVE_NON_EMPTY_STD) || match_path(path, &paths::RANGE_INCLUSIVE_NON_EMPTY) { Some(UnsugaredRange { start: get_field("start", fields), end: get_field("end", fields), limits: RangeLimits::Closed, }) - } else if match_path(path, &paths::RANGE) { + } else if match_path(path, &paths::RANGE_STD) || match_path(path, &paths::RANGE) { Some(UnsugaredRange { start: get_field("start", fields), end: get_field("end", fields), limits: RangeLimits::HalfOpen, }) - } else if match_path(path, &paths::RANGE_TO_INCLUSIVE) { + } else if match_path(path, &paths::RANGE_TO_INCLUSIVE_STD) || match_path(path, &paths::RANGE_TO_INCLUSIVE) { Some(UnsugaredRange { start: None, end: get_field("end", fields), limits: RangeLimits::Closed, }) - } else if match_path(path, &paths::RANGE_TO) { + } else if match_path(path, &paths::RANGE_TO_STD) || match_path(path, &paths::RANGE_TO) { Some(UnsugaredRange { start: None, end: get_field("end", fields), diff --git a/src/utils/paths.rs b/src/utils/paths.rs index 38985373085b..7da75c3cd55a 100644 --- a/src/utils/paths.rs +++ b/src/utils/paths.rs @@ -1,12 +1,16 @@ //! This module contains paths to types and functions Clippy needs to know about. pub const BEGIN_UNWIND: [&'static str; 3] = ["std", "rt", "begin_unwind"]; -pub const BOX_NEW: [&'static str; 4] = ["std", "boxed", "Box", "new"]; +pub const BINARY_HEAP: [&'static str; 3] = ["collections", "binary_heap", "BinaryHeap"]; pub const BOX: [&'static str; 3] = ["std", "boxed", "Box"]; -pub const BTREEMAP_ENTRY: [&'static str; 4] = ["collections", "btree", "map", "Entry"]; +pub const BOX_NEW: [&'static str; 4] = ["std", "boxed", "Box", "new"]; pub const BTREEMAP: [&'static str; 4] = ["collections", "btree", "map", "BTreeMap"]; +pub const BTREEMAP_ENTRY: [&'static str; 4] = ["collections", "btree", "map", "Entry"]; +pub const BTREESET: [&'static str; 4] = ["collections", "btree", "set", "BTreeSet"]; pub const CLONE: [&'static str; 3] = ["clone", "Clone", "clone"]; pub const CLONE_TRAIT: [&'static str; 2] = ["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 DEBUG_FMT_METHOD: [&'static str; 4] = ["std", "fmt", "Debug", "fmt"]; @@ -15,25 +19,36 @@ pub const DISPLAY_FMT_METHOD: [&'static str; 4] = ["std", "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 HASHMAP_ENTRY: [&'static str; 5] = ["std", "collections", "hash", "map", "Entry"]; -pub const HASHMAP: [&'static str; 5] = ["std", "collections", "hash", "map", "HashMap"]; 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 IO_PRINT: [&'static str; 3] = ["std", "io", "_print"]; +pub const ITERATOR: [&'static str; 4] = ["core", "iter", "iterator", "Iterator"]; pub const LINKED_LIST: [&'static str; 3] = ["collections", "linked_list", "LinkedList"]; pub const MEM_FORGET: [&'static str; 3] = ["core", "mem", "forget"]; pub const MUTEX: [&'static str; 4] = ["std", "sync", "mutex", "Mutex"]; pub const OPEN_OPTIONS: [&'static str; 3] = ["std", "fs", "OpenOptions"]; pub const OPTION: [&'static str; 3] = ["core", "option", "Option"]; -pub const RANGE_FROM: [&'static str; 3] = ["std", "ops", "RangeFrom"]; -pub const RANGE_FULL: [&'static str; 3] = ["std", "ops", "RangeFull"]; -pub const RANGE_INCLUSIVE_NON_EMPTY: [&'static str; 4] = ["std", "ops", "RangeInclusive", "NonEmpty"]; -pub const RANGE: [&'static str; 3] = ["std", "ops", "Range"]; -pub const RANGE_TO_INCLUSIVE: [&'static str; 3] = ["std", "ops", "RangeToInclusive"]; -pub const RANGE_TO: [&'static str; 3] = ["std", "ops", "RangeTo"]; +pub const RANGE: [&'static str; 3] = ["core", "ops", "Range"]; +pub const RANGE_FROM: [&'static str; 3] = ["core", "ops", "RangeFrom"]; +pub const RANGE_FROM_STD: [&'static str; 3] = ["std", "ops", "RangeFrom"]; +pub const RANGE_FULL: [&'static str; 3] = ["core", "ops", "RangeFull"]; +pub const RANGE_FULL_STD: [&'static str; 3] = ["std", "ops", "RangeFull"]; +pub const RANGE_INCLUSIVE: [&'static str; 3] = ["core", "ops", "RangeInclusive"]; +pub const RANGE_INCLUSIVE_NON_EMPTY: [&'static str; 4] = ["core", "ops", "RangeInclusive", "NonEmpty"]; +pub const RANGE_INCLUSIVE_NON_EMPTY_STD: [&'static str; 4] = ["std", "ops", "RangeInclusive", "NonEmpty"]; +pub const RANGE_INCLUSIVE_STD: [&'static str; 3] = ["std", "ops", "RangeInclusive"]; +pub const RANGE_STD: [&'static str; 3] = ["std", "ops", "Range"]; +pub const RANGE_TO: [&'static str; 3] = ["core", "ops", "RangeTo"]; +pub const RANGE_TO_INCLUSIVE: [&'static str; 3] = ["core", "ops", "RangeToInclusive"]; +pub const RANGE_TO_INCLUSIVE_STD: [&'static str; 3] = ["std", "ops", "RangeToInclusive"]; +pub const RANGE_TO_STD: [&'static str; 3] = ["std", "ops", "RangeTo"]; pub const REGEX: [&'static str; 3] = ["regex", "re_unicode", "Regex"]; pub const REGEX_NEW: [&'static str; 3] = ["regex", "Regex", "new"]; pub const RESULT: [&'static str; 3] = ["core", "result", "Result"]; pub const STRING: [&'static str; 3] = ["collections", "string", "String"]; pub const TRANSMUTE: [&'static str; 4] = ["core", "intrinsics", "", "transmute"]; -pub const VEC_FROM_ELEM: [&'static str; 3] = ["std", "vec", "from_elem"]; 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"]; diff --git a/tests/compile-fail/range.rs b/tests/compile-fail/range.rs index 064d9f551732..fc12155ce9cb 100644 --- a/tests/compile-fail/range.rs +++ b/tests/compile-fail/range.rs @@ -1,4 +1,5 @@ #![feature(step_by)] +#![feature(inclusive_range_syntax)] #![feature(plugin)] #![plugin(clippy)] @@ -14,6 +15,7 @@ fn main() { (0..1).step_by(1); (1..).step_by(0); //~ERROR Range::step_by(0) produces an infinite iterator + (1...2).step_by(0); //~ERROR Range::step_by(0) produces an infinite iterator let x = 0..1; x.step_by(0); //~ERROR Range::step_by(0) produces an infinite iterator