Skip to content

Commit d13ffbe

Browse files
committed
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`
2 parents 5d8a3e8 + b574941 commit d13ffbe

File tree

7 files changed

+287
-3
lines changed

7 files changed

+287
-3
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1431,6 +1431,7 @@ Released 2018-09-13
14311431
[`map_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
14321432
[`match_as_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_as_ref
14331433
[`match_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_bool
1434+
[`match_on_vec_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_on_vec_items
14341435
[`match_overlapping_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_overlapping_arm
14351436
[`match_ref_pats`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_ref_pats
14361437
[`match_same_arms`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_same_arms

clippy_lints/src/consts.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -358,9 +358,9 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
358358
},
359359
(Some(Constant::Vec(vec)), _) => {
360360
if !vec.is_empty() && vec.iter().all(|x| *x == vec[0]) {
361-
match vec[0] {
362-
Constant::F32(x) => Some(Constant::F32(x)),
363-
Constant::F64(x) => Some(Constant::F64(x)),
361+
match vec.get(0) {
362+
Some(Constant::F32(x)) => Some(Constant::F32(*x)),
363+
Some(Constant::F64(x)) => Some(Constant::F64(*x)),
364364
_ => None,
365365
}
366366
} else {

clippy_lints/src/lib.rs

+5
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,7 @@ mod macro_use;
249249
mod main_recursion;
250250
mod map_clone;
251251
mod map_unit_fn;
252+
mod match_on_vec_items;
252253
mod matches;
253254
mod mem_discriminant;
254255
mod mem_forget;
@@ -630,6 +631,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
630631
&map_clone::MAP_CLONE,
631632
&map_unit_fn::OPTION_MAP_UNIT_FN,
632633
&map_unit_fn::RESULT_MAP_UNIT_FN,
634+
&match_on_vec_items::MATCH_ON_VEC_ITEMS,
633635
&matches::INFALLIBLE_DESTRUCTURING_MATCH,
634636
&matches::MATCH_AS_REF,
635637
&matches::MATCH_BOOL,
@@ -1061,6 +1063,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10611063
store.register_late_pass(|| box future_not_send::FutureNotSend);
10621064
store.register_late_pass(|| box utils::internal_lints::CollapsibleCalls);
10631065
store.register_late_pass(|| box if_let_mutex::IfLetMutex);
1066+
store.register_late_pass(|| box match_on_vec_items::MatchOnVecItems);
10641067

10651068
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
10661069
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1280,6 +1283,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12801283
LintId::of(&map_clone::MAP_CLONE),
12811284
LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN),
12821285
LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN),
1286+
LintId::of(&match_on_vec_items::MATCH_ON_VEC_ITEMS),
12831287
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
12841288
LintId::of(&matches::MATCH_AS_REF),
12851289
LintId::of(&matches::MATCH_OVERLAPPING_ARM),
@@ -1643,6 +1647,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16431647
LintId::of(&loops::NEVER_LOOP),
16441648
LintId::of(&loops::REVERSE_RANGE_LOOP),
16451649
LintId::of(&loops::WHILE_IMMUTABLE_CONDITION),
1650+
LintId::of(&match_on_vec_items::MATCH_ON_VEC_ITEMS),
16461651
LintId::of(&mem_discriminant::MEM_DISCRIMINANT_NON_ENUM),
16471652
LintId::of(&mem_replace::MEM_REPLACE_WITH_UNINIT),
16481653
LintId::of(&methods::CLONE_DOUBLE_REF),
+89
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
use crate::utils::{is_type_diagnostic_item, snippet, span_lint_and_sugg, walk_ptrs_ty};
2+
use if_chain::if_chain;
3+
use rustc_errors::Applicability;
4+
use rustc_hir::{Expr, ExprKind, MatchSource};
5+
use rustc_lint::{LateContext, LateLintPass, LintContext};
6+
use rustc_middle::lint::in_external_macro;
7+
use rustc_session::{declare_lint_pass, declare_tool_lint};
8+
9+
declare_clippy_lint! {
10+
/// **What it does:** Checks for `match vec[idx]` or `match vec[n..m]`.
11+
///
12+
/// **Why is this bad?** This can panic at runtime.
13+
///
14+
/// **Known problems:** None.
15+
///
16+
/// **Example:**
17+
/// ```rust, no_run
18+
/// let arr = vec![0, 1, 2, 3];
19+
/// let idx = 1;
20+
///
21+
/// // Bad
22+
/// match arr[idx] {
23+
/// 0 => println!("{}", 0),
24+
/// 1 => println!("{}", 3),
25+
/// _ => {},
26+
/// }
27+
/// ```
28+
/// Use instead:
29+
/// ```rust, no_run
30+
/// let arr = vec![0, 1, 2, 3];
31+
/// let idx = 1;
32+
///
33+
/// // Good
34+
/// match arr.get(idx) {
35+
/// Some(0) => println!("{}", 0),
36+
/// Some(1) => println!("{}", 3),
37+
/// _ => {},
38+
/// }
39+
/// ```
40+
pub MATCH_ON_VEC_ITEMS,
41+
correctness,
42+
"matching on vector elements can panic"
43+
}
44+
45+
declare_lint_pass!(MatchOnVecItems => [MATCH_ON_VEC_ITEMS]);
46+
47+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MatchOnVecItems {
48+
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) {
49+
if_chain! {
50+
if !in_external_macro(cx.sess(), expr.span);
51+
if let ExprKind::Match(ref match_expr, _, MatchSource::Normal) = expr.kind;
52+
if let Some(idx_expr) = is_vec_indexing(cx, match_expr);
53+
if let ExprKind::Index(vec, idx) = idx_expr.kind;
54+
55+
then {
56+
// FIXME: could be improved to suggest surrounding every pattern with Some(_),
57+
// but only when `or_patterns` are stabilized.
58+
span_lint_and_sugg(
59+
cx,
60+
MATCH_ON_VEC_ITEMS,
61+
match_expr.span,
62+
"indexing into a vector may panic",
63+
"try this",
64+
format!(
65+
"{}.get({})",
66+
snippet(cx, vec.span, ".."),
67+
snippet(cx, idx.span, "..")
68+
),
69+
Applicability::MaybeIncorrect
70+
);
71+
}
72+
}
73+
}
74+
}
75+
76+
fn is_vec_indexing<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
77+
if_chain! {
78+
if let ExprKind::Index(ref array, _) = expr.kind;
79+
let ty = cx.tables.expr_ty(array);
80+
let ty = walk_ptrs_ty(ty);
81+
if is_type_diagnostic_item(cx, ty, sym!(vec_type));
82+
83+
then {
84+
return Some(expr);
85+
}
86+
}
87+
88+
None
89+
}

src/lintlist/mod.rs

+7
Original file line numberDiff line numberDiff line change
@@ -1144,6 +1144,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
11441144
deprecation: None,
11451145
module: "matches",
11461146
},
1147+
Lint {
1148+
name: "match_on_vec_items",
1149+
group: "correctness",
1150+
desc: "matching on vector elements can panic",
1151+
deprecation: None,
1152+
module: "match_on_vec_items",
1153+
},
11471154
Lint {
11481155
name: "match_overlapping_arm",
11491156
group: "style",

tests/ui/match_on_vec_items.rs

+130
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
#![warn(clippy::match_on_vec_items)]
2+
3+
fn match_with_wildcard() {
4+
let arr = vec![0, 1, 2, 3];
5+
let range = 1..3;
6+
let idx = 1;
7+
8+
// Lint, may panic
9+
match arr[idx] {
10+
0 => println!("0"),
11+
1 => println!("1"),
12+
_ => {},
13+
}
14+
15+
// Lint, may panic
16+
match arr[range] {
17+
[0, 1] => println!("0 1"),
18+
[1, 2] => println!("1 2"),
19+
_ => {},
20+
}
21+
}
22+
23+
fn match_without_wildcard() {
24+
let arr = vec![0, 1, 2, 3];
25+
let range = 1..3;
26+
let idx = 2;
27+
28+
// Lint, may panic
29+
match arr[idx] {
30+
0 => println!("0"),
31+
1 => println!("1"),
32+
num => {},
33+
}
34+
35+
// Lint, may panic
36+
match arr[range] {
37+
[0, 1] => println!("0 1"),
38+
[1, 2] => println!("1 2"),
39+
[ref sub @ ..] => {},
40+
}
41+
}
42+
43+
fn match_wildcard_and_action() {
44+
let arr = vec![0, 1, 2, 3];
45+
let range = 1..3;
46+
let idx = 3;
47+
48+
// Lint, may panic
49+
match arr[idx] {
50+
0 => println!("0"),
51+
1 => println!("1"),
52+
_ => println!("Hello, World!"),
53+
}
54+
55+
// Lint, may panic
56+
match arr[range] {
57+
[0, 1] => println!("0 1"),
58+
[1, 2] => println!("1 2"),
59+
_ => println!("Hello, World!"),
60+
}
61+
}
62+
63+
fn match_vec_ref() {
64+
let arr = &vec![0, 1, 2, 3];
65+
let range = 1..3;
66+
let idx = 3;
67+
68+
// Lint, may panic
69+
match arr[idx] {
70+
0 => println!("0"),
71+
1 => println!("1"),
72+
_ => {},
73+
}
74+
75+
// Lint, may panic
76+
match arr[range] {
77+
[0, 1] => println!("0 1"),
78+
[1, 2] => println!("1 2"),
79+
_ => {},
80+
}
81+
}
82+
83+
fn match_with_get() {
84+
let arr = vec![0, 1, 2, 3];
85+
let range = 1..3;
86+
let idx = 3;
87+
88+
// Ok
89+
match arr.get(idx) {
90+
Some(0) => println!("0"),
91+
Some(1) => println!("1"),
92+
_ => {},
93+
}
94+
95+
// Ok
96+
match arr.get(range) {
97+
Some(&[0, 1]) => println!("0 1"),
98+
Some(&[1, 2]) => println!("1 2"),
99+
_ => {},
100+
}
101+
}
102+
103+
fn match_with_array() {
104+
let arr = [0, 1, 2, 3];
105+
let range = 1..3;
106+
let idx = 3;
107+
108+
// Ok
109+
match arr[idx] {
110+
0 => println!("0"),
111+
1 => println!("1"),
112+
_ => {},
113+
}
114+
115+
// Ok
116+
match arr[range] {
117+
[0, 1] => println!("0 1"),
118+
[1, 2] => println!("1 2"),
119+
_ => {},
120+
}
121+
}
122+
123+
fn main() {
124+
match_with_wildcard();
125+
match_without_wildcard();
126+
match_wildcard_and_action();
127+
match_vec_ref();
128+
match_with_get();
129+
match_with_array();
130+
}

tests/ui/match_on_vec_items.stderr

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
error: indexing into a vector may panic
2+
--> $DIR/match_on_vec_items.rs:9:11
3+
|
4+
LL | match arr[idx] {
5+
| ^^^^^^^^ help: try this: `arr.get(idx)`
6+
|
7+
= note: `-D clippy::match-on-vec-items` implied by `-D warnings`
8+
9+
error: indexing into a vector may panic
10+
--> $DIR/match_on_vec_items.rs:16:11
11+
|
12+
LL | match arr[range] {
13+
| ^^^^^^^^^^ help: try this: `arr.get(range)`
14+
15+
error: indexing into a vector may panic
16+
--> $DIR/match_on_vec_items.rs:29:11
17+
|
18+
LL | match arr[idx] {
19+
| ^^^^^^^^ help: try this: `arr.get(idx)`
20+
21+
error: indexing into a vector may panic
22+
--> $DIR/match_on_vec_items.rs:36:11
23+
|
24+
LL | match arr[range] {
25+
| ^^^^^^^^^^ help: try this: `arr.get(range)`
26+
27+
error: indexing into a vector may panic
28+
--> $DIR/match_on_vec_items.rs:49:11
29+
|
30+
LL | match arr[idx] {
31+
| ^^^^^^^^ help: try this: `arr.get(idx)`
32+
33+
error: indexing into a vector may panic
34+
--> $DIR/match_on_vec_items.rs:56:11
35+
|
36+
LL | match arr[range] {
37+
| ^^^^^^^^^^ help: try this: `arr.get(range)`
38+
39+
error: indexing into a vector may panic
40+
--> $DIR/match_on_vec_items.rs:69:11
41+
|
42+
LL | match arr[idx] {
43+
| ^^^^^^^^ help: try this: `arr.get(idx)`
44+
45+
error: indexing into a vector may panic
46+
--> $DIR/match_on_vec_items.rs:76:11
47+
|
48+
LL | match arr[range] {
49+
| ^^^^^^^^^^ help: try this: `arr.get(range)`
50+
51+
error: aborting due to 8 previous errors
52+

0 commit comments

Comments
 (0)