Move MatchOnVecItems into Matches lint pass

Author: Jarcho

clippy_lints/src/lib.register_lints.rs

@@ -258,7 +258,6 @@ store.register_lints(&[
     map_err_ignore::MAP_ERR_IGNORE,
     map_unit_fn::OPTION_MAP_UNIT_FN,
     map_unit_fn::RESULT_MAP_UNIT_FN,
-    match_on_vec_items::MATCH_ON_VEC_ITEMS,
     match_result_ok::MATCH_RESULT_OK,
     match_str_case_mismatch::MATCH_STR_CASE_MISMATCH,
     matches::COLLAPSIBLE_MATCH,
@@ -267,6 +266,7 @@ store.register_lints(&[
     matches::MATCH_AS_REF,
     matches::MATCH_BOOL,
     matches::MATCH_LIKE_MATCHES_MACRO,
+    matches::MATCH_ON_VEC_ITEMS,
     matches::MATCH_OVERLAPPING_ARM,
     matches::MATCH_REF_PATS,
     matches::MATCH_SAME_ARMS,

clippy_lints/src/lib.register_pedantic.rs

@@ -51,8 +51,8 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
     LintId::of(macro_use::MACRO_USE_IMPORTS),
     LintId::of(manual_assert::MANUAL_ASSERT),
     LintId::of(manual_ok_or::MANUAL_OK_OR),
-    LintId::of(match_on_vec_items::MATCH_ON_VEC_ITEMS),
     LintId::of(matches::MATCH_BOOL),
+    LintId::of(matches::MATCH_ON_VEC_ITEMS),
     LintId::of(matches::MATCH_SAME_ARMS),
     LintId::of(matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS),
     LintId::of(matches::MATCH_WILD_ERR_ARM),

clippy_lints/src/lib.rs

@@ -286,7 +286,6 @@ mod manual_strip;
 mod map_clone;
 mod map_err_ignore;
 mod map_unit_fn;
-mod match_on_vec_items;
 mod match_result_ok;
 mod match_str_case_mismatch;
 mod matches;
@@ -815,7 +814,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(if_not_else::IfNotElse));
     store.register_late_pass(|| Box::new(equatable_if_let::PatternEquality));
     store.register_late_pass(|| Box::new(mut_mutex_lock::MutMutexLock));
-    store.register_late_pass(|| Box::new(match_on_vec_items::MatchOnVecItems));
     store.register_late_pass(|| Box::new(manual_async_fn::ManualAsyncFn));
     store.register_late_pass(|| Box::new(vec_resize_to_zero::VecResizeToZero));
     store.register_late_pass(|| Box::new(panic_in_result_fn::PanicInResultFn));

clippy_lints/src/match_on_vec_items.rs

@@ -0,0 +1,61 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet;
+use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item};
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, LangItem};
+use rustc_lint::LateContext;
+use rustc_span::sym;
+
+use super::MATCH_ON_VEC_ITEMS;
+
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, scrutinee: &'tcx Expr<'_>) {
+    if_chain! {
+        if let Some(idx_expr) = is_vec_indexing(cx, scrutinee);
+        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,
+                scrutinee.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<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
+    if_chain! {
+        if let ExprKind::Index(array, index) = expr.kind;
+        if is_vector(cx, array);
+        if !is_full_range(cx, index);
+
+        then {
+            return Some(expr);
+        }
+    }
+
+    None
+}
+
+fn is_vector(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    let ty = cx.typeck_results().expr_ty(expr);
+    let ty = ty.peel_refs();
+    is_type_diagnostic_item(cx, ty, sym::Vec)
+}
+
+fn is_full_range(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    let ty = cx.typeck_results().expr_ty(expr);
+    let ty = ty.peel_refs();
+    is_type_lang_item(cx, ty, LangItem::RangeFull)
+}

clippy_lints/src/matches/match_on_vec_items.rs

@@ -14,6 +14,7 @@ mod manual_unwrap_or;
 mod match_as_ref;
 mod match_bool;
 mod match_like_matches;
+mod match_on_vec_items;
 mod match_ref_pats;
 mod match_same_arms;
 mod match_single_binding;
@@ -678,6 +679,43 @@ declare_clippy_lint! {
     "finds patterns that can be encoded more concisely with `Option::unwrap_or` or `Result::unwrap_or`"
 }
 
+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.
+    ///
+    /// ### 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),
+    ///     _ => {},
+    /// }
+    /// ```
+    #[clippy::version = "1.45.0"]
+    pub MATCH_ON_VEC_ITEMS,
+    pedantic,
+    "matching on vector elements can panic"
+}
+
 #[derive(Default)]
 pub struct Matches {
     msrv: Option<RustcVersion>,
@@ -714,6 +752,7 @@ impl_lint_pass!(Matches => [
     NEEDLESS_MATCH,
     COLLAPSIBLE_MATCH,
     MANUAL_UNWRAP_OR,
+    MATCH_ON_VEC_ITEMS,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Matches {
@@ -750,6 +789,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
                     match_wild_enum::check(cx, ex, arms);
                     match_as_ref::check(cx, ex, arms, expr);
                     needless_match::check_match(cx, ex, arms, expr);
+                    match_on_vec_items::check(cx, ex);
 
                     if !in_constant(cx, expr.hir_id) {
                         manual_unwrap_or::check(cx, expr, ex, arms);