From 96e2bc80f556e54b95ae224dc2f59cfa47397a38 Mon Sep 17 00:00:00 2001 From: CrazyRoka Date: Fri, 24 Apr 2020 00:28:18 +0300 Subject: [PATCH 1/5] Added lint `match_vec_item` --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 5 ++ clippy_lints/src/match_vec_item.rs | 111 +++++++++++++++++++++++++++++ src/lintlist/mod.rs | 7 ++ tests/ui/match_vec_item.rs | 109 ++++++++++++++++++++++++++++ tests/ui/match_vec_item.stderr | 27 +++++++ 6 files changed, 260 insertions(+) create mode 100644 clippy_lints/src/match_vec_item.rs create mode 100644 tests/ui/match_vec_item.rs create mode 100644 tests/ui/match_vec_item.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index abd7167502b9c..3ba2c51623b26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1352,6 +1352,7 @@ Released 2018-09-13 [`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 [`match_single_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding +[`match_vec_item`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_vec_item [`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm [`maybe_infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#maybe_infinite_iter [`mem_discriminant_non_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index dee4188b75f38..79bf13a100540 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -249,6 +249,7 @@ mod macro_use; mod main_recursion; mod map_clone; mod map_unit_fn; +mod match_vec_item; mod matches; mod mem_discriminant; mod mem_forget; @@ -629,6 +630,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_vec_item::MATCH_VEC_ITEM, &matches::INFALLIBLE_DESTRUCTURING_MATCH, &matches::MATCH_AS_REF, &matches::MATCH_BOOL, @@ -1060,6 +1062,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_vec_item::MatchVecItem); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(&arithmetic::FLOAT_ARITHMETIC), @@ -1277,6 +1280,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_vec_item::MATCH_VEC_ITEM), LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH), LintId::of(&matches::MATCH_AS_REF), LintId::of(&matches::MATCH_BOOL), @@ -1469,6 +1473,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&loops::WHILE_LET_ON_ITERATOR), LintId::of(&main_recursion::MAIN_RECURSION), LintId::of(&map_clone::MAP_CLONE), + LintId::of(&match_vec_item::MATCH_VEC_ITEM), LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH), LintId::of(&matches::MATCH_BOOL), LintId::of(&matches::MATCH_OVERLAPPING_ARM), diff --git a/clippy_lints/src/match_vec_item.rs b/clippy_lints/src/match_vec_item.rs new file mode 100644 index 0000000000000..7ea5a24abbd56 --- /dev/null +++ b/clippy_lints/src/match_vec_item.rs @@ -0,0 +1,111 @@ +use crate::utils::{is_wild, span_lint_and_help}; +use if_chain::if_chain; +use rustc_hir::{Arm, Expr, ExprKind, MatchSource}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_middle::ty::{self, AdtDef}; +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_VEC_ITEM, + style, + "match vector by indexing can panic" +} + +declare_lint_pass!(MatchVecItem => [MATCH_VEC_ITEM]); + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MatchVecItem { + 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 ex, ref arms, MatchSource::Normal) = expr.kind; + if contains_wild_arm(arms); + if is_vec_indexing(cx, ex); + + then { + span_lint_and_help( + cx, + MATCH_VEC_ITEM, + expr.span, + "indexing vector may panic", + None, + "consider using `get(..)` instead.", + ); + } + } + } +} + +fn contains_wild_arm(arms: &[Arm<'_>]) -> bool { + arms.iter().any(|arm| is_wild(&arm.pat) && is_unit_expr(arm.body)) +} + +fn is_vec_indexing<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) -> bool { + if_chain! { + if let ExprKind::Index(ref array, _) = expr.kind; + let ty = cx.tables.expr_ty(array); + if let ty::Adt(def, _) = ty.kind; + if is_vec(cx, def); + + then { + return true; + } + } + + false +} + +fn is_vec<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, def: &'a AdtDef) -> bool { + if_chain! { + let def_path = cx.tcx.def_path(def.did); + if def_path.data.len() == 2; + if let Some(module) = def_path.data.get(0); + if module.data.as_symbol() == sym!(vec); + if let Some(name) = def_path.data.get(1); + if name.data.as_symbol() == sym!(Vec); + + then { + return true; + } + } + + false +} + +fn is_unit_expr(expr: &Expr<'_>) -> bool { + match expr.kind { + ExprKind::Tup(ref v) if v.is_empty() => true, + ExprKind::Block(ref b, _) if b.stmts.is_empty() && b.expr.is_none() => true, + _ => false, + } +} diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 2c466aa20c675..7c9e044895011 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -1172,6 +1172,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "matches", }, + Lint { + name: "match_vec_item", + group: "style", + desc: "match vector by indexing can panic", + deprecation: None, + module: "match_vec_item", + }, Lint { name: "match_wild_err_arm", group: "style", diff --git a/tests/ui/match_vec_item.rs b/tests/ui/match_vec_item.rs new file mode 100644 index 0000000000000..8e7c098d75c85 --- /dev/null +++ b/tests/ui/match_vec_item.rs @@ -0,0 +1,109 @@ +#![warn(clippy::match_vec_item)] + +fn main() { + match_with_wildcard(); + match_without_wildcard(); + match_wildcard_and_action(); + match_with_get(); + match_with_array(); +} + +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; + + // Ok + match arr[idx] { + 0 => println!("0"), + 1 => println!("1"), + num => {}, + } + + // Ok + 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; + + // Ok + match arr[idx] { + 0 => println!("0"), + 1 => println!("1"), + _ => println!("Hello, World!"), + } + + // Ok + match arr[range] { + [0, 1] => println!("0 1"), + [1, 2] => println!("1 2"), + _ => println!("Hello, World!"), + } +} + +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"), + _ => {}, + } +} diff --git a/tests/ui/match_vec_item.stderr b/tests/ui/match_vec_item.stderr new file mode 100644 index 0000000000000..2494c3143f862 --- /dev/null +++ b/tests/ui/match_vec_item.stderr @@ -0,0 +1,27 @@ +error: indexing vector may panic + --> $DIR/match_vec_item.rs:17:5 + | +LL | / match arr[idx] { +LL | | 0 => println!("0"), +LL | | 1 => println!("1"), +LL | | _ => {}, +LL | | } + | |_____^ + | + = note: `-D clippy::match-vec-item` implied by `-D warnings` + = help: consider using `get(..)` instead. + +error: indexing vector may panic + --> $DIR/match_vec_item.rs:24:5 + | +LL | / match arr[range] { +LL | | [0, 1] => println!("0 1"), +LL | | [1, 2] => println!("1 2"), +LL | | _ => {}, +LL | | } + | |_____^ + | + = help: consider using `get(..)` instead. + +error: aborting due to 2 previous errors + From b0115fb996b8cee2202ca379efd209d74ce0f751 Mon Sep 17 00:00:00 2001 From: CrazyRoka Date: Sat, 25 Apr 2020 00:52:02 +0300 Subject: [PATCH 2/5] Removed unnecessary code, added support for vector references --- clippy_lints/src/consts.rs | 6 +-- clippy_lints/src/match_vec_item.rs | 67 ++++++++++-------------------- tests/ui/match_vec_item.rs | 29 +++++++++++-- tests/ui/match_vec_item.stderr | 63 +++++++++++++++++++--------- 4 files changed, 94 insertions(+), 71 deletions(-) diff --git a/clippy_lints/src/consts.rs b/clippy_lints/src/consts.rs index 7916996e990dc..81ddc8c0067c7 100644 --- a/clippy_lints/src/consts.rs +++ b/clippy_lints/src/consts.rs @@ -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 { diff --git a/clippy_lints/src/match_vec_item.rs b/clippy_lints/src/match_vec_item.rs index 7ea5a24abbd56..07121dedefe29 100644 --- a/clippy_lints/src/match_vec_item.rs +++ b/clippy_lints/src/match_vec_item.rs @@ -1,9 +1,9 @@ -use crate::utils::{is_wild, span_lint_and_help}; +use crate::utils::{is_type_diagnostic_item, snippet_with_applicability, span_lint_and_sugg, walk_ptrs_ty}; use if_chain::if_chain; -use rustc_hir::{Arm, Expr, ExprKind, MatchSource}; +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_middle::ty::{self, AdtDef}; use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { @@ -48,64 +48,41 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MatchVecItem { 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 ex, ref arms, MatchSource::Normal) = expr.kind; - if contains_wild_arm(arms); - if is_vec_indexing(cx, ex); + 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 { - span_lint_and_help( + let mut applicability = Applicability::MaybeIncorrect; + span_lint_and_sugg( cx, MATCH_VEC_ITEM, - expr.span, - "indexing vector may panic", - None, - "consider using `get(..)` instead.", + match_expr.span, + "indexing vector may panic. Consider using `get`", + "try this", + format!( + "{}.get({})", + snippet_with_applicability(cx, vec.span, "..", &mut applicability), + snippet_with_applicability(cx, idx.span, "..", &mut applicability) + ), + applicability ); } } } } -fn contains_wild_arm(arms: &[Arm<'_>]) -> bool { - arms.iter().any(|arm| is_wild(&arm.pat) && is_unit_expr(arm.body)) -} - -fn is_vec_indexing<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) -> bool { +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); - if let ty::Adt(def, _) = ty.kind; - if is_vec(cx, def); + let ty = walk_ptrs_ty(ty); + if is_type_diagnostic_item(cx, ty, sym!(vec_type)); then { - return true; + return Some(expr); } } - false -} - -fn is_vec<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, def: &'a AdtDef) -> bool { - if_chain! { - let def_path = cx.tcx.def_path(def.did); - if def_path.data.len() == 2; - if let Some(module) = def_path.data.get(0); - if module.data.as_symbol() == sym!(vec); - if let Some(name) = def_path.data.get(1); - if name.data.as_symbol() == sym!(Vec); - - then { - return true; - } - } - - false -} - -fn is_unit_expr(expr: &Expr<'_>) -> bool { - match expr.kind { - ExprKind::Tup(ref v) if v.is_empty() => true, - ExprKind::Block(ref b, _) if b.stmts.is_empty() && b.expr.is_none() => true, - _ => false, - } + None } diff --git a/tests/ui/match_vec_item.rs b/tests/ui/match_vec_item.rs index 8e7c098d75c85..d00d0bc2fe530 100644 --- a/tests/ui/match_vec_item.rs +++ b/tests/ui/match_vec_item.rs @@ -4,6 +4,7 @@ fn main() { match_with_wildcard(); match_without_wildcard(); match_wildcard_and_action(); + match_vec_ref(); match_with_get(); match_with_array(); } @@ -33,14 +34,14 @@ fn match_without_wildcard() { let range = 1..3; let idx = 2; - // Ok + // Lint, may panic match arr[idx] { 0 => println!("0"), 1 => println!("1"), num => {}, } - // Ok + // Lint, may panic match arr[range] { [0, 1] => println!("0 1"), [1, 2] => println!("1 2"), @@ -53,14 +54,14 @@ fn match_wildcard_and_action() { let range = 1..3; let idx = 3; - // Ok + // Lint, may panic match arr[idx] { 0 => println!("0"), 1 => println!("1"), _ => println!("Hello, World!"), } - // Ok + // Lint, may panic match arr[range] { [0, 1] => println!("0 1"), [1, 2] => println!("1 2"), @@ -68,6 +69,26 @@ fn match_wildcard_and_action() { } } +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; diff --git a/tests/ui/match_vec_item.stderr b/tests/ui/match_vec_item.stderr index 2494c3143f862..2da2eaeee9b11 100644 --- a/tests/ui/match_vec_item.stderr +++ b/tests/ui/match_vec_item.stderr @@ -1,27 +1,52 @@ -error: indexing vector may panic - --> $DIR/match_vec_item.rs:17:5 +error: indexing vector may panic. Consider using `get` + --> $DIR/match_vec_item.rs:18:11 | -LL | / match arr[idx] { -LL | | 0 => println!("0"), -LL | | 1 => println!("1"), -LL | | _ => {}, -LL | | } - | |_____^ +LL | match arr[idx] { + | ^^^^^^^^ help: try this: `arr.get(idx)` | = note: `-D clippy::match-vec-item` implied by `-D warnings` - = help: consider using `get(..)` instead. -error: indexing vector may panic - --> $DIR/match_vec_item.rs:24:5 +error: indexing vector may panic. Consider using `get` + --> $DIR/match_vec_item.rs:25:11 | -LL | / match arr[range] { -LL | | [0, 1] => println!("0 1"), -LL | | [1, 2] => println!("1 2"), -LL | | _ => {}, -LL | | } - | |_____^ +LL | match arr[range] { + | ^^^^^^^^^^ help: try this: `arr.get(range)` + +error: indexing vector may panic. Consider using `get` + --> $DIR/match_vec_item.rs:38:11 + | +LL | match arr[idx] { + | ^^^^^^^^ help: try this: `arr.get(idx)` + +error: indexing vector may panic. Consider using `get` + --> $DIR/match_vec_item.rs:45:11 + | +LL | match arr[range] { + | ^^^^^^^^^^ help: try this: `arr.get(range)` + +error: indexing vector may panic. Consider using `get` + --> $DIR/match_vec_item.rs:58:11 + | +LL | match arr[idx] { + | ^^^^^^^^ help: try this: `arr.get(idx)` + +error: indexing vector may panic. Consider using `get` + --> $DIR/match_vec_item.rs:65:11 + | +LL | match arr[range] { + | ^^^^^^^^^^ help: try this: `arr.get(range)` + +error: indexing vector may panic. Consider using `get` + --> $DIR/match_vec_item.rs:78:11 + | +LL | match arr[idx] { + | ^^^^^^^^ help: try this: `arr.get(idx)` + +error: indexing vector may panic. Consider using `get` + --> $DIR/match_vec_item.rs:85:11 | - = help: consider using `get(..)` instead. +LL | match arr[range] { + | ^^^^^^^^^^ help: try this: `arr.get(range)` -error: aborting due to 2 previous errors +error: aborting due to 8 previous errors From 63b451ea25c7797530c84b82781e0800c9bda68d Mon Sep 17 00:00:00 2001 From: CrazyRoka Date: Sat, 25 Apr 2020 11:33:40 +0300 Subject: [PATCH 3/5] Renamed lint to `match_on_vec_items` --- CHANGELOG.md | 2 +- clippy_lints/src/lib.rs | 10 +++++----- ...match_vec_item.rs => match_on_vec_items.rs} | 10 +++++----- src/lintlist/mod.rs | 14 +++++++------- ...match_vec_item.rs => match_on_vec_items.rs} | 2 +- ...c_item.stderr => match_on_vec_items.stderr} | 18 +++++++++--------- 6 files changed, 28 insertions(+), 28 deletions(-) rename clippy_lints/src/{match_vec_item.rs => match_on_vec_items.rs} (91%) rename tests/ui/{match_vec_item.rs => match_on_vec_items.rs} (98%) rename tests/ui/{match_vec_item.stderr => match_on_vec_items.stderr} (76%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ba2c51623b26..f49980f712b6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1348,11 +1348,11 @@ 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 [`match_single_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding -[`match_vec_item`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_vec_item [`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm [`maybe_infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#maybe_infinite_iter [`mem_discriminant_non_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 79bf13a100540..a72fad8622754 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -249,7 +249,7 @@ mod macro_use; mod main_recursion; mod map_clone; mod map_unit_fn; -mod match_vec_item; +mod match_on_vec_items; mod matches; mod mem_discriminant; mod mem_forget; @@ -630,7 +630,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_vec_item::MATCH_VEC_ITEM, + &match_on_vec_items::MATCH_ON_VEC_ITEMS, &matches::INFALLIBLE_DESTRUCTURING_MATCH, &matches::MATCH_AS_REF, &matches::MATCH_BOOL, @@ -1062,7 +1062,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_vec_item::MatchVecItem); + 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,7 +1280,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_vec_item::MATCH_VEC_ITEM), + 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_BOOL), @@ -1473,7 +1473,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&loops::WHILE_LET_ON_ITERATOR), LintId::of(&main_recursion::MAIN_RECURSION), LintId::of(&map_clone::MAP_CLONE), - LintId::of(&match_vec_item::MATCH_VEC_ITEM), + LintId::of(&match_on_vec_items::MATCH_ON_VEC_ITEMS), LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH), LintId::of(&matches::MATCH_BOOL), LintId::of(&matches::MATCH_OVERLAPPING_ARM), diff --git a/clippy_lints/src/match_vec_item.rs b/clippy_lints/src/match_on_vec_items.rs similarity index 91% rename from clippy_lints/src/match_vec_item.rs rename to clippy_lints/src/match_on_vec_items.rs index 07121dedefe29..167b4abd93d1b 100644 --- a/clippy_lints/src/match_vec_item.rs +++ b/clippy_lints/src/match_on_vec_items.rs @@ -37,14 +37,14 @@ declare_clippy_lint! { /// _ => {}, /// } /// ``` - pub MATCH_VEC_ITEM, + pub MATCH_ON_VEC_ITEMS, style, - "match vector by indexing can panic" + "matching on vector elements can panic" } -declare_lint_pass!(MatchVecItem => [MATCH_VEC_ITEM]); +declare_lint_pass!(MatchOnVecItems => [MATCH_ON_VEC_ITEMS]); -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MatchVecItem { +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); @@ -56,7 +56,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MatchVecItem { let mut applicability = Applicability::MaybeIncorrect; span_lint_and_sugg( cx, - MATCH_VEC_ITEM, + MATCH_ON_VEC_ITEMS, match_expr.span, "indexing vector may panic. Consider using `get`", "try this", diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 7c9e044895011..17f944112989f 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -1144,6 +1144,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "matches", }, + Lint { + name: "match_on_vec_items", + group: "style", + desc: "matching on vector elements can panic", + deprecation: None, + module: "match_on_vec_items", + }, Lint { name: "match_overlapping_arm", group: "style", @@ -1172,13 +1179,6 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "matches", }, - Lint { - name: "match_vec_item", - group: "style", - desc: "match vector by indexing can panic", - deprecation: None, - module: "match_vec_item", - }, Lint { name: "match_wild_err_arm", group: "style", diff --git a/tests/ui/match_vec_item.rs b/tests/ui/match_on_vec_items.rs similarity index 98% rename from tests/ui/match_vec_item.rs rename to tests/ui/match_on_vec_items.rs index d00d0bc2fe530..02b6da0343914 100644 --- a/tests/ui/match_vec_item.rs +++ b/tests/ui/match_on_vec_items.rs @@ -1,4 +1,4 @@ -#![warn(clippy::match_vec_item)] +#![warn(clippy::match_on_vec_items)] fn main() { match_with_wildcard(); diff --git a/tests/ui/match_vec_item.stderr b/tests/ui/match_on_vec_items.stderr similarity index 76% rename from tests/ui/match_vec_item.stderr rename to tests/ui/match_on_vec_items.stderr index 2da2eaeee9b11..0e53a58f1418b 100644 --- a/tests/ui/match_vec_item.stderr +++ b/tests/ui/match_on_vec_items.stderr @@ -1,49 +1,49 @@ error: indexing vector may panic. Consider using `get` - --> $DIR/match_vec_item.rs:18:11 + --> $DIR/match_on_vec_items.rs:18:11 | LL | match arr[idx] { | ^^^^^^^^ help: try this: `arr.get(idx)` | - = note: `-D clippy::match-vec-item` implied by `-D warnings` + = note: `-D clippy::match-on-vec-items` implied by `-D warnings` error: indexing vector may panic. Consider using `get` - --> $DIR/match_vec_item.rs:25:11 + --> $DIR/match_on_vec_items.rs:25:11 | LL | match arr[range] { | ^^^^^^^^^^ help: try this: `arr.get(range)` error: indexing vector may panic. Consider using `get` - --> $DIR/match_vec_item.rs:38:11 + --> $DIR/match_on_vec_items.rs:38:11 | LL | match arr[idx] { | ^^^^^^^^ help: try this: `arr.get(idx)` error: indexing vector may panic. Consider using `get` - --> $DIR/match_vec_item.rs:45:11 + --> $DIR/match_on_vec_items.rs:45:11 | LL | match arr[range] { | ^^^^^^^^^^ help: try this: `arr.get(range)` error: indexing vector may panic. Consider using `get` - --> $DIR/match_vec_item.rs:58:11 + --> $DIR/match_on_vec_items.rs:58:11 | LL | match arr[idx] { | ^^^^^^^^ help: try this: `arr.get(idx)` error: indexing vector may panic. Consider using `get` - --> $DIR/match_vec_item.rs:65:11 + --> $DIR/match_on_vec_items.rs:65:11 | LL | match arr[range] { | ^^^^^^^^^^ help: try this: `arr.get(range)` error: indexing vector may panic. Consider using `get` - --> $DIR/match_vec_item.rs:78:11 + --> $DIR/match_on_vec_items.rs:78:11 | LL | match arr[idx] { | ^^^^^^^^ help: try this: `arr.get(idx)` error: indexing vector may panic. Consider using `get` - --> $DIR/match_vec_item.rs:85:11 + --> $DIR/match_on_vec_items.rs:85:11 | LL | match arr[range] { | ^^^^^^^^^^ help: try this: `arr.get(range)` From 940c6626541664597c41577a3d54ab7e5bbe10ee Mon Sep 17 00:00:00 2001 From: CrazyRoka Date: Sun, 26 Apr 2020 17:57:19 +0300 Subject: [PATCH 4/5] Small lint update - Changed lint category to `correctness` - Moved main function to bottom in test file - Added `FIXME` comment to `span_lint_and_sugg` to improve later --- clippy_lints/src/match_on_vec_items.rs | 15 ++++++------ tests/ui/match_on_vec_items.rs | 18 +++++++-------- tests/ui/match_on_vec_items.stderr | 32 +++++++++++++------------- 3 files changed, 33 insertions(+), 32 deletions(-) diff --git a/clippy_lints/src/match_on_vec_items.rs b/clippy_lints/src/match_on_vec_items.rs index 167b4abd93d1b..4071406cc84c3 100644 --- a/clippy_lints/src/match_on_vec_items.rs +++ b/clippy_lints/src/match_on_vec_items.rs @@ -1,4 +1,4 @@ -use crate::utils::{is_type_diagnostic_item, snippet_with_applicability, span_lint_and_sugg, walk_ptrs_ty}; +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}; @@ -38,7 +38,7 @@ declare_clippy_lint! { /// } /// ``` pub MATCH_ON_VEC_ITEMS, - style, + correctness, "matching on vector elements can panic" } @@ -53,19 +53,20 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MatchOnVecItems { if let ExprKind::Index(vec, idx) = idx_expr.kind; then { - let mut applicability = Applicability::MaybeIncorrect; + // 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 vector may panic. Consider using `get`", + "indexing into a vector may panic", "try this", format!( "{}.get({})", - snippet_with_applicability(cx, vec.span, "..", &mut applicability), - snippet_with_applicability(cx, idx.span, "..", &mut applicability) + snippet(cx, vec.span, ".."), + snippet(cx, idx.span, "..") ), - applicability + Applicability::MaybeIncorrect ); } } diff --git a/tests/ui/match_on_vec_items.rs b/tests/ui/match_on_vec_items.rs index 02b6da0343914..0bb39d77e461e 100644 --- a/tests/ui/match_on_vec_items.rs +++ b/tests/ui/match_on_vec_items.rs @@ -1,14 +1,5 @@ #![warn(clippy::match_on_vec_items)] -fn main() { - match_with_wildcard(); - match_without_wildcard(); - match_wildcard_and_action(); - match_vec_ref(); - match_with_get(); - match_with_array(); -} - fn match_with_wildcard() { let arr = vec![0, 1, 2, 3]; let range = 1..3; @@ -128,3 +119,12 @@ fn match_with_array() { _ => {}, } } + +fn main() { + match_with_wildcard(); + match_without_wildcard(); + match_wildcard_and_action(); + match_vec_ref(); + match_with_get(); + match_with_array(); +} diff --git a/tests/ui/match_on_vec_items.stderr b/tests/ui/match_on_vec_items.stderr index 0e53a58f1418b..49446d715abe2 100644 --- a/tests/ui/match_on_vec_items.stderr +++ b/tests/ui/match_on_vec_items.stderr @@ -1,49 +1,49 @@ -error: indexing vector may panic. Consider using `get` - --> $DIR/match_on_vec_items.rs:18:11 +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 vector may panic. Consider using `get` - --> $DIR/match_on_vec_items.rs:25:11 +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 vector may panic. Consider using `get` - --> $DIR/match_on_vec_items.rs:38:11 +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 vector may panic. Consider using `get` - --> $DIR/match_on_vec_items.rs:45:11 +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 vector may panic. Consider using `get` - --> $DIR/match_on_vec_items.rs:58:11 +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 vector may panic. Consider using `get` - --> $DIR/match_on_vec_items.rs:65:11 +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 vector may panic. Consider using `get` - --> $DIR/match_on_vec_items.rs:78:11 +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 vector may panic. Consider using `get` - --> $DIR/match_on_vec_items.rs:85:11 +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)` From b574941dcbf34b529b5c1eb1fa75ca6b1496b666 Mon Sep 17 00:00:00 2001 From: CrazyRoka Date: Sun, 26 Apr 2020 18:11:21 +0300 Subject: [PATCH 5/5] Updated lint info in lib.rs --- clippy_lints/src/lib.rs | 2 +- src/lintlist/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index a72fad8622754..f7d8314cceea9 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1473,7 +1473,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&loops::WHILE_LET_ON_ITERATOR), LintId::of(&main_recursion::MAIN_RECURSION), LintId::of(&map_clone::MAP_CLONE), - LintId::of(&match_on_vec_items::MATCH_ON_VEC_ITEMS), LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH), LintId::of(&matches::MATCH_BOOL), LintId::of(&matches::MATCH_OVERLAPPING_ARM), @@ -1646,6 +1645,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), diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 17f944112989f..fbcf4fde5e57b 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -1146,7 +1146,7 @@ pub static ref ALL_LINTS: Vec = vec![ }, Lint { name: "match_on_vec_items", - group: "style", + group: "correctness", desc: "matching on vector elements can panic", deprecation: None, module: "match_on_vec_items",