Improved snippets and added tests

This commit is contained in:
Red Rapious 2023-08-22 18:46:16 +02:00
parent b0bd6219c8
commit df8bb47f17
4 changed files with 124 additions and 40 deletions

View File

@ -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 {

View File

@ -1,17 +1,56 @@
#![warn(clippy::reserve_after_initialization)]
#![no_main]
fn main() {
// Should lint
// Should lint
fn standard() {
let mut v1: Vec<usize> = Vec::with_capacity(10);
}
// Should lint
// Should lint
fn capacity_as_expr() {
let capacity = 10;
let mut v2: Vec<usize> = 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<usize> = Vec::with_capacity(10);
}
// Shouldn't lint
fn called_with_capacity() {
let _v4: Vec<usize> = Vec::with_capacity(10);
}
// Should lint
fn assign_expression() {
let mut v5: Vec<usize> = Vec::new();
v5 = Vec::with_capacity(10);
}
/*fn in_macros() {
external! {
// Should lint
let mut v1: Vec<usize> = vec![];
v1.reserve(10);
// Should lint
let capacity = 10;
let mut v2: Vec<usize> = vec![];
v2.reserve(capacity);
}
with_span! {
span
// Should lint
let mut v1: Vec<usize> = vec![];
v1.reserve(10);
// Should lint
let capacity = 10;
let mut v2: Vec<usize> = vec![];
v2.reserve(capacity);
}
}*/

View File

@ -1,19 +1,59 @@
#![warn(clippy::reserve_after_initialization)]
#![no_main]
fn main() {
// Should lint
// Should lint
fn standard() {
let mut v1: Vec<usize> = vec![];
v1.reserve(10);
}
// Should lint
// Should lint
fn capacity_as_expr() {
let capacity = 10;
let mut v2: Vec<usize> = 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<usize> = Vec::with_capacity(10);
}
// Shouldn't lint
fn called_with_capacity() {
let _v4: Vec<usize> = Vec::with_capacity(10);
}
// Should lint
fn assign_expression() {
let mut v5: Vec<usize> = Vec::new();
v5 = Vec::new();
v5.reserve(10);
}
/*fn in_macros() {
external! {
// Should lint
let mut v1: Vec<usize> = vec![];
v1.reserve(10);
// Should lint
let capacity = 10;
let mut v2: Vec<usize> = vec![];
v2.reserve(capacity);
}
with_span! {
span
// Should lint
let mut v1: Vec<usize> = vec![];
v1.reserve(10);
// Should lint
let capacity = 10;
let mut v2: Vec<usize> = vec![];
v2.reserve(capacity);
}
}*/

View File

@ -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<usize> = vec![];
LL | | v1.reserve(10);
| |___________________^ help: consider using `Vec::with_capacity(space_hint)`: `let mut v1: Vec<usize> = Vec::with_capacity(10);`
| |___________________^ help: consider using `Vec::with_capacity(/* Space hint */)`: `let mut v1: Vec<usize> = 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<usize> = vec![];
LL | | v2.reserve(capacity);
| |_________________________^ help: consider using `Vec::with_capacity(space_hint)`: `let mut v2: Vec<usize> = Vec::with_capacity(capacity);`
| |_________________________^ help: consider using `Vec::with_capacity(/* Space hint */)`: `let mut v2: Vec<usize> = 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