diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs index b3489142558..7cfcf3fe946 100644 --- a/clippy_lints/src/vec.rs +++ b/clippy_lints/src/vec.rs @@ -4,12 +4,12 @@ use std::ops::ControlFlow; use clippy_config::msrvs::{self, Msrv}; use clippy_utils::consts::{constant, Constant}; use clippy_utils::diagnostics::span_lint_hir_and_then; -use clippy_utils::source::snippet_with_applicability; +use clippy_utils::source::snippet_opt; 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,6 +52,172 @@ declare_clippy_lint! { impl_lint_pass!(UselessVec => [USELESS_VEC]); +impl<'tcx> LateLintPass<'tcx> for UselessVec { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + let Some(vec_args) = higher::VecArgs::hir(cx, expr.peel_borrows()) else { + return; + }; + // the parent callsite of this `vec!` expression, or span to the borrowed one such as `&vec!` + let callsite = expr.span.parent_callsite().unwrap_or(expr.span); + + 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) + 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) + || matches!(parent.kind, ExprKind::Index(..)) + || is_allowed_vec_method(cx, parent)) + { + ControlFlow::Continue(()) + } else { + ControlFlow::Break(()) + } + }) + .is_continue(); + + if only_slice_uses { + self.check_vec_macro(cx, &vec_args, callsite, expr.hir_id, SuggestedType::Array); + } else { + self.span_to_lint_map.insert(callsite, None); + } + }, + // if the local pattern has a specified type, do not lint. + Node::Local(Local { ty: Some(_), .. }) if higher::VecArgs::hir(cx, expr).is_some() => { + self.span_to_lint_map.insert(callsite, None); + }, + // search for `for _ in vec![...]` + Node::Expr(Expr { span, .. }) + if span.is_desugaring(DesugaringKind::ForLoop) && self.msrv.meets(msrvs::ARRAY_INTO_ITERATOR) => + { + let suggest_slice = suggest_type(expr); + self.check_vec_macro(cx, &vec_args, callsite, expr.hir_id, suggest_slice); + }, + // search for `&vec![_]` or `vec![_]` expressions where the adjusted type is `&[_]` + _ => { + let suggest_slice = suggest_type(expr); + + if adjusts_to_slice(cx, expr) { + self.check_vec_macro(cx, &vec_args, callsite, expr.hir_id, suggest_slice); + } else { + self.span_to_lint_map.insert(callsite, 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", 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); + }); + } + } + } + + extract_msrv_attr!(LateContext); +} + +impl UselessVec { + fn check_vec_macro<'tcx>( + &mut self, + cx: &LateContext<'tcx>, + vec_args: &higher::VecArgs<'tcx>, + span: Span, + hir_id: HirId, + suggest_slice: SuggestedType, + ) { + if span.from_expansion() { + return; + } + + let snippet = match *vec_args { + higher::VecArgs::Repeat(elem, len) => { + if let Some(Constant::Int(len_constant)) = constant(cx, cx.typeck_results(), len) { + // vec![ty; N] works when ty is Clone, [ty; N] requires it to be Copy also + if !is_copy(cx, cx.typeck_results().expr_ty(elem)) { + return; + } + + #[expect(clippy::cast_possible_truncation)] + if len_constant as u64 * size_of(cx, elem) > self.too_large_for_stack { + return; + } + + suggest_slice.snippet(cx, Some(elem.span), Some(len.span)) + } else { + return; + } + }, + higher::VecArgs::Vec(args) => { + 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; + } + Some(args[0].span.source_callsite().to(last.span.source_callsite())) + } else { + None + }; + suggest_slice.snippet(cx, args_span, None) + }, + }; + + self.span_to_lint_map.entry(span).or_insert(Some(( + hir_id, + suggest_slice, + snippet, + Applicability::MachineApplicable, + ))); + } +} + +#[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, len_span: Option) -> String { + let maybe_args = args_span.and_then(|sp| snippet_opt(cx, sp)).unwrap_or_default(); + let maybe_len = len_span + .and_then(|sp| snippet_opt(cx, sp).map(|s| format!("; {s}"))) + .unwrap_or_default(); + + match self { + Self::SliceRef(Mutability::Mut) => format!("&mut [{maybe_args}{maybe_len}]"), + Self::SliceRef(Mutability::Not) => format!("&[{maybe_args}{maybe_len}]"), + Self::Array => format!("[{maybe_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()) } @@ -69,179 +235,12 @@ pub fn is_allowed_vec_method(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { } } -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()) { - // 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| { - // 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) - || matches!(parent.kind, ExprKind::Index(..)) - || is_allowed_vec_method(cx, parent)) - { - ControlFlow::Continue(()) - } else { - ControlFlow::Break(()) - } - }) - .is_continue(); - - let span = expr.span.ctxt().outer_expn_data().call_site; - if only_slice_uses { - self.check_vec_macro(cx, &vec_args, span, expr.hir_id, SuggestedType::Array); - } 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() - { - 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) - { - // report the error around the `vec!` not inside `:` - 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) - } else { - // `expr` is the `vec![_]` expansion, so suggest `[_]` - // and also use the span of the actual `vec![_]` expression - (SuggestedType::Array, expr.span.ctxt().outer_expn_data().call_site) - }; - - if adjusts_to_slice(cx, expr) { - self.check_vec_macro(cx, &vec_args, span, expr.hir_id, suggest_slice); - } 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", - } - ); - span_lint_hir_and_then(cx, USELESS_VEC, *hir_id, *span, "useless use of `vec!`", |diag| { - diag.span_suggestion(*span, help_msg, snippet, *applicability); - }); - } - } - } - - 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, - cx: &LateContext<'tcx>, - vec_args: &higher::VecArgs<'tcx>, - span: Span, - hir_id: HirId, - suggest_slice: SuggestedType, - ) { - if span.from_expansion() { - return; - } - - let mut applicability = Applicability::MachineApplicable; - - let snippet = match *vec_args { - higher::VecArgs::Repeat(elem, len) => { - if let Some(Constant::Int(len_constant)) = constant(cx, cx.typeck_results(), len) { - // vec![ty; N] works when ty is Clone, [ty; N] requires it to be Copy also - if !is_copy(cx, cx.typeck_results().expr_ty(elem)) { - return; - } - - #[expect(clippy::cast_possible_truncation)] - if len_constant as u64 * size_of(cx, elem) > self.too_large_for_stack { - 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}]"), - } - } else { - return; - } - }, - higher::VecArgs::Vec(args) => { - 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}]") - }, - } - } else { - match suggest_slice { - SuggestedType::SliceRef(Mutability::Mut) => "&mut []".to_owned(), - SuggestedType::SliceRef(Mutability::Not) => "&[]".to_owned(), - SuggestedType::Array => "[]".to_owned(), - } - } - }, - }; - - self.span_to_lint_map - .entry(span) - .or_insert(Some((hir_id, suggest_slice, snippet, applicability))); +fn suggest_type(expr: &Expr<'_>) -> SuggestedType { + if let ExprKind::AddrOf(BorrowKind::Ref, mutability, _) = expr.kind { + // `expr` is `&vec![_]`, so suggest `&[_]` (or `&mut[_]` resp.) + SuggestedType::SliceRef(mutability) + } else { + // `expr` is the `vec![_]` expansion, so suggest `[_]` + SuggestedType::Array } } - -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()) -} diff --git a/tests/ui/vec.fixed b/tests/ui/vec.fixed index b318fd42f7c..9e755a82af4 100644 --- a/tests/ui/vec.fixed +++ b/tests/ui/vec.fixed @@ -217,3 +217,7 @@ fn issue_11958() { // should not lint, `String` is not `Copy` f(&vec!["test".to_owned(); 2]); } + +fn issue_12101() { + for a in &[1, 2] {} +} diff --git a/tests/ui/vec.rs b/tests/ui/vec.rs index 08ad6efa37f..c483271438b 100644 --- a/tests/ui/vec.rs +++ b/tests/ui/vec.rs @@ -217,3 +217,7 @@ fn issue_11958() { // should not lint, `String` is not `Copy` f(&vec!["test".to_owned(); 2]); } + +fn issue_12101() { + for a in &(vec![1, 2]) {} +} diff --git a/tests/ui/vec.stderr b/tests/ui/vec.stderr index 6fd4748fff3..3faea8033fe 100644 --- a/tests/ui/vec.stderr +++ b/tests/ui/vec.stderr @@ -121,5 +121,11 @@ error: useless use of `vec!` LL | this_macro_doesnt_need_vec!(vec![1]); | ^^^^^^^ help: you can use an array directly: `[1]` -error: aborting due to 20 previous errors +error: useless use of `vec!` + --> tests/ui/vec.rs:222:14 + | +LL | for a in &(vec![1, 2]) {} + | ^^^^^^^^^^^^^ help: you can use a slice directly: `&[1, 2]` + +error: aborting due to 21 previous errors