Merge branch 'master' into fix_manual_memcpy

CHANGELOG.md

@@ -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

clippy_lints/src/consts.rs

@@ -358,9 +358,9 @@ fn index(&mut self, lhs: &'_ Expr<'_>, index: &'_ Expr<'_>) -> Option<Constant>
             },
             (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 {

clippy_lints/src/lib.rs

@@ -249,6 +249,7 @@ macro_rules! declare_clippy_lint {
 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),

clippy_lints/src/match_on_vec_items.rs

@@ -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
+}

src/lintlist/mod.rs

@@ -1144,6 +1144,13 @@
         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",

tests/ui/match_on_vec_items.rs

@@ -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();
+}

tests/ui/match_on_vec_items.stderr

@@ -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
+