diff options
| author | bors <bors@rust-lang.org> | 2023-08-23 12:06:41 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2023-08-23 12:06:41 +0000 |
| commit | 4932d0573360bb71d078579842d7b7712edff1a3 (patch) | |
| tree | a71b8b37a5dd9e1e7f73b8df8c17ad145b7d5d27 | |
| parent | edfee16ade1d4af7fa33e34e39ff443cbdd7ffe9 (diff) | |
| parent | f3c58773026bf042142be2bf8ed1e8b8797a4f68 (diff) | |
| download | rust-4932d0573360bb71d078579842d7b7712edff1a3.tar.gz rust-4932d0573360bb71d078579842d7b7712edff1a3.zip | |
Auto merge of #11373 - Red-Rapious:master, r=blyxyas,y21
Added new lint: `reserve_after_initialization` Closes https://github.com/rust-lang/rust-clippy/issues/11330. A new lint that informs the user about a more concise way to create a vector with a known capacity. Example: ```rust let mut v: Vec<usize> = vec![]; v.reserve(10); ``` Produces the following help: ```rust | 2 | / let mut v: Vec<usize> = vec![]; 3 | | v.reserve(10); | |__________________^ help: consider using `Vec::with_capacity(space_hint)`: `let v: Vec<usize> = Vec::with_capacity(10);` | ``` And can be rewritten as: ```rust let v: Vec<usize> = Vec::with_capacity(10); ``` changelog: new lint [`reserve_after_initialization`]
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_lints/src/declared_lints.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/reserve_after_initialization.rs | 134 | ||||
| -rw-r--r-- | tests/ui/reserve_after_initialization.fixed | 48 | ||||
| -rw-r--r-- | tests/ui/reserve_after_initialization.rs | 51 | ||||
| -rw-r--r-- | tests/ui/reserve_after_initialization.stderr | 25 |
7 files changed, 262 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index fe64283462d..4e33cb7b457 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5299,6 +5299,7 @@ Released 2018-09-13 [`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro [`repeat_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_once [`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts +[`reserve_after_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#reserve_after_initialization [`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs [`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used [`result_large_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index bab77f91269..1be9720fbbf 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -579,6 +579,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::reference::DEREF_ADDROF_INFO, crate::regex::INVALID_REGEX_INFO, crate::regex::TRIVIAL_REGEX_INFO, + crate::reserve_after_initialization::RESERVE_AFTER_INITIALIZATION_INFO, crate::return_self_not_must_use::RETURN_SELF_NOT_MUST_USE_INFO, crate::returns::LET_AND_RETURN_INFO, crate::returns::NEEDLESS_RETURN_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 358004cf460..f50019f3cf7 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -285,6 +285,7 @@ mod ref_option_ref; mod ref_patterns; mod reference; mod regex; +mod reserve_after_initialization; mod return_self_not_must_use; mod returns; mod same_name_method; @@ -1095,6 +1096,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: }); store.register_late_pass(|_| Box::new(redundant_locals::RedundantLocals)); store.register_late_pass(|_| Box::new(ignored_unit_patterns::IgnoredUnitPatterns)); + store.register_late_pass(|_| Box::<reserve_after_initialization::ReserveAfterInitialization>::default()); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/reserve_after_initialization.rs b/clippy_lints/src/reserve_after_initialization.rs new file mode 100644 index 00000000000..0c8c904e374 --- /dev/null +++ b/clippy_lints/src/reserve_after_initialization.rs @@ -0,0 +1,134 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::higher::{get_vec_init_kind, VecInitKind}; +use clippy_utils::source::snippet; +use clippy_utils::{is_from_proc_macro, path_to_local_id}; +use rustc_errors::Applicability; +use rustc_hir::def::Res; +use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, HirId, Local, PatKind, QPath, Stmt, StmtKind}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::Span; + +declare_clippy_lint! { + /// ### What it does + /// 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 less complex. + /// + /// ### Example + /// ```rust + /// let mut v: Vec<usize> = vec![]; + /// v.reserve(10); + /// ``` + /// Use instead: + /// ```rust + /// let mut v: Vec<usize> = Vec::with_capacity(10); + /// ``` + #[clippy::version = "1.73.0"] + pub RESERVE_AFTER_INITIALIZATION, + complexity, + "`reserve` called immediatly after `Vec` creation" +} +impl_lint_pass!(ReserveAfterInitialization => [RESERVE_AFTER_INITIALIZATION]); + +#[derive(Default)] +pub struct ReserveAfterInitialization { + searcher: Option<VecReserveSearcher>, +} + +struct VecReserveSearcher { + local_id: HirId, + err_span: Span, + init_part: String, + space_hint: String, +} +impl VecReserveSearcher { + fn display_err(&self, cx: &LateContext<'_>) { + if self.space_hint.is_empty() { + return; + } + + let s = format!("{}Vec::with_capacity({});", self.init_part, self.space_hint); + + span_lint_and_sugg( + cx, + RESERVE_AFTER_INITIALIZATION, + self.err_span, + "call to `reserve` immediately after creation", + "consider using `Vec::with_capacity(/* Space hint */)`", + s, + Applicability::HasPlaceholders, + ); + } +} + +impl<'tcx> LateLintPass<'tcx> for ReserveAfterInitialization { + fn check_block(&mut self, _: &LateContext<'tcx>, _: &'tcx Block<'tcx>) { + self.searcher = None; + } + + 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, _, 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(_) | VecInitKind::WithConstCapacity(_)) + { + self.searcher = Some(VecReserveSearcher { + local_id: id, + err_span: local.span, + init_part: snippet(cx, local.span.shrink_to_lo() + .to(init_expr.span.source_callsite().shrink_to_lo()), "..") + .into_owned(), + space_hint: String::new() + }); + } + } + + 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 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(_) | VecInitKind::WithConstCapacity(_)) + { + self.searcher = Some(VecReserveSearcher { + local_id: id, + err_span: expr.span, + 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() + }); + } + } + + 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, [space_hint], _) = expr.kind + && path_to_local_id(self_arg, searcher.local_id) + && name.ident.as_str() == "reserve" + && !is_from_proc_macro(cx, expr) + { + self.searcher = Some(VecReserveSearcher { + err_span: searcher.err_span.to(stmt.span), + space_hint: snippet(cx, space_hint.span, "..").into_owned(), + .. searcher + }); + } else { + searcher.display_err(cx); + } + } + } + + fn check_block_post(&mut self, cx: &LateContext<'tcx>, _: &'tcx Block<'tcx>) { + if let Some(searcher) = self.searcher.take() { + searcher.display_err(cx); + } + } +} diff --git a/tests/ui/reserve_after_initialization.fixed b/tests/ui/reserve_after_initialization.fixed new file mode 100644 index 00000000000..0675277849a --- /dev/null +++ b/tests/ui/reserve_after_initialization.fixed @@ -0,0 +1,48 @@ +//@aux-build:proc_macros.rs +#![warn(clippy::reserve_after_initialization)] +#![no_main] + +extern crate proc_macros; +use proc_macros::{external, with_span}; + +// Should lint +fn standard() { + let mut v1: Vec<usize> = Vec::with_capacity(10); +} + +// Should lint +fn capacity_as_expr() { + let capacity = 10; + let mut v2: Vec<usize> = Vec::with_capacity(capacity); +} + +// Shouldn't lint +fn vec_init_with_argument() { + let mut v3 = vec![1]; + v3.reserve(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! { + let mut v: Vec<usize> = vec![]; + v.reserve(10); + } + + with_span! { + span + + let mut v: Vec<usize> = vec![]; + v.reserve(10); + } +} diff --git a/tests/ui/reserve_after_initialization.rs b/tests/ui/reserve_after_initialization.rs new file mode 100644 index 00000000000..b57a8e162c5 --- /dev/null +++ b/tests/ui/reserve_after_initialization.rs @@ -0,0 +1,51 @@ +//@aux-build:proc_macros.rs +#![warn(clippy::reserve_after_initialization)] +#![no_main] + +extern crate proc_macros; +use proc_macros::{external, with_span}; + +// Should lint +fn standard() { + let mut v1: Vec<usize> = vec![]; + v1.reserve(10); +} + +// Should lint +fn capacity_as_expr() { + let capacity = 10; + let mut v2: Vec<usize> = vec![]; + v2.reserve(capacity); +} + +// Shouldn't lint +fn vec_init_with_argument() { + let mut v3 = vec![1]; + v3.reserve(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! { + let mut v: Vec<usize> = vec![]; + v.reserve(10); + } + + with_span! { + span + + let mut v: Vec<usize> = vec![]; + v.reserve(10); + } +} diff --git a/tests/ui/reserve_after_initialization.stderr b/tests/ui/reserve_after_initialization.stderr new file mode 100644 index 00000000000..4a6164d8ebc --- /dev/null +++ b/tests/ui/reserve_after_initialization.stderr @@ -0,0 +1,25 @@ +error: call to `reserve` immediately after creation + --> $DIR/reserve_after_initialization.rs:10: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);` + | + = note: `-D clippy::reserve-after-initialization` implied by `-D warnings` + +error: call to `reserve` immediately after creation + --> $DIR/reserve_after_initialization.rs:17: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);` + +error: call to `reserve` immediately after creation + --> $DIR/reserve_after_initialization.rs:35: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 + |
