diff --git a/CHANGELOG.md b/CHANGELOG.md index fe64283462d1..4e33cb7b4570 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 bab77f912691..1be9720fbbf8 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 358004cf460b..f50019f3cf76 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::::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 000000000000..0c8c904e3745 --- /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 = vec![]; + /// v.reserve(10); + /// ``` + /// Use instead: + /// ```rust + /// let mut v: Vec = 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, +} + +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 000000000000..0675277849ad --- /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 = Vec::with_capacity(10); +} + +// Should lint +fn capacity_as_expr() { + let capacity = 10; + let mut v2: Vec = 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 = Vec::with_capacity(10); +} + +// Should lint +fn assign_expression() { + let mut v5: Vec = Vec::new(); + v5 = Vec::with_capacity(10); +} + +fn in_macros() { + external! { + let mut v: Vec = vec![]; + v.reserve(10); + } + + with_span! { + span + + let mut v: Vec = 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 000000000000..b57a8e162c53 --- /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 = vec![]; + v1.reserve(10); +} + +// Should lint +fn capacity_as_expr() { + let capacity = 10; + let mut v2: Vec = 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 = 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! { + let mut v: Vec = vec![]; + v.reserve(10); + } + + with_span! { + span + + let mut v: Vec = 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 000000000000..4a6164d8ebc9 --- /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 = vec![]; +LL | | v1.reserve(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: call to `reserve` immediately after creation + --> $DIR/reserve_after_initialization.rs:17: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);` + +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 +