From b0115fb996b8cee2202ca379efd209d74ce0f751 Mon Sep 17 00:00:00 2001 From: CrazyRoka Date: Sat, 25 Apr 2020 00:52:02 +0300 Subject: [PATCH] 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 7916996e990..81ddc8c0067 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 7ea5a24abbd..07121dedefe 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 8e7c098d75c..d00d0bc2fe5 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 2494c3143f8..2da2eaeee9b 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 | - = help: consider using `get(..)` instead. +LL | match arr[idx] { + | ^^^^^^^^ help: try this: `arr.get(idx)` -error: aborting due to 2 previous errors +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 + | +LL | match arr[range] { + | ^^^^^^^^^^ help: try this: `arr.get(range)` + +error: aborting due to 8 previous errors