From df8bb47f1711d0db0dd6e9e8446b15b394721126 Mon Sep 17 00:00:00 2001 From: Red Rapious Date: Tue, 22 Aug 2023 18:46:16 +0200 Subject: [PATCH] Improved snippets and added tests --- .../src/reserve_after_initialization.rs | 36 ++++++------- tests/ui/reserve_after_initialization.fixed | 53 +++++++++++++++--- tests/ui/reserve_after_initialization.rs | 54 ++++++++++++++++--- tests/ui/reserve_after_initialization.stderr | 21 +++++--- 4 files changed, 124 insertions(+), 40 deletions(-) diff --git a/clippy_lints/src/reserve_after_initialization.rs b/clippy_lints/src/reserve_after_initialization.rs index d530d5758b1..17ca9c8aab8 100644 --- a/clippy_lints/src/reserve_after_initialization.rs +++ b/clippy_lints/src/reserve_after_initialization.rs @@ -2,7 +2,6 @@ use clippy_utils::higher::{get_vec_init_kind, VecInitKind}; use clippy_utils::path_to_local_id; use clippy_utils::source::snippet; -//use rustc_ast::LitKind; use rustc_errors::Applicability; use rustc_hir::def::Res; use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, HirId, Local, PatKind, QPath, Stmt, StmtKind}; @@ -16,7 +15,7 @@ /// Informs the user about a more concise way to create a vector with a known capacity. /// /// ### Why is this bad? - /// The `Vec::with_capacity` constructor is easier to understand. + /// The `Vec::with_capacity` constructor is less complex. /// /// ### Example /// ```rust @@ -51,14 +50,14 @@ fn display_err(&self, cx: &LateContext<'_>) { return; } - let s = format!("{} = Vec::with_capacity({});", self.init_part, self.space_hint); + let s = format!("{}Vec::with_capacity({});", self.init_part, self.space_hint); span_lint_and_sugg( cx, RESERVE_AFTER_INITIALIZATION, self.err_span, - "calls to `reverse` immediately after creation", - "consider using `Vec::with_capacity(space_hint)`", + "call to `reserve` immediately after creation", + "consider using `Vec::with_capacity(/* Space hint */)`", s, Applicability::HasPlaceholders, ); @@ -72,7 +71,7 @@ fn check_block(&mut self, _: &LateContext<'tcx>, _: &'tcx Block<'tcx>) { fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) { if let Some(init_expr) = local.init - && let PatKind::Binding(BindingAnnotation::MUT, id, _name, None) = local.pat.kind + && let PatKind::Binding(BindingAnnotation::MUT, id, _, None) = local.pat.kind && !in_external_macro(cx.sess(), local.span) && let Some(init) = get_vec_init_kind(cx, init_expr) && !matches!(init, VecInitKind::WithExprCapacity(_)) @@ -81,12 +80,9 @@ fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) { self.searcher = Some(VecReserveSearcher { local_id: id, err_span: local.span, - init_part: format!("let {}: {}", - snippet(cx, local.pat.span, ""), - match local.ty { - Some(type_inference) => snippet(cx, type_inference.span, "").to_string(), - None => String::new() - }), + init_part: snippet(cx, local.span.shrink_to_lo() + .to(init_expr.span.source_callsite().shrink_to_lo()), "..") + .into_owned(), space_hint: String::new() }); } @@ -96,17 +92,20 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if self.searcher.is_none() && let ExprKind::Assign(left, right, _) = expr.kind && let ExprKind::Path(QPath::Resolved(None, path)) = left.kind - && let [_name] = &path.segments + && let [_] = &path.segments && let Res::Local(id) = path.res && !in_external_macro(cx.sess(), expr.span) && let Some(init) = get_vec_init_kind(cx, right) - && !matches!(init, VecInitKind::WithExprCapacity(_)) - && !matches!(init, VecInitKind::WithConstCapacity(_)) + && !matches!(init, VecInitKind::WithExprCapacity(_) + | VecInitKind::WithConstCapacity(_) + ) { self.searcher = Some(VecReserveSearcher { local_id: id, err_span: expr.span, - init_part: snippet(cx, left.span, "").to_string(), + init_part: snippet(cx, left.span.shrink_to_lo() + .to(right.span.source_callsite().shrink_to_lo()), "..") + .into_owned(), // see `assign_expression` test space_hint: String::new() }); } @@ -115,14 +114,13 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { if let Some(searcher) = self.searcher.take() { if let StmtKind::Expr(expr) | StmtKind::Semi(expr) = stmt.kind - && let ExprKind::MethodCall(name, self_arg, other_args, _) = expr.kind - && other_args.len() == 1 + && let ExprKind::MethodCall(name, self_arg, [space_hint], _) = expr.kind && path_to_local_id(self_arg, searcher.local_id) && name.ident.as_str() == "reserve" { self.searcher = Some(VecReserveSearcher { err_span: searcher.err_span.to(stmt.span), - space_hint: snippet(cx, other_args[0].span, "").to_string(), + space_hint: snippet(cx, space_hint.span, "..").to_string(), .. searcher }); } else { diff --git a/tests/ui/reserve_after_initialization.fixed b/tests/ui/reserve_after_initialization.fixed index e7dd8f6b14f..5ef848ed8cf 100644 --- a/tests/ui/reserve_after_initialization.fixed +++ b/tests/ui/reserve_after_initialization.fixed @@ -1,17 +1,56 @@ #![warn(clippy::reserve_after_initialization)] +#![no_main] -fn main() { - // Should lint +// Should lint +fn standard() { let mut v1: Vec = Vec::with_capacity(10); +} - // Should lint +// Should lint +fn capacity_as_expr() { let capacity = 10; let mut v2: Vec = Vec::with_capacity(capacity); +} - // Shouldn't lint +// Shouldn't lint +fn vec_init_with_argument() { let mut v3 = vec![1]; v3.reserve(10); - - // Shouldn't lint - let mut v4: Vec = Vec::with_capacity(10); } + +// Shouldn't lint +fn called_with_capacity() { + let _v4: Vec = Vec::with_capacity(10); +} + +// Should lint +fn assign_expression() { + let mut v5: Vec = Vec::new(); + v5 = Vec::with_capacity(10); +} + +/*fn in_macros() { + external! { + // Should lint + let mut v1: Vec = vec![]; + v1.reserve(10); + + // Should lint + let capacity = 10; + let mut v2: Vec = vec![]; + v2.reserve(capacity); + } + + with_span! { + span + + // Should lint + let mut v1: Vec = vec![]; + v1.reserve(10); + + // Should lint + let capacity = 10; + let mut v2: Vec = vec![]; + v2.reserve(capacity); + } +}*/ diff --git a/tests/ui/reserve_after_initialization.rs b/tests/ui/reserve_after_initialization.rs index 07f9377dbfe..eba7bc8dab3 100644 --- a/tests/ui/reserve_after_initialization.rs +++ b/tests/ui/reserve_after_initialization.rs @@ -1,19 +1,59 @@ #![warn(clippy::reserve_after_initialization)] +#![no_main] -fn main() { - // Should lint +// Should lint +fn standard() { let mut v1: Vec = vec![]; v1.reserve(10); +} - // Should lint +// Should lint +fn capacity_as_expr() { let capacity = 10; let mut v2: Vec = vec![]; v2.reserve(capacity); +} - // Shouldn't lint +// Shouldn't lint +fn vec_init_with_argument() { let mut v3 = vec![1]; v3.reserve(10); - - // Shouldn't lint - let mut v4: Vec = Vec::with_capacity(10); } + +// Shouldn't lint +fn called_with_capacity() { + let _v4: Vec = Vec::with_capacity(10); +} + +// Should lint +fn assign_expression() { + let mut v5: Vec = Vec::new(); + v5 = Vec::new(); + v5.reserve(10); +} + +/*fn in_macros() { + external! { + // Should lint + let mut v1: Vec = vec![]; + v1.reserve(10); + + // Should lint + let capacity = 10; + let mut v2: Vec = vec![]; + v2.reserve(capacity); + } + + with_span! { + span + + // Should lint + let mut v1: Vec = vec![]; + v1.reserve(10); + + // Should lint + let capacity = 10; + let mut v2: Vec = vec![]; + v2.reserve(capacity); + } +}*/ diff --git a/tests/ui/reserve_after_initialization.stderr b/tests/ui/reserve_after_initialization.stderr index ab2753cdd60..e50a8a065ba 100644 --- a/tests/ui/reserve_after_initialization.stderr +++ b/tests/ui/reserve_after_initialization.stderr @@ -1,18 +1,25 @@ -error: calls to `reverse` immediately after creation - --> $DIR/reserve_after_initialization.rs:5:5 +error: call to `reserve` immediately after creation + --> $DIR/reserve_after_initialization.rs:6:5 | LL | / let mut v1: Vec = vec![]; LL | | v1.reserve(10); - | |___________________^ help: consider using `Vec::with_capacity(space_hint)`: `let mut v1: Vec = Vec::with_capacity(10);` + | |___________________^ help: consider using `Vec::with_capacity(/* Space hint */)`: `let mut v1: Vec = Vec::with_capacity(10);` | = note: `-D clippy::reserve-after-initialization` implied by `-D warnings` -error: calls to `reverse` immediately after creation - --> $DIR/reserve_after_initialization.rs:10:5 +error: call to `reserve` immediately after creation + --> $DIR/reserve_after_initialization.rs:13:5 | LL | / let mut v2: Vec = vec![]; LL | | v2.reserve(capacity); - | |_________________________^ help: consider using `Vec::with_capacity(space_hint)`: `let mut v2: Vec = Vec::with_capacity(capacity);` + | |_________________________^ help: consider using `Vec::with_capacity(/* Space hint */)`: `let mut v2: Vec = Vec::with_capacity(capacity);` -error: aborting due to 2 previous errors +error: call to `reserve` immediately after creation + --> $DIR/reserve_after_initialization.rs:31:5 + | +LL | / v5 = Vec::new(); +LL | | v5.reserve(10); + | |___________________^ help: consider using `Vec::with_capacity(/* Space hint */)`: `v5 = Vec::with_capacity(10);` + +error: aborting due to 3 previous errors