diff options
| -rw-r--r-- | clippy_lints/src/vec.rs | 168 |
1 files changed, 86 insertions, 82 deletions
diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs index b3489142558..a918bd1c824 100644 --- a/clippy_lints/src/vec.rs +++ b/clippy_lints/src/vec.rs @@ -9,7 +9,7 @@ use clippy_utils::ty::is_copy; use clippy_utils::visitors::for_each_local_use_after_expr; use clippy_utils::{get_parent_expr, higher, is_trait_method}; use rustc_errors::Applicability; -use rustc_hir::{BorrowKind, Expr, ExprKind, HirId, Mutability, Node, PatKind}; +use rustc_hir::{BorrowKind, Expr, ExprKind, HirId, Local, Mutability, Node, Pat, PatKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_middle::ty::layout::LayoutOf; @@ -52,35 +52,25 @@ declare_clippy_lint! { impl_lint_pass!(UselessVec => [USELESS_VEC]); -fn adjusts_to_slice(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { - matches!(cx.typeck_results().expr_ty_adjusted(e).kind(), ty::Ref(_, ty, _) if ty.is_slice()) -} - -/// Checks if the given expression is a method call to a `Vec` method -/// that also exists on slices. If this returns true, it means that -/// this expression does not actually require a `Vec` and could just work with an array. -pub fn is_allowed_vec_method(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { - const ALLOWED_METHOD_NAMES: &[&str] = &["len", "as_ptr", "is_empty"]; - - if let ExprKind::MethodCall(path, ..) = e.kind { - ALLOWED_METHOD_NAMES.contains(&path.ident.name.as_str()) - } else { - is_trait_method(cx, e, sym::IntoIterator) - } -} - impl<'tcx> LateLintPass<'tcx> for UselessVec { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let Some(vec_args) = higher::VecArgs::hir(cx, expr.peel_borrows()) { + let Some(vec_args) = higher::VecArgs::hir(cx, expr.peel_borrows()) else { + return; + }; + + match cx.tcx.parent_hir_node(expr.hir_id) { // search for `let foo = vec![_]` expressions where all uses of `foo` // adjust to slices or call a method that exist on slices (e.g. len) - if let Node::Local(local) = cx.tcx.parent_hir_node(expr.hir_id) - // for now ignore locals with type annotations. - // this is to avoid compile errors when doing the suggestion here: let _: Vec<_> = vec![..]; - && local.ty.is_none() - && let PatKind::Binding(_, id, ..) = local.pat.kind - { - let only_slice_uses = for_each_local_use_after_expr(cx, id, expr.hir_id, |expr| { + Node::Local(Local { + ty: None, + pat: + Pat { + kind: PatKind::Binding(_, id, ..), + .. + }, + .. + }) => { + let only_slice_uses = for_each_local_use_after_expr(cx, *id, expr.hir_id, |expr| { // allow indexing into a vec and some set of allowed method calls that exist on slices, too if let Some(parent) = get_parent_expr(cx, expr) && (adjusts_to_slice(cx, expr) @@ -100,26 +90,22 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec { } else { self.span_to_lint_map.insert(span, None); } - } + }, // if the local pattern has a specified type, do not lint. - else if let Some(_) = higher::VecArgs::hir(cx, expr) - && let Node::Local(local) = cx.tcx.parent_hir_node(expr.hir_id) - && local.ty.is_some() - { + Node::Local(Local { ty: Some(_), .. }) if higher::VecArgs::hir(cx, expr).is_some() => { let span = expr.span.ctxt().outer_expn_data().call_site; self.span_to_lint_map.insert(span, None); - } + }, // search for `for _ in vec![...]` - else if let Some(parent) = get_parent_expr(cx, expr) - && parent.span.is_desugaring(DesugaringKind::ForLoop) - && self.msrv.meets(msrvs::ARRAY_INTO_ITERATOR) + Node::Expr(Expr { span, .. }) + if span.is_desugaring(DesugaringKind::ForLoop) && self.msrv.meets(msrvs::ARRAY_INTO_ITERATOR) => { // report the error around the `vec!` not inside `<std macros>:` let span = expr.span.ctxt().outer_expn_data().call_site; self.check_vec_macro(cx, &vec_args, span, expr.hir_id, SuggestedType::Array); - } + }, // search for `&vec![_]` or `vec![_]` expressions where the adjusted type is `&[_]` - else { + _ => { let (suggest_slice, span) = if let ExprKind::AddrOf(BorrowKind::Ref, mutability, _) = expr.kind { // `expr` is `&vec![_]`, so suggest `&[_]` (or `&mut[_]` resp.) (SuggestedType::SliceRef(mutability), expr.span) @@ -134,20 +120,14 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec { } else { self.span_to_lint_map.insert(span, None); } - } + }, } } fn check_crate_post(&mut self, cx: &LateContext<'tcx>) { for (span, lint_opt) in &self.span_to_lint_map { if let Some((hir_id, suggest_slice, snippet, applicability)) = lint_opt { - let help_msg = format!( - "you can use {} directly", - match suggest_slice { - SuggestedType::SliceRef(_) => "a slice", - SuggestedType::Array => "an array", - } - ); + let help_msg = format!("you can use {} directly", suggest_slice.desc(),); span_lint_hir_and_then(cx, USELESS_VEC, *hir_id, *span, "useless use of `vec!`", |diag| { diag.span_suggestion(*span, help_msg, snippet, *applicability); }); @@ -158,14 +138,6 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec { extract_msrv_attr!(LateContext); } -#[derive(Copy, Clone)] -pub(crate) enum SuggestedType { - /// Suggest using a slice `&[..]` / `&mut [..]` - SliceRef(Mutability), - /// Suggest using an array: `[..]` - Array, -} - impl UselessVec { fn check_vec_macro<'tcx>( &mut self, @@ -194,44 +166,21 @@ impl UselessVec { return; } - let elem = snippet_with_applicability(cx, elem.span, "elem", &mut applicability); - let len = snippet_with_applicability(cx, len.span, "len", &mut applicability); - - match suggest_slice { - SuggestedType::SliceRef(Mutability::Mut) => format!("&mut [{elem}; {len}]"), - SuggestedType::SliceRef(Mutability::Not) => format!("&[{elem}; {len}]"), - SuggestedType::Array => format!("[{elem}; {len}]"), - } + suggest_slice.snippet(cx, Some(elem.span), Some(len.span), &mut applicability) } else { return; } }, higher::VecArgs::Vec(args) => { - if let Some(last) = args.iter().last() { + let args_span = if let Some(last) = args.iter().last() { if args.len() as u64 * size_of(cx, last) > self.too_large_for_stack { return; } - let span = args[0].span.source_callsite().to(last.span.source_callsite()); - let args = snippet_with_applicability(cx, span, "..", &mut applicability); - - match suggest_slice { - SuggestedType::SliceRef(Mutability::Mut) => { - format!("&mut [{args}]") - }, - SuggestedType::SliceRef(Mutability::Not) => { - format!("&[{args}]") - }, - SuggestedType::Array => { - format!("[{args}]") - }, - } + Some(args[0].span.source_callsite().to(last.span.source_callsite())) } else { - match suggest_slice { - SuggestedType::SliceRef(Mutability::Mut) => "&mut []".to_owned(), - SuggestedType::SliceRef(Mutability::Not) => "&[]".to_owned(), - SuggestedType::Array => "[]".to_owned(), - } - } + None + }; + suggest_slice.snippet(cx, args_span, None, &mut applicability) }, }; @@ -241,7 +190,62 @@ impl UselessVec { } } +#[derive(Copy, Clone)] +pub(crate) enum SuggestedType { + /// Suggest using a slice `&[..]` / `&mut [..]` + SliceRef(Mutability), + /// Suggest using an array: `[..]` + Array, +} + +impl SuggestedType { + fn desc(self) -> &'static str { + match self { + Self::SliceRef(_) => "a slice", + Self::Array => "an array", + } + } + + fn snippet( + self, + cx: &LateContext<'_>, + args_span: Option<Span>, + len_span: Option<Span>, + app: &mut Applicability, + ) -> String { + let args = args_span + .map(|sp| snippet_with_applicability(cx, sp, "..", app)) + .unwrap_or_default(); + let maybe_len = len_span + .map(|sp| format!("; {}", snippet_with_applicability(cx, sp, "len", app))) + .unwrap_or_default(); + + match self { + Self::SliceRef(Mutability::Mut) => format!("&mut [{args}{maybe_len}]"), + Self::SliceRef(Mutability::Not) => format!("&[{args}{maybe_len}]"), + Self::Array => format!("[{args}{maybe_len}]"), + } + } +} + fn size_of(cx: &LateContext<'_>, expr: &Expr<'_>) -> u64 { let ty = cx.typeck_results().expr_ty_adjusted(expr); cx.layout_of(ty).map_or(0, |l| l.size.bytes()) } + +fn adjusts_to_slice(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { + matches!(cx.typeck_results().expr_ty_adjusted(e).kind(), ty::Ref(_, ty, _) if ty.is_slice()) +} + +/// Checks if the given expression is a method call to a `Vec` method +/// that also exists on slices. If this returns true, it means that +/// this expression does not actually require a `Vec` and could just work with an array. +pub fn is_allowed_vec_method(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { + const ALLOWED_METHOD_NAMES: &[&str] = &["len", "as_ptr", "is_empty"]; + + if let ExprKind::MethodCall(path, ..) = e.kind { + ALLOWED_METHOD_NAMES.contains(&path.ident.name.as_str()) + } else { + is_trait_method(cx, e, sym::IntoIterator) + } +} |