From 929f746b5c19f7bf8e52cc32a3bb868fef0461e6 Mon Sep 17 00:00:00 2001 From: J-ZhengLi Date: Sat, 24 Feb 2024 00:35:48 +0800 Subject: [PATCH] fix the actual bug --- clippy_lints/src/vec.rs | 75 +++++++++++++++++++---------------------- tests/ui/vec.fixed | 4 +++ tests/ui/vec.rs | 4 +++ tests/ui/vec.stderr | 8 ++++- 4 files changed, 50 insertions(+), 41 deletions(-) diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs index a918bd1c824..7cfcf3fe946 100644 --- a/clippy_lints/src/vec.rs +++ b/clippy_lints/src/vec.rs @@ -4,7 +4,7 @@ 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}; @@ -57,6 +57,8 @@ 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` @@ -84,41 +86,31 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { }) .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); + self.check_vec_macro(cx, &vec_args, callsite, expr.hir_id, SuggestedType::Array); } else { - self.span_to_lint_map.insert(span, None); + 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() => { - let span = expr.span.ctxt().outer_expn_data().call_site; - self.span_to_lint_map.insert(span, None); + 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) => { - // 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); + 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, 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) - }; + let suggest_slice = suggest_type(expr); if adjusts_to_slice(cx, expr) { - self.check_vec_macro(cx, &vec_args, span, expr.hir_id, suggest_slice); + self.check_vec_macro(cx, &vec_args, callsite, expr.hir_id, suggest_slice); } else { - self.span_to_lint_map.insert(span, None); + self.span_to_lint_map.insert(callsite, None); } }, } @@ -151,8 +143,6 @@ fn check_vec_macro<'tcx>( 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) { @@ -166,7 +156,7 @@ fn check_vec_macro<'tcx>( return; } - suggest_slice.snippet(cx, Some(elem.span), Some(len.span), &mut applicability) + suggest_slice.snippet(cx, Some(elem.span), Some(len.span)) } else { return; } @@ -180,13 +170,16 @@ fn check_vec_macro<'tcx>( } else { None }; - suggest_slice.snippet(cx, args_span, None, &mut applicability) + suggest_slice.snippet(cx, args_span, None) }, }; - self.span_to_lint_map - .entry(span) - .or_insert(Some((hir_id, suggest_slice, snippet, applicability))); + self.span_to_lint_map.entry(span).or_insert(Some(( + hir_id, + suggest_slice, + snippet, + Applicability::MachineApplicable, + ))); } } @@ -206,24 +199,16 @@ fn desc(self) -> &'static str { } } - fn snippet( - self, - cx: &LateContext<'_>, - args_span: Option, - len_span: Option, - app: &mut Applicability, - ) -> String { - let args = args_span - .map(|sp| snippet_with_applicability(cx, sp, "..", app)) - .unwrap_or_default(); + 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 - .map(|sp| format!("; {}", snippet_with_applicability(cx, sp, "len", app))) + .and_then(|sp| snippet_opt(cx, sp).map(|s| format!("; {s}"))) .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}]"), + 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}]"), } } } @@ -249,3 +234,13 @@ pub fn is_allowed_vec_method(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { is_trait_method(cx, e, sym::IntoIterator) } } + +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 + } +} 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 f(_s: &[String]) {} // 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