dnrops
/
rust-clippy
mirror of https://github.com/rust-lang/rust-clippy.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Auto merge of #5522 - CrazyRoka:match_vec_item, r=phansch 

New  lint `match_vec_item`

Added new lint to warn a match on index item which can panic. It's always better to use `get(..)` instead.
Closes #5500
changelog: New lint `match_on_vec_items`
This commit is contained in:
bors 2020-04-27 06:02:05 +00:00
parent 5d8a3e8724 b574941dcb
commit d13ffbe3fe
7 changed files with 287 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -1431,6 +1431,7 @@ Released 2018-09-13
[`map_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
[`match_as_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_as_ref
[`match_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_bool
[`match_on_vec_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_on_vec_items
[`match_overlapping_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_overlapping_arm
[`match_ref_pats`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_ref_pats
[`match_same_arms`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_same_arms

6
clippy_lints/src/consts.rs
View File

@ -358,9 +358,9 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
},
(Some(Constant::Vec(vec)), _) => {
if !vec.is_empty() && vec.iter().all(|x| *x == vec[0]) {
match vec[0] {
Constant::F32(x) => Some(Constant::F32(x)),
Constant::F64(x) => Some(Constant::F64(x)),
match vec.get(0) {
Some(Constant::F32(x)) => Some(Constant::F32(*x)),
Some(Constant::F64(x)) => Some(Constant::F64(*x)),
_ => None,
}
} else {

5
clippy_lints/src/lib.rs
View File

@ -249,6 +249,7 @@ mod macro_use;
mod main_recursion;
mod map_clone;
mod map_unit_fn;
mod match_on_vec_items;
mod matches;
mod mem_discriminant;
mod mem_forget;
@ -630,6 +631,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&map_clone::MAP_CLONE,
&map_unit_fn::OPTION_MAP_UNIT_FN,
&map_unit_fn::RESULT_MAP_UNIT_FN,
&match_on_vec_items::MATCH_ON_VEC_ITEMS,
&matches::INFALLIBLE_DESTRUCTURING_MATCH,
&matches::MATCH_AS_REF,
&matches::MATCH_BOOL,
@ -1061,6 +1063,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box future_not_send::FutureNotSend);
store.register_late_pass(|| box utils::internal_lints::CollapsibleCalls);
store.register_late_pass(|| box if_let_mutex::IfLetMutex);
store.register_late_pass(|| box match_on_vec_items::MatchOnVecItems);
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@ -1280,6 +1283,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&map_clone::MAP_CLONE),
LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN),
LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN),
LintId::of(&match_on_vec_items::MATCH_ON_VEC_ITEMS),
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
LintId::of(&matches::MATCH_AS_REF),
LintId::of(&matches::MATCH_OVERLAPPING_ARM),
@ -1643,6 +1647,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&loops::NEVER_LOOP),
LintId::of(&loops::REVERSE_RANGE_LOOP),
LintId::of(&loops::WHILE_IMMUTABLE_CONDITION),
LintId::of(&match_on_vec_items::MATCH_ON_VEC_ITEMS),
LintId::of(&mem_discriminant::MEM_DISCRIMINANT_NON_ENUM),
LintId::of(&mem_replace::MEM_REPLACE_WITH_UNINIT),
LintId::of(&methods::CLONE_DOUBLE_REF),

89
clippy_lints/src/match_on_vec_items.rs Normal file
View File

@ -0,0 +1,89 @@
use crate::utils::{is_type_diagnostic_item, snippet, span_lint_and_sugg, walk_ptrs_ty};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, MatchSource};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
declare_clippy_lint! {
/// **What it does:** Checks for `match vec[idx]` or `match vec[n..m]`.
///
/// **Why is this bad?** This can panic at runtime.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust, no_run
/// let arr = vec![0, 1, 2, 3];
/// let idx = 1;
///
/// // Bad
/// match arr[idx] {
/// 0 => println!("{}", 0),
/// 1 => println!("{}", 3),
/// _ => {},
/// }
/// ```
/// Use instead:
/// ```rust, no_run
/// let arr = vec![0, 1, 2, 3];
/// let idx = 1;
///
/// // Good
/// match arr.get(idx) {
/// Some(0) => println!("{}", 0),
/// Some(1) => println!("{}", 3),
/// _ => {},
/// }
/// ```
pub MATCH_ON_VEC_ITEMS,
correctness,
"matching on vector elements can panic"
}
declare_lint_pass!(MatchOnVecItems => [MATCH_ON_VEC_ITEMS]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MatchOnVecItems {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) {
if_chain! {
if !in_external_macro(cx.sess(), expr.span);
if let ExprKind::Match(ref match_expr, _, MatchSource::Normal) = expr.kind;
if let Some(idx_expr) = is_vec_indexing(cx, match_expr);
if let ExprKind::Index(vec, idx) = idx_expr.kind;
then {
// FIXME: could be improved to suggest surrounding every pattern with Some(_),
// but only when `or_patterns` are stabilized.
span_lint_and_sugg(
cx,
MATCH_ON_VEC_ITEMS,
match_expr.span,
"indexing into a vector may panic",
"try this",
format!(
"{}.get({})",
snippet(cx, vec.span, ".."),
snippet(cx, idx.span, "..")
),
Applicability::MaybeIncorrect
);
}
}
}
}
fn is_vec_indexing<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
if_chain! {
if let ExprKind::Index(ref array, _) = expr.kind;
let ty = cx.tables.expr_ty(array);
let ty = walk_ptrs_ty(ty);
if is_type_diagnostic_item(cx, ty, sym!(vec_type));
then {
return Some(expr);
}
}
None
}

7
src/lintlist/mod.rs
View File

@ -1144,6 +1144,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None,
module: "matches",
},
Lint {
name: "match_on_vec_items",
group: "correctness",
desc: "matching on vector elements can panic",
deprecation: None,
module: "match_on_vec_items",
},
Lint {
name: "match_overlapping_arm",
group: "style",

130
tests/ui/match_on_vec_items.rs Normal file
View File

@ -0,0 +1,130 @@
#![warn(clippy::match_on_vec_items)]
fn match_with_wildcard() {
let arr = vec![0, 1, 2, 3];
let range = 1..3;
let idx = 1;
// Lint, may panic
match arr[idx] {
0 => println!("0"),
1 => println!("1"),
_ => {},
}
// Lint, may panic
match arr[range] {
[0, 1] => println!("0 1"),
[1, 2] => println!("1 2"),
_ => {},
}
}
fn match_without_wildcard() {
let arr = vec![0, 1, 2, 3];
let range = 1..3;
let idx = 2;
// Lint, may panic
match arr[idx] {
0 => println!("0"),
1 => println!("1"),
num => {},
}
// Lint, may panic
match arr[range] {
[0, 1] => println!("0 1"),
[1, 2] => println!("1 2"),
[ref sub @ ..] => {},
}
}
fn match_wildcard_and_action() {
let arr = vec![0, 1, 2, 3];
let range = 1..3;
let idx = 3;
// Lint, may panic
match arr[idx] {
0 => println!("0"),
1 => println!("1"),
_ => println!("Hello, World!"),
}
// Lint, may panic
match arr[range] {
[0, 1] => println!("0 1"),
[1, 2] => println!("1 2"),
_ => println!("Hello, World!"),
}
}
fn match_vec_ref() {
let arr = &vec![0, 1, 2, 3];
let range = 1..3;
let idx = 3;
// Lint, may panic
match arr[idx] {
0 => println!("0"),
1 => println!("1"),
_ => {},
}
// Lint, may panic
match arr[range] {
[0, 1] => println!("0 1"),
[1, 2] => println!("1 2"),
_ => {},
}
}
fn match_with_get() {
let arr = vec![0, 1, 2, 3];
let range = 1..3;
let idx = 3;
// Ok
match arr.get(idx) {
Some(0) => println!("0"),
Some(1) => println!("1"),
_ => {},
}
// Ok
match arr.get(range) {
Some(&[0, 1]) => println!("0 1"),
Some(&[1, 2]) => println!("1 2"),
_ => {},
}
}
fn match_with_array() {
let arr = [0, 1, 2, 3];
let range = 1..3;
let idx = 3;
// Ok
match arr[idx] {
0 => println!("0"),
1 => println!("1"),
_ => {},
}
// Ok
match arr[range] {
[0, 1] => println!("0 1"),
[1, 2] => println!("1 2"),
_ => {},
}
}
fn main() {
match_with_wildcard();
match_without_wildcard();
match_wildcard_and_action();
match_vec_ref();
match_with_get();
match_with_array();
}

52
tests/ui/match_on_vec_items.stderr Normal file
View File

@ -0,0 +1,52 @@
error: indexing into a vector may panic
--> $DIR/match_on_vec_items.rs:9:11
|
LL | match arr[idx] {
| ^^^^^^^^ help: try this: `arr.get(idx)`
|
= note: `-D clippy::match-on-vec-items` implied by `-D warnings`
error: indexing into a vector may panic
--> $DIR/match_on_vec_items.rs:16:11
|
LL | match arr[range] {
| ^^^^^^^^^^ help: try this: `arr.get(range)`
error: indexing into a vector may panic
--> $DIR/match_on_vec_items.rs:29:11
|
LL | match arr[idx] {
| ^^^^^^^^ help: try this: `arr.get(idx)`
error: indexing into a vector may panic
--> $DIR/match_on_vec_items.rs:36:11
|
LL | match arr[range] {
| ^^^^^^^^^^ help: try this: `arr.get(range)`
error: indexing into a vector may panic
--> $DIR/match_on_vec_items.rs:49:11
|
LL | match arr[idx] {
| ^^^^^^^^ help: try this: `arr.get(idx)`
error: indexing into a vector may panic
--> $DIR/match_on_vec_items.rs:56:11
|
LL | match arr[range] {
| ^^^^^^^^^^ help: try this: `arr.get(range)`
error: indexing into a vector may panic
--> $DIR/match_on_vec_items.rs:69:11
|
LL | match arr[idx] {
| ^^^^^^^^ help: try this: `arr.get(idx)`
error: indexing into a vector may panic
--> $DIR/match_on_vec_items.rs:76:11
|
LL | match arr[range] {
| ^^^^^^^^^^ help: try this: `arr.get(range)`
error: aborting due to 8 previous errors