diff --git a/README.md b/README.md index a74de7c69983..48a106e3591d 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ A collection of lints to catch common mistakes and improve your Rust code. [Jump to link with clippy-service](#link-with-clippy-service) ##Lints -There are 133 lints included in this crate: +There are 134 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -58,6 +58,7 @@ name [if_not_else](https://github.com/Manishearth/rust-clippy/wiki#if_not_else) | warn | finds if branches that could be swapped so no negation operation is necessary on the condition [if_same_then_else](https://github.com/Manishearth/rust-clippy/wiki#if_same_then_else) | warn | if with the same *then* and *else* blocks [ifs_same_cond](https://github.com/Manishearth/rust-clippy/wiki#ifs_same_cond) | warn | consecutive `ifs` with the same condition +[indexing_slicing](https://github.com/Manishearth/rust-clippy/wiki#indexing_slicing) | allow | indexing/slicing usage [ineffective_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#ineffective_bit_mask) | warn | expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2` [inline_always](https://github.com/Manishearth/rust-clippy/wiki#inline_always) | warn | `#[inline(always)]` is a bad idea in most cases [invalid_regex](https://github.com/Manishearth/rust-clippy/wiki#invalid_regex) | deny | finds invalid regular expressions in `Regex::new(_)` invocations @@ -234,7 +235,7 @@ And, in your `main.rs` or `lib.rs`: Both projects are independent and maintained by different people (even if some `clippy-service`'s contributions are authored by some `rust-clippy` members). -You can check it out this great service at [clippy.bashy.io](https://clippy.bashy.io/) +You can check it out this great service at [clippy.bashy.io](https://clippy.bashy.io/). ##License Licensed under [MPL](https://www.mozilla.org/MPL/2.0/). If you're having issues with the license, let me know and I'll try to change it to something more permissive. diff --git a/src/array_indexing.rs b/src/array_indexing.rs index cfa52f390d29..274491b7c963 100644 --- a/src/array_indexing.rs +++ b/src/array_indexing.rs @@ -3,7 +3,8 @@ use rustc::middle::const_eval::EvalHint::ExprTypeChecked; use rustc::middle::const_eval::{eval_const_expr_partial, ConstVal}; use rustc::middle::ty::TyArray; use rustc_front::hir::*; -use utils::span_lint; +use syntax::ast::RangeLimits; +use utils; /// **What it does:** Check for out of bounds array indexing with a constant index. /// @@ -17,6 +18,7 @@ use utils::span_lint; /// let x = [1,2,3,4]; /// ... /// x[9]; +/// &x[2..9]; /// ``` declare_lint! { pub OUT_OF_BOUNDS_INDEXING, @@ -24,28 +26,114 @@ declare_lint! { "out of bound constant indexing" } +/// **What it does:** Check for usage of indexing or slicing. +/// +/// **Why is this bad?** Usually, this can be safely allowed. However, +/// in some domains such as kernel development, a panic can cause the +/// whole operating system to crash. +/// +/// **Known problems:** Hopefully none. +/// +/// **Example:** +/// +/// ``` +/// ... +/// x[2]; +/// &x[0..2]; +/// ``` +declare_lint! { + pub INDEXING_SLICING, + Allow, + "indexing/slicing usage" +} + #[derive(Copy,Clone)] pub struct ArrayIndexing; impl LintPass for ArrayIndexing { fn get_lints(&self) -> LintArray { - lint_array!(OUT_OF_BOUNDS_INDEXING) + lint_array!(INDEXING_SLICING, OUT_OF_BOUNDS_INDEXING) } } impl LateLintPass for ArrayIndexing { fn check_expr(&mut self, cx: &LateContext, e: &Expr) { if let ExprIndex(ref array, ref index) = e.node { + // Array with known size can be checked statically let ty = cx.tcx.expr_ty(array); - if let TyArray(_, size) = ty.sty { - let index = eval_const_expr_partial(cx.tcx, &index, ExprTypeChecked, None); - if let Ok(ConstVal::Uint(index)) = index { - if size as u64 <= index { - span_lint(cx, OUT_OF_BOUNDS_INDEXING, e.span, "const index-expr is out of bounds"); + let size = size as u64; + + // Index is a constant uint + let const_index = eval_const_expr_partial(cx.tcx, &index, ExprTypeChecked, None); + if let Ok(ConstVal::Uint(const_index)) = const_index { + if size <= const_index { + utils::span_lint(cx, OUT_OF_BOUNDS_INDEXING, e.span, "const index is out of bounds"); + } + + return; + } + + // Index is a constant range + if let Some(range) = utils::unsugar_range(index) { + let start = range.start.map(|start| + eval_const_expr_partial(cx.tcx, start, ExprTypeChecked, None)).map(|v| v.ok()); + let end = range.end.map(|end| + eval_const_expr_partial(cx.tcx, end, ExprTypeChecked, None)).map(|v| v.ok()); + + if let Some((start, end)) = to_const_range(start, end, range.limits, size) { + if start >= size || end >= size { + utils::span_lint(cx, + OUT_OF_BOUNDS_INDEXING, + e.span, + "range is out of bounds"); + } + return; } } } + + if let Some(range) = utils::unsugar_range(index) { + // Full ranges are always valid + if range.start.is_none() && range.end.is_none() { + return; + } + + // Impossible to know if indexing or slicing is correct + utils::span_lint(cx, INDEXING_SLICING, e.span, "slicing may panic"); + } else { + utils::span_lint(cx, INDEXING_SLICING, e.span, "indexing may panic"); + } } } } + +/// Returns an option containing a tuple with the start and end (exclusive) of the range +/// +/// Note: we assume the start and the end of the range are unsigned, since array slicing +/// works only on usize +fn to_const_range(start: Option>, + end: Option>, + limits: RangeLimits, + array_size: u64) + -> Option<(u64, u64)> { + let start = match start { + Some(Some(ConstVal::Uint(x))) => x, + Some(_) => return None, + None => 0, + }; + + let end = match end { + Some(Some(ConstVal::Uint(x))) => { + if limits == RangeLimits::Closed { + x + } else { + x - 1 + } + } + Some(_) => return None, + None => array_size - 1, + }; + + Some((start, end)) +} diff --git a/src/lib.rs b/src/lib.rs index 8c619a1eaf72..4b9d5d8c9c4a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,7 +2,7 @@ #![feature(rustc_private, collections)] #![feature(iter_arith)] #![feature(custom_attribute)] -#![allow(unknown_lints)] +#![allow(indexing_slicing, shadow_reuse, unknown_lints)] // this only exists to allow the "dogfood" integration test to work #[allow(dead_code)] @@ -181,6 +181,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box new_without_default::NewWithoutDefault); reg.register_lint_group("clippy_pedantic", vec![ + array_indexing::INDEXING_SLICING, enum_glob_use::ENUM_GLOB_USE, matches::SINGLE_MATCH_ELSE, methods::OPTION_UNWRAP_USED, diff --git a/src/utils/mod.rs b/src/utils/mod.rs index c626fcb8930c..625e8da197d5 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -681,6 +681,7 @@ pub fn camel_case_from(s: &str) -> usize { } /// Represents 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>, @@ -711,24 +712,30 @@ pub fn unsugar_range(expr: &Expr) -> Option { Some(unwrap_unstable(expr)) } - if let ExprStruct(ref path, ref fields, None) = unwrap_unstable(&expr).node { - if match_path(path, &RANGE_FROM_PATH) { - Some(UnsugaredRange { start: get_field("start", fields), end: None, limits: RangeLimits::HalfOpen }) - } else if match_path(path, &RANGE_FULL_PATH) { - Some(UnsugaredRange { start: None, end: None, limits: RangeLimits::HalfOpen }) - } else if match_path(path, &RANGE_INCLUSIVE_NON_EMPTY_PATH) { - Some(UnsugaredRange { start: get_field("start", fields), end: get_field("end", fields), limits: RangeLimits::Closed }) - } else if match_path(path, &RANGE_PATH) { - Some(UnsugaredRange { start: get_field("start", fields), end: get_field("end", fields), limits: RangeLimits::HalfOpen }) - } else if match_path(path, &RANGE_TO_INCLUSIVE_PATH) { - Some(UnsugaredRange { start: None, end: get_field("end", fields), limits: RangeLimits::Closed }) - } else if match_path(path, &RANGE_TO_PATH) { - Some(UnsugaredRange { start: None, end: get_field("end", fields), limits: RangeLimits::HalfOpen }) - } else { - None + match unwrap_unstable(&expr).node { + ExprPath(None, ref path) => { + if match_path(path, &RANGE_FULL_PATH) { + Some(UnsugaredRange { start: None, end: None, limits: RangeLimits::HalfOpen }) + } else { + None + } } - } else { - None + ExprStruct(ref path, ref fields, None) => { + if match_path(path, &RANGE_FROM_PATH) { + Some(UnsugaredRange { start: get_field("start", fields), end: None, limits: RangeLimits::HalfOpen }) + } else if match_path(path, &RANGE_INCLUSIVE_NON_EMPTY_PATH) { + Some(UnsugaredRange { start: get_field("start", fields), end: get_field("end", fields), limits: RangeLimits::Closed }) + } else if match_path(path, &RANGE_PATH) { + Some(UnsugaredRange { start: get_field("start", fields), end: get_field("end", fields), limits: RangeLimits::HalfOpen }) + } else if match_path(path, &RANGE_TO_INCLUSIVE_PATH) { + Some(UnsugaredRange { start: None, end: get_field("end", fields), limits: RangeLimits::Closed }) + } else if match_path(path, &RANGE_TO_PATH) { + Some(UnsugaredRange { start: None, end: get_field("end", fields), limits: RangeLimits::HalfOpen }) + } else { + None + } + } + _ => None, } } diff --git a/tests/compile-fail/array_indexing.rs b/tests/compile-fail/array_indexing.rs index 1d9492bc0ab1..14f3448a9f67 100644 --- a/tests/compile-fail/array_indexing.rs +++ b/tests/compile-fail/array_indexing.rs @@ -1,6 +1,7 @@ -#![feature(plugin)] +#![feature(inclusive_range_syntax, plugin)] #![plugin(clippy)] +#![deny(indexing_slicing)] #![deny(out_of_bounds_indexing)] #![allow(no_effect)] @@ -8,6 +9,19 @@ fn main() { let x = [1,2,3,4]; x[0]; x[3]; - x[4]; //~ERROR: const index-expr is out of bounds - x[1 << 3]; //~ERROR: const index-expr is out of bounds + x[4]; //~ERROR: const index is out of bounds + x[1 << 3]; //~ERROR: const index is out of bounds + &x[1..5]; //~ERROR: range is out of bounds + &x[0..3]; + &x[0...4]; //~ERROR: range is out of bounds + &x[..]; + &x[1..]; + &x[..4]; + &x[..5]; //~ERROR: range is out of bounds + + let y = &x; + y[0]; //~ERROR: indexing may panic + &y[1..2]; //~ERROR: slicing may panic + &y[..]; + &y[0...4]; //~ERROR: slicing may panic }