-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #2790 from shnewto/vectors-to-indexing-slicing-lint
Extend `indexing_slicing` lint
- Loading branch information
Showing
7 changed files
with
542 additions
and
295 deletions.
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,183 @@ | ||
//! lint on indexing and slicing operations | ||
|
||
use crate::consts::{constant, Constant}; | ||
use crate::utils; | ||
use crate::utils::higher; | ||
use crate::utils::higher::Range; | ||
use rustc::hir::*; | ||
use rustc::lint::*; | ||
use rustc::ty; | ||
use syntax::ast::RangeLimits; | ||
|
||
/// **What it does:** Checks for out of bounds array indexing with a constant | ||
/// index. | ||
/// | ||
/// **Why is this bad?** This will always panic at runtime. | ||
/// | ||
/// **Known problems:** Hopefully none. | ||
/// | ||
/// **Example:** | ||
/// ```rust | ||
/// let x = [1,2,3,4]; | ||
/// | ||
/// // Bad | ||
/// x[9]; | ||
/// &x[2..9]; | ||
/// | ||
/// // Good | ||
/// x[0]; | ||
/// x[3]; | ||
/// ``` | ||
declare_clippy_lint! { | ||
pub OUT_OF_BOUNDS_INDEXING, | ||
correctness, | ||
"out of bounds constant indexing" | ||
} | ||
|
||
/// **What it does:** Checks for usage of indexing or slicing. Arrays are special cased, this lint | ||
/// does report on arrays if we can tell that slicing operations are in bounds and does not | ||
/// lint on constant `usize` indexing on arrays because that is handled by rustc's `const_err` lint. | ||
/// | ||
/// **Why is this bad?** Indexing and slicing can panic at runtime and there are | ||
/// safe alternatives. | ||
/// | ||
/// **Known problems:** Hopefully none. | ||
/// | ||
/// **Example:** | ||
/// ```rust | ||
/// // Vector | ||
/// let x = vec![0; 5]; | ||
/// | ||
/// // Bad | ||
/// x[2]; | ||
/// &x[2..100]; | ||
/// &x[2..]; | ||
/// &x[..100]; | ||
/// | ||
/// // Good | ||
/// x.get(2); | ||
/// x.get(2..100); | ||
/// x.get(2..); | ||
/// x.get(..100); | ||
/// | ||
/// // Array | ||
/// let y = [0, 1, 2, 3]; | ||
/// | ||
/// // Bad | ||
/// &y[10..100]; | ||
/// &y[10..]; | ||
/// &y[..100]; | ||
/// | ||
/// // Good | ||
/// &y[2..]; | ||
/// &y[..2]; | ||
/// &y[0..3]; | ||
/// y.get(10); | ||
/// y.get(10..100); | ||
/// y.get(10..); | ||
/// y.get(..100); | ||
/// ``` | ||
declare_clippy_lint! { | ||
pub INDEXING_SLICING, | ||
pedantic, | ||
"indexing/slicing usage" | ||
} | ||
|
||
#[derive(Copy, Clone)] | ||
pub struct IndexingSlicing; | ||
|
||
impl LintPass for IndexingSlicing { | ||
fn get_lints(&self) -> LintArray { | ||
lint_array!(INDEXING_SLICING, OUT_OF_BOUNDS_INDEXING) | ||
} | ||
} | ||
|
||
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IndexingSlicing { | ||
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { | ||
if let ExprIndex(ref array, ref index) = &expr.node { | ||
let ty = cx.tables.expr_ty(array); | ||
if let Some(range) = higher::range(cx, index) { | ||
// Ranged indexes, i.e. &x[n..m], &x[n..], &x[..n] and &x[..] | ||
if let ty::TyArray(_, s) = ty.sty { | ||
let size: u128 = s.assert_usize(cx.tcx).unwrap().into(); | ||
// Index is a constant range. | ||
if let Some((start, end)) = to_const_range(cx, range, size) { | ||
if start > size || end > size { | ||
utils::span_lint( | ||
cx, | ||
OUT_OF_BOUNDS_INDEXING, | ||
expr.span, | ||
"range is out of bounds", | ||
); | ||
} | ||
return; | ||
} | ||
} | ||
|
||
let help_msg = match (range.start, range.end) { | ||
(None, Some(_)) => "Consider using `.get(..n)`or `.get_mut(..n)` instead", | ||
(Some(_), None) => "Consider using `.get(n..)` or .get_mut(n..)` instead", | ||
(Some(_), Some(_)) => "Consider using `.get(n..m)` or `.get_mut(n..m)` instead", | ||
(None, None) => return, // [..] is ok. | ||
}; | ||
|
||
utils::span_help_and_lint( | ||
cx, | ||
INDEXING_SLICING, | ||
expr.span, | ||
"slicing may panic.", | ||
help_msg, | ||
); | ||
} else { | ||
// Catchall non-range index, i.e. [n] or [n << m] | ||
if let ty::TyArray(..) = ty.sty { | ||
// Index is a constant uint. | ||
if let Some(..) = constant(cx, cx.tables, index) { | ||
// Let rustc's `const_err` lint handle constant `usize` indexing on arrays. | ||
return; | ||
} | ||
} | ||
|
||
utils::span_help_and_lint( | ||
cx, | ||
INDEXING_SLICING, | ||
expr.span, | ||
"indexing may panic.", | ||
"Consider using `.get(n)` or `.get_mut(n)` instead", | ||
); | ||
} | ||
} | ||
} | ||
} | ||
|
||
/// Returns an option containing a tuple with the start and end (exclusive) of | ||
/// the range. | ||
fn to_const_range<'a, 'tcx>( | ||
cx: &LateContext<'a, 'tcx>, | ||
range: Range, | ||
array_size: u128, | ||
) -> Option<(u128, u128)> { | ||
let s = range | ||
.start | ||
.map(|expr| constant(cx, cx.tables, expr).map(|(c, _)| c)); | ||
let start = match s { | ||
Some(Some(Constant::Int(x))) => x, | ||
Some(_) => return None, | ||
None => 0, | ||
}; | ||
|
||
let e = range | ||
.end | ||
.map(|expr| constant(cx, cx.tables, expr).map(|(c, _)| c)); | ||
let end = match e { | ||
Some(Some(Constant::Int(x))) => if range.limits == RangeLimits::Closed { | ||
x + 1 | ||
} else { | ||
x | ||
}, | ||
Some(_) => return None, | ||
None => array_size, | ||
}; | ||
|
||
Some((start, end)) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.