rust-clippy/clippy_lints/src/indexing_slicing.rs

215 lines
6.7 KiB
Rust
Raw Normal View History

Extend `indexing_slicing` lint Hey there clippy team! I've made some assumptions in this PR and I'm not at all certain they'll look like the right approach to you. I'm looking forward to any feedback or revision requests you have, thanks! Prior to this commit the `indexing_slicing` lint was limited to indexing/slicing operations on arrays. This meant that the scope of a really useful lint didn't include vectors. In order to include vectors in the `indexing_slicing` lint a few steps were taken. The `array_indexing.rs` source file in `clippy_lints` was renamed to `indexing_slicing.rs` to more accurately reflect the lint's new scope. The `OUT_OF_BOUNDS_INDEXING` lint persists through these changes so if we can know that a constant index or slice on an array is in bounds no lint is triggered. The `array_indexing` tests in the `tests/ui` directory were also extended and moved to `indexing_slicing.rs` and `indexing_slicing.stderr`. The `indexing_slicing` lint was moved to the `clippy_pedantic` lint group. A specific "Consider using" string was added to each of the `indexing_slicing` lint reports. At least one of the test scenarios might look peculiar and I'll leave it up to y'all to decide if it's palatable. It's the result of indexing the array `x` after `let x = [1, 2, 3, 4];` ``` error: slicing may panic. Consider using `.get(..n)`or `.get_mut(..n)`instead --> $DIR/indexing_slicing.rs:23:6 | 23 | &x[0..][..3]; | ^^^^^^^^^^^ ``` The error string reports only on the second half's range-to, because the range-from is in bounds! Again, thanks for taking a look. Closes #2536
2018-05-23 04:56:02 +00:00
//! lint on indexing and slicing operations
use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
use clippy_utils::higher;
2020-03-01 03:23:33 +00:00
use rustc_ast::ast::RangeLimits;
2020-02-21 08:39:38 +00:00
use rustc_hir::{Expr, ExprKind};
2020-01-12 06:08:41 +00:00
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
2020-01-11 11:37:08 +00:00
use rustc_session::{declare_lint_pass, declare_tool_lint};
2015-12-21 18:22:29 +00:00
2018-03-28 13:24:26 +00:00
declare_clippy_lint! {
/// ### 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
2019-03-05 22:23:50 +00:00
/// ```no_run
/// # #![allow(const_err)]
/// let x = [1, 2, 3, 4];
///
/// // Bad
/// x[9];
/// &x[2..9];
///
/// // Good
/// x[0];
/// x[3];
/// ```
Added `clippy::version` attribute to all normal lints So, some context for this, well, more a story. I'm not used to scripting, I've never really scripted anything, even if it's a valuable skill. I just never really needed it. Now, `@flip1995` correctly suggested using a script for this in `rust-clippy#7813`... And I decided to write a script using nushell because why not? This was a mistake... I spend way more time on this than I would like to admit. It has definitely been more than 4 hours. It shouldn't take that long, but me being new to scripting and nushell just wasn't a good mixture... Anyway, here is the script that creates another script which adds the versions. Fun... Just execute this on the `gh-pages` branch and the resulting `replacer.sh` in `clippy_lints` and it should all work. ```nu mv v0.0.212 rust-1.00.0; mv beta rust-1.57.0; mv master rust-1.58.0; let paths = (open ./rust-1.58.0/lints.json | select id id_span | flatten | select id path); let versions = ( ls | where name =~ "rust-" | select name | format {name}/lints.json | each { open $it | select id | insert version $it | str substring "5,11" version} | group-by id | rotate counter-clockwise id version | update version {get version | first 1} | flatten | select id version); $paths | each { |row| let version = ($versions | where id == ($row.id) | format {version}) let idu = ($row.id | str upcase) $"sed -i '0,/($idu),/{s/pub ($idu),/#[clippy::version = "($version)"]\n pub ($idu),/}' ($row.path)" } | str collect ";" | str find-replace --all '1.00.0' 'pre 1.29.0' | save "replacer.sh"; ``` And this still has some problems, but at this point I just want to be done -.-
2021-10-21 19:06:26 +00:00
#[clippy::version = "pre 1.29.0"]
2015-12-21 18:22:29 +00:00
pub OUT_OF_BOUNDS_INDEXING,
2018-03-28 13:24:26 +00:00
correctness,
"out of bounds constant indexing"
2015-12-21 18:22:29 +00:00
}
2018-03-28 13:24:26 +00:00
declare_clippy_lint! {
/// ### What it does
/// Checks for usage of indexing or slicing. Arrays are special cases, 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,no_run
/// // 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);
/// ```
Added `clippy::version` attribute to all normal lints So, some context for this, well, more a story. I'm not used to scripting, I've never really scripted anything, even if it's a valuable skill. I just never really needed it. Now, `@flip1995` correctly suggested using a script for this in `rust-clippy#7813`... And I decided to write a script using nushell because why not? This was a mistake... I spend way more time on this than I would like to admit. It has definitely been more than 4 hours. It shouldn't take that long, but me being new to scripting and nushell just wasn't a good mixture... Anyway, here is the script that creates another script which adds the versions. Fun... Just execute this on the `gh-pages` branch and the resulting `replacer.sh` in `clippy_lints` and it should all work. ```nu mv v0.0.212 rust-1.00.0; mv beta rust-1.57.0; mv master rust-1.58.0; let paths = (open ./rust-1.58.0/lints.json | select id id_span | flatten | select id path); let versions = ( ls | where name =~ "rust-" | select name | format {name}/lints.json | each { open $it | select id | insert version $it | str substring "5,11" version} | group-by id | rotate counter-clockwise id version | update version {get version | first 1} | flatten | select id version); $paths | each { |row| let version = ($versions | where id == ($row.id) | format {version}) let idu = ($row.id | str upcase) $"sed -i '0,/($idu),/{s/pub ($idu),/#[clippy::version = "($version)"]\n pub ($idu),/}' ($row.path)" } | str collect ";" | str find-replace --all '1.00.0' 'pre 1.29.0' | save "replacer.sh"; ``` And this still has some problems, but at this point I just want to be done -.-
2021-10-21 19:06:26 +00:00
#[clippy::version = "pre 1.29.0"]
pub INDEXING_SLICING,
restriction,
"indexing/slicing usage"
}
2019-04-08 20:43:55 +00:00
declare_lint_pass!(IndexingSlicing => [INDEXING_SLICING, OUT_OF_BOUNDS_INDEXING]);
2015-12-21 18:22:29 +00:00
impl<'tcx> LateLintPass<'tcx> for IndexingSlicing {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if cx.tcx.hir().is_inside_const_context(expr.hir_id) {
return;
}
if let ExprKind::Index(array, index) = &expr.kind {
let ty = cx.typeck_results().expr_ty(array).peel_refs();
2021-08-08 14:49:13 +00:00
if let Some(range) = higher::Range::hir(index) {
2019-01-31 01:15:29 +00:00
// Ranged indexes, i.e., &x[n..m], &x[n..], &x[..n] and &x[..]
if let ty::Array(_, s) = ty.kind() {
2020-03-02 20:53:41 +00:00
let size: u128 = if let Some(size) = s.try_eval_usize(cx.tcx, cx.param_env) {
size.into()
} else {
return;
};
2018-10-14 14:49:28 +00:00
let const_range = to_const_range(cx, range, size);
if let (Some(start), _) = const_range {
if start > size {
2020-02-18 13:28:18 +00:00
span_lint(
2018-10-14 14:49:28 +00:00
cx,
OUT_OF_BOUNDS_INDEXING,
range.start.map_or(expr.span, |start| start.span),
2018-10-14 14:49:28 +00:00
"range is out of bounds",
);
return;
2018-10-14 14:49:28 +00:00
}
}
if let (_, Some(end)) = const_range {
if end > size {
2020-02-18 13:28:18 +00:00
span_lint(
2018-10-14 14:49:28 +00:00
cx,
OUT_OF_BOUNDS_INDEXING,
range.end.map_or(expr.span, |end| end.span),
2018-10-14 14:49:28 +00:00
"range is out of bounds",
);
return;
}
}
if let (Some(_), Some(_)) = const_range {
// early return because both start and end are constants
// and we have proven above that they are in bounds
return;
2017-09-13 13:34:04 +00:00
}
2015-12-21 18:22:29 +00:00
}
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.
};
span_lint_and_help(cx, INDEXING_SLICING, expr.span, "slicing may panic", None, help_msg);
} else {
2019-01-31 01:15:29 +00:00
// Catchall non-range index, i.e., [n] or [n << m]
if let ty::Array(..) = ty.kind() {
// Index is a const block.
if let ExprKind::ConstBlock(..) = index.kind {
return;
}
// Index is a constant uint.
2020-07-17 08:47:04 +00:00
if let Some(..) = constant(cx, cx.typeck_results(), index) {
// Let rustc's `const_err` lint handle constant `usize` indexing on arrays.
return;
}
}
2020-02-18 13:28:18 +00:00
span_lint_and_help(
cx,
INDEXING_SLICING,
expr.span,
"indexing may panic",
None,
"consider using `.get(n)` or `.get_mut(n)` instead",
);
2015-12-21 18:22:29 +00:00
}
}
}
}
/// Returns a tuple of options with the start and end (exclusive) values of
/// the range. If the start or end is not constant, None is returned.
fn to_const_range<'tcx>(
cx: &LateContext<'tcx>,
2020-02-18 13:28:18 +00:00
range: higher::Range<'_>,
Extend `indexing_slicing` lint Hey there clippy team! I've made some assumptions in this PR and I'm not at all certain they'll look like the right approach to you. I'm looking forward to any feedback or revision requests you have, thanks! Prior to this commit the `indexing_slicing` lint was limited to indexing/slicing operations on arrays. This meant that the scope of a really useful lint didn't include vectors. In order to include vectors in the `indexing_slicing` lint a few steps were taken. The `array_indexing.rs` source file in `clippy_lints` was renamed to `indexing_slicing.rs` to more accurately reflect the lint's new scope. The `OUT_OF_BOUNDS_INDEXING` lint persists through these changes so if we can know that a constant index or slice on an array is in bounds no lint is triggered. The `array_indexing` tests in the `tests/ui` directory were also extended and moved to `indexing_slicing.rs` and `indexing_slicing.stderr`. The `indexing_slicing` lint was moved to the `clippy_pedantic` lint group. A specific "Consider using" string was added to each of the `indexing_slicing` lint reports. At least one of the test scenarios might look peculiar and I'll leave it up to y'all to decide if it's palatable. It's the result of indexing the array `x` after `let x = [1, 2, 3, 4];` ``` error: slicing may panic. Consider using `.get(..n)`or `.get_mut(..n)`instead --> $DIR/indexing_slicing.rs:23:6 | 23 | &x[0..][..3]; | ^^^^^^^^^^^ ``` The error string reports only on the second half's range-to, because the range-from is in bounds! Again, thanks for taking a look. Closes #2536
2018-05-23 04:56:02 +00:00
array_size: u128,
) -> (Option<u128>, Option<u128>) {
2020-07-17 08:47:04 +00:00
let s = range
.start
.map(|expr| constant(cx, cx.typeck_results(), expr).map(|(c, _)| c));
let start = match s {
Some(Some(Constant::Int(x))) => Some(x),
Some(_) => None,
None => Some(0),
};
2020-07-17 08:47:04 +00:00
let e = range
.end
.map(|expr| constant(cx, cx.typeck_results(), expr).map(|(c, _)| c));
let end = match e {
2018-11-27 20:14:15 +00:00
Some(Some(Constant::Int(x))) => {
if range.limits == RangeLimits::Closed {
Some(x + 1)
} else {
Some(x)
}
2016-12-20 17:21:30 +00:00
},
Some(_) => None,
None => Some(array_size),
};
(start, end)
}