diff --git a/clippy_lints/src/array_indexing.rs b/clippy_lints/src/array_indexing.rs index f3b7297b296..2c84c7cc132 100644 --- a/clippy_lints/src/array_indexing.rs +++ b/clippy_lints/src/array_indexing.rs @@ -6,7 +6,7 @@ use rustc_const_eval::eval_const_expr_partial; use rustc_const_math::ConstInt; use rustc::hir::*; use syntax::ast::RangeLimits; -use utils; +use utils::{self, higher}; /// **What it does:** Check for out of bounds array indexing with a constant index. /// @@ -77,7 +77,7 @@ impl LateLintPass for ArrayIndexing { } // Index is a constant range - if let Some(range) = utils::unsugar_range(index) { + if let Some(range) = higher::range(index) { let start = range.start .map(|start| eval_const_expr_partial(cx.tcx, start, ExprTypeChecked, None)) .map(|v| v.ok()); @@ -94,7 +94,7 @@ impl LateLintPass for ArrayIndexing { } } - if let Some(range) = utils::unsugar_range(index) { + if let Some(range) = higher::range(index) { // Full ranges are always valid if range.start.is_none() && range.end.is_none() { return; diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 1816d88bddb..e945afbf659 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -14,10 +14,9 @@ use std::collections::HashMap; use syntax::ast; use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, in_external_macro, - span_help_and_lint, is_integer_literal, get_enclosing_block, span_lint_and_then, unsugar_range, + span_help_and_lint, is_integer_literal, get_enclosing_block, span_lint_and_then, higher, walk_ptrs_ty, recover_for_loop}; use utils::paths; -use utils::UnsugaredRange; /// **What it does:** This lint checks for looping over the range of `0..len` of some collection just to get the values by index. /// @@ -333,7 +332,7 @@ fn check_for_loop(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &E /// Check for looping over a range and then indexing a sequence with it. /// The iteratee must be a range literal. fn check_for_loop_range(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &Expr) { - if let Some(UnsugaredRange { start: Some(ref start), ref end, .. }) = unsugar_range(arg) { + if let Some(higher::Range { start: Some(ref start), ref end, .. }) = higher::range(arg) { // the var must be a single name if let PatKind::Binding(_, ref ident, _) = pat.node { let mut visitor = VarVisitor { @@ -427,7 +426,7 @@ fn is_len_call(expr: &Expr, var: &Name) -> bool { fn check_for_loop_reverse_range(cx: &LateContext, arg: &Expr, expr: &Expr) { // if this for loop is iterating over a two-sided range... - if let Some(UnsugaredRange { start: Some(ref start), end: Some(ref end), limits }) = unsugar_range(arg) { + if let Some(higher::Range { start: Some(ref start), end: Some(ref end), limits }) = higher::range(arg) { // ...and both sides are compile-time constant integers... if let Ok(start_idx) = eval_const_expr_partial(cx.tcx, start, ExprTypeChecked, None) { if let Ok(end_idx) = eval_const_expr_partial(cx.tcx, end, ExprTypeChecked, None) { diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index 8eacbadf8df..a042c966d94 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -1,7 +1,8 @@ use rustc::lint::*; use rustc::hir::*; use syntax::codemap::Spanned; -use utils::{is_integer_literal, match_type, paths, snippet, span_lint, unsugar_range, UnsugaredRange}; +use utils::{is_integer_literal, match_type, paths, snippet, span_lint}; +use utils::higher; /// **What it does:** This lint checks for iterating over ranges with a `.step_by(0)`, which never terminates. /// @@ -54,7 +55,7 @@ impl LateLintPass for StepByZero { let ExprMethodCall( Spanned { node: ref iter_name, .. }, _, ref iter_args ) = *iter, iter_name.as_str() == "iter", // range expression in .zip() call: 0..x.len() - let Some(UnsugaredRange { start: Some(ref start), end: Some(ref end), .. }) = unsugar_range(zip_arg), + let Some(higher::Range { start: Some(ref start), end: Some(ref end), .. }) = higher::range(zip_arg), is_integer_literal(start, 0), // .len() call let ExprMethodCall(Spanned { node: ref len_name, .. }, _, ref len_args) = end.node, diff --git a/clippy_lints/src/utils/higher.rs b/clippy_lints/src/utils/higher.rs index 5df91ee9ab4..979c044cdbf 100644 --- a/clippy_lints/src/utils/higher.rs +++ b/clippy_lints/src/utils/higher.rs @@ -2,6 +2,7 @@ use rustc::hir; use syntax::ast; +use utils::{match_path, paths}; /// Convert a hir binary operator to the corresponding `ast` type. pub fn binop(op: hir::BinOp_) -> ast::BinOpKind { @@ -26,3 +27,91 @@ pub fn binop(op: hir::BinOp_) -> ast::BinOpKind { hir::BiSub => ast::BinOpKind::Sub, } } + +/// Represent a range akin to `ast::ExprKind::Range`. +#[derive(Debug, Copy, Clone)] +pub struct Range<'a> { + pub start: Option<&'a hir::Expr>, + pub end: Option<&'a hir::Expr>, + pub limits: ast::RangeLimits, +} + +/// Higher a `hir` range to something similar to `ast::ExprKind::Range`. +pub fn range(expr: &hir::Expr) -> Option { + // To be removed when ranges get stable. + fn unwrap_unstable(expr: &hir::Expr) -> &hir::Expr { + if let hir::ExprBlock(ref block) = expr.node { + if block.rules == hir::BlockCheckMode::PushUnstableBlock || block.rules == hir::BlockCheckMode::PopUnstableBlock { + if let Some(ref expr) = block.expr { + return expr; + } + } + } + + expr + } + + fn get_field<'a>(name: &str, fields: &'a [hir::Field]) -> Option<&'a hir::Expr> { + let expr = &fields.iter() + .find(|field| field.name.node.as_str() == name) + .unwrap_or_else(|| panic!("missing {} field for range", name)) + .expr; + + Some(unwrap_unstable(expr)) + } + + // 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 { + hir::ExprPath(None, ref path) => { + if match_path(path, &paths::RANGE_FULL_STD) || match_path(path, &paths::RANGE_FULL) { + Some(Range { + start: None, + end: None, + limits: ast::RangeLimits::HalfOpen, + }) + } else { + None + } + } + hir::ExprStruct(ref path, ref fields, None) => { + if match_path(path, &paths::RANGE_FROM_STD) || match_path(path, &paths::RANGE_FROM) { + Some(Range { + start: get_field("start", fields), + end: None, + limits: ast::RangeLimits::HalfOpen, + }) + } else if match_path(path, &paths::RANGE_INCLUSIVE_NON_EMPTY_STD) || + match_path(path, &paths::RANGE_INCLUSIVE_NON_EMPTY) { + Some(Range { + start: get_field("start", fields), + end: get_field("end", fields), + limits: ast::RangeLimits::Closed, + }) + } else if match_path(path, &paths::RANGE_STD) || match_path(path, &paths::RANGE) { + Some(Range { + start: get_field("start", fields), + end: get_field("end", fields), + limits: ast::RangeLimits::HalfOpen, + }) + } else if match_path(path, &paths::RANGE_TO_INCLUSIVE_STD) || match_path(path, &paths::RANGE_TO_INCLUSIVE) { + Some(Range { + start: None, + end: get_field("end", fields), + limits: ast::RangeLimits::Closed, + }) + } else if match_path(path, &paths::RANGE_TO_STD) || match_path(path, &paths::RANGE_TO) { + Some(Range { + start: None, + end: get_field("end", fields), + limits: ast::RangeLimits::HalfOpen, + }) + } else { + None + } + } + _ => None, + } +} + diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 6d41d5039f7..3ceae788a4d 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -13,7 +13,7 @@ use std::borrow::Cow; use std::env; use std::mem; use std::str::FromStr; -use syntax::ast::{self, LitKind, RangeLimits}; +use syntax::ast::{self, LitKind}; use syntax::codemap::{ExpnInfo, Span, ExpnFormat}; use syntax::errors::DiagnosticBuilder; use syntax::ptr::P; @@ -683,93 +683,6 @@ pub fn camel_case_from(s: &str) -> usize { last_i } -/// Represent a range akin to `ast::ExprKind::Range`. -#[derive(Debug, Copy, Clone)] -pub struct UnsugaredRange<'a> { - pub start: Option<&'a Expr>, - pub end: Option<&'a Expr>, - pub limits: RangeLimits, -} - -/// Unsugar a `hir` range. -pub fn unsugar_range(expr: &Expr) -> Option { - // To be removed when ranges get stable. - fn unwrap_unstable(expr: &Expr) -> &Expr { - if let ExprBlock(ref block) = expr.node { - if block.rules == BlockCheckMode::PushUnstableBlock || block.rules == BlockCheckMode::PopUnstableBlock { - if let Some(ref expr) = block.expr { - return expr; - } - } - } - - expr - } - - fn get_field<'a>(name: &str, fields: &'a [Field]) -> Option<&'a Expr> { - let expr = &fields.iter() - .find(|field| field.name.node.as_str() == name) - .unwrap_or_else(|| panic!("missing {} field for range", name)) - .expr; - - Some(unwrap_unstable(expr)) - } - - // 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_STD) || match_path(path, &paths::RANGE_FULL) { - Some(UnsugaredRange { - start: None, - end: None, - limits: RangeLimits::HalfOpen, - }) - } else { - None - } - } - ExprStruct(ref path, ref fields, None) => { - 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_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_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_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_STD) || match_path(path, &paths::RANGE_TO) { - Some(UnsugaredRange { - start: None, - end: get_field("end", fields), - limits: RangeLimits::HalfOpen, - }) - } else { - None - } - } - _ => None, - } -} - /// Convenience function to get the return type of a function or `None` if the function diverges. pub fn return_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, fn_item: NodeId) -> Option> { let parameter_env = ty::ParameterEnvironment::for_item(cx.tcx, fn_item);