diff --git a/CHANGELOG.md b/CHANGELOG.md index d74ec71a0e8b..504ebd167c5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4127,6 +4127,7 @@ Released 2018-09-13 [`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation [`unnecessary_owned_empty_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings +[`unnecessary_reserve`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_reserve [`unnecessary_self_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_self_imports [`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by [`unnecessary_to_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index c540573b8022..0bc56d55fc19 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -563,6 +563,7 @@ store.register_lints(&[ unnamed_address::FN_ADDRESS_COMPARISONS, unnamed_address::VTABLE_ADDRESS_COMPARISONS, unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS, + unnecessary_reserve::UNNECESSARY_RESERVE, unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS, unnecessary_sort_by::UNNECESSARY_SORT_BY, unnecessary_wraps::UNNECESSARY_WRAPS, diff --git a/clippy_lints/src/lib.register_pedantic.rs b/clippy_lints/src/lib.register_pedantic.rs index 4250ee055e6c..69fbc625983b 100644 --- a/clippy_lints/src/lib.register_pedantic.rs +++ b/clippy_lints/src/lib.register_pedantic.rs @@ -91,6 +91,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![ LintId::of(types::LINKEDLIST), LintId::of(types::OPTION_OPTION), LintId::of(unicode::UNICODE_NOT_NFC), + LintId::of(unnecessary_reserve::UNNECESSARY_RESERVE), LintId::of(unnecessary_wraps::UNNECESSARY_WRAPS), LintId::of(unnested_or_patterns::UNNESTED_OR_PATTERNS), LintId::of(unused_async::UNUSED_ASYNC), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index e6a405f8170d..9ee3ba7ce2dc 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -390,6 +390,7 @@ mod unit_return_expecting_ord; mod unit_types; mod unnamed_address; mod unnecessary_owned_empty_strings; +mod unnecessary_reserve; mod unnecessary_self_imports; mod unnecessary_sort_by; mod unnecessary_wraps; @@ -933,6 +934,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports::default())); store.register_late_pass(|| Box::new(manual_instant_elapsed::ManualInstantElapsed)); store.register_late_pass(|| Box::new(partialeq_to_none::PartialeqToNone)); + store.register_late_pass(move || Box::new(unnecessary_reserve::UnnecessaryReserve::new(msrv))); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/unnecessary_reserve.rs b/clippy_lints/src/unnecessary_reserve.rs new file mode 100644 index 000000000000..261945d39afb --- /dev/null +++ b/clippy_lints/src/unnecessary_reserve.rs @@ -0,0 +1,151 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::{match_def_path, meets_msrv, msrvs, paths, visitors::expr_visitor_no_bodies}; +use rustc_errors::Applicability; +use rustc_hir::{intravisit::Visitor, Block, ExprKind, QPath, StmtKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +use rustc_semver::RustcVersion; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::sym; + +declare_clippy_lint! { + /// ### What it does + /// This lint checks for a call to `reserve` before `extend` on a `Vec` or `VecDeque`. + /// ### Why is this bad? + /// Since Rust 1.62, `extend` implicitly calls `reserve` + /// ### Example + /// ```rust + /// let mut vec: Vec = vec![]; + /// let array: &[usize] = &[1, 2]; + /// vec.reserve(array.len()); + /// vec.extend(array); + /// ``` + /// Use instead: + /// ```rust + /// let mut vec: Vec = vec![]; + /// let array: &[usize] = &[1, 2]; + /// vec.extend(array); + /// ``` + #[clippy::version = "1.64.0"] + pub UNNECESSARY_RESERVE, + pedantic, + "calling `reserve` before `extend` on a `Vec` or `VecDeque`, when it will be called implicitly" +} + +pub struct UnnecessaryReserve { + msrv: Option, +} + +impl UnnecessaryReserve { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} + +impl_lint_pass!(UnnecessaryReserve => [UNNECESSARY_RESERVE]); + +impl<'tcx> LateLintPass<'tcx> for UnnecessaryReserve { + fn check_block(&mut self, cx: &LateContext<'tcx>, block: &Block<'tcx>) { + if !meets_msrv(self.msrv, msrvs::UNNECESSARY_RESERVE) { + return; + } + + for (idx, stmt) in block.stmts.iter().enumerate() { + if let StmtKind::Semi(semi_expr) = stmt.kind + && let ExprKind::MethodCall(_, [struct_calling_on, _], _) = semi_expr.kind + && let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(semi_expr.hir_id) + && (match_def_path(cx, expr_def_id, &paths::VEC_RESERVE) || + match_def_path(cx, expr_def_id, &paths::VEC_DEQUE_RESERVE)) + && acceptable_type(cx, struct_calling_on) + && let Some(next_stmt_span) = check_extend_method(cx, block, idx, struct_calling_on) + && !next_stmt_span.from_expansion() + { + span_lint_and_then( + cx, + UNNECESSARY_RESERVE, + next_stmt_span, + "unnecessary call to `reserve`", + |diag| { + diag.span_suggestion( + semi_expr.span, + "remove this line", + String::new(), + Applicability::MaybeIncorrect, + ); + } + ); + } + } + } + + extract_msrv_attr!(LateContext); +} + +#[must_use] +fn acceptable_type(cx: &LateContext<'_>, struct_calling_on: &rustc_hir::Expr<'_>) -> bool { + let acceptable_types = [sym::Vec, sym::VecDeque]; + acceptable_types.iter().any(|&acceptable_ty| { + match cx.typeck_results().expr_ty(struct_calling_on).peel_refs().kind() { + ty::Adt(def, _) => cx.tcx.is_diagnostic_item(acceptable_ty, def.did()), + _ => false, + } + }) +} + +#[must_use] +fn check_extend_method( + cx: &LateContext<'_>, + block: &Block<'_>, + idx: usize, + struct_expr: &rustc_hir::Expr<'_>, +) -> Option { + let mut read_found = false; + let next_stmt_span; + + let mut visitor = expr_visitor_no_bodies(|expr| { + if let ExprKind::MethodCall(_, [struct_calling_on, _], _) = expr.kind + && let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) + && match_def_path(cx, expr_def_id, &paths::ITER_EXTEND) + && acceptable_type(cx, struct_calling_on) + && equal_ident(struct_calling_on, struct_expr) + { + read_found = true; + } + !read_found + }); + + if idx == block.stmts.len() - 1 { + if let Some(e) = block.expr { + visitor.visit_expr(e); + next_stmt_span = e.span; + } else { + return None; + } + } else { + let next_stmt = &block.stmts[idx + 1]; + visitor.visit_stmt(next_stmt); + next_stmt_span = next_stmt.span; + } + drop(visitor); + + if read_found { + return Some(next_stmt_span); + } + + None +} + +#[must_use] +fn equal_ident(left: &rustc_hir::Expr<'_>, right: &rustc_hir::Expr<'_>) -> bool { + fn ident_name(expr: &rustc_hir::Expr<'_>) -> Option { + if let ExprKind::Path(QPath::Resolved(None, inner_path)) = expr.kind + && let [inner_seg] = inner_path.segments + { + return Some(inner_seg.ident.name); + } + None + } + + ident_name(left) == ident_name(right) +} diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 9e238c6f1ac0..bb05fdfffc8c 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -12,7 +12,7 @@ macro_rules! msrv_aliases { // names may refer to stabilized feature flags or library items msrv_aliases! { - 1,62,0 { BOOL_THEN_SOME } + 1,62,0 { BOOL_THEN_SOME, UNNECESSARY_RESERVE } 1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN } 1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST } 1,51,0 { BORROW_AS_PTR, UNSIGNED_ABS } diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 8d697a301c44..c72a999843bd 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -70,6 +70,7 @@ pub const IPADDR_V4: [&str; 5] = ["std", "net", "ip", "IpAddr", "V4"]; pub const IPADDR_V6: [&str; 5] = ["std", "net", "ip", "IpAddr", "V6"]; pub const ITER_COUNT: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "count"]; pub const ITER_EMPTY: [&str; 5] = ["core", "iter", "sources", "empty", "Empty"]; +pub const ITER_EXTEND: [&str; 6] = ["core", "iter", "traits", "collect", "Extend", "extend"]; pub const ITER_REPEAT: [&str; 5] = ["core", "iter", "sources", "repeat", "repeat"]; #[expect(clippy::invalid_paths)] // internal lints do not know about all external crates pub const ITERTOOLS_NEXT_TUPLE: [&str; 3] = ["itertools", "Itertools", "next_tuple"]; @@ -191,6 +192,8 @@ pub const VEC_DEQUE_ITER: [&str; 5] = ["alloc", "collections", "vec_deque", "Vec pub const VEC_FROM_ELEM: [&str; 3] = ["alloc", "vec", "from_elem"]; pub const VEC_NEW: [&str; 4] = ["alloc", "vec", "Vec", "new"]; pub const VEC_RESIZE: [&str; 4] = ["alloc", "vec", "Vec", "resize"]; +pub const VEC_RESERVE: [&str; 4] = ["alloc", "vec", "Vec", "reserve"]; +pub const VEC_DEQUE_RESERVE: [&str; 5] = ["alloc", "collections", "vec_deque", "VecDeque", "reserve"]; pub const WEAK_ARC: [&str; 3] = ["alloc", "sync", "Weak"]; pub const WEAK_RC: [&str; 3] = ["alloc", "rc", "Weak"]; pub const PTR_NON_NULL: [&str; 4] = ["core", "ptr", "non_null", "NonNull"]; diff --git a/tests/ui/unnecessary_reserve.rs b/tests/ui/unnecessary_reserve.rs new file mode 100644 index 000000000000..372c7f434ea5 --- /dev/null +++ b/tests/ui/unnecessary_reserve.rs @@ -0,0 +1,95 @@ +#![warn(clippy::unnecessary_reserve)] +#![feature(custom_inner_attributes)] + +use std::collections::HashMap; +use std::collections::VecDeque; + +fn main() { + vec_reserve(); + vec_deque_reserve(); + hash_map_reserve(); + msrv_1_62(); +} + +fn vec_reserve() { + let mut vec: Vec = vec![]; + let array: &[usize] = &[1, 2]; + + // do lint + vec.reserve(1); + vec.extend([1]); + + // do lint + vec.reserve(array.len()); + vec.extend(array); + + // do lint + { + vec.reserve(1); + vec.extend([1]) + }; + + // do not lint + vec.reserve(array.len()); + vec.push(1); + vec.extend(array); + + // do not lint + let mut other_vec: Vec = vec![]; + other_vec.reserve(1); + vec.extend([1]) +} + +fn vec_deque_reserve() { + let mut vec_deque: VecDeque = [1].into(); + let array: &[usize] = &[1, 2]; + + // do lint + vec_deque.reserve(1); + vec_deque.extend([1]); + + // do lint + vec_deque.reserve(array.len()); + vec_deque.extend(array); + + // do lint + { + vec_deque.reserve(1); + vec_deque.extend([1]) + }; + + // do not lint + vec_deque.reserve(array.len() + 1); + vec_deque.push_back(1); + vec_deque.extend(array); + + // do not lint + let mut other_vec_deque: VecDeque = [1].into(); + other_vec_deque.reserve(1); + vec_deque.extend([1]) +} + +fn hash_map_reserve() { + let mut map: HashMap = HashMap::new(); + let mut other_map: HashMap = HashMap::new(); + // do not lint + map.reserve(other_map.len()); + map.extend(other_map); +} + +fn msrv_1_62() { + #![clippy::msrv = "1.61"] + let mut vec: Vec = vec![]; + let array: &[usize] = &[1, 2]; + + // do not lint + vec.reserve(1); + vec.extend([1]); + + let mut vec_deque: VecDeque = [1].into(); + let array: &[usize] = &[1, 2]; + + // do not lint + vec_deque.reserve(1); + vec_deque.extend([1]); +} diff --git a/tests/ui/unnecessary_reserve.stderr b/tests/ui/unnecessary_reserve.stderr new file mode 100644 index 000000000000..75c7d58e7301 --- /dev/null +++ b/tests/ui/unnecessary_reserve.stderr @@ -0,0 +1,52 @@ +error: unnecessary call to `reserve` + --> $DIR/unnecessary_reserve.rs:20:5 + | +LL | vec.reserve(1); + | -------------- help: remove this line +LL | vec.extend([1]); + | ^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::unnecessary-reserve` implied by `-D warnings` + +error: unnecessary call to `reserve` + --> $DIR/unnecessary_reserve.rs:24:5 + | +LL | vec.reserve(array.len()); + | ------------------------ help: remove this line +LL | vec.extend(array); + | ^^^^^^^^^^^^^^^^^^ + +error: unnecessary call to `reserve` + --> $DIR/unnecessary_reserve.rs:29:9 + | +LL | vec.reserve(1); + | -------------- help: remove this line +LL | vec.extend([1]) + | ^^^^^^^^^^^^^^^ + +error: unnecessary call to `reserve` + --> $DIR/unnecessary_reserve.rs:49:5 + | +LL | vec_deque.reserve(1); + | -------------------- help: remove this line +LL | vec_deque.extend([1]); + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: unnecessary call to `reserve` + --> $DIR/unnecessary_reserve.rs:53:5 + | +LL | vec_deque.reserve(array.len()); + | ------------------------------ help: remove this line +LL | vec_deque.extend(array); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: unnecessary call to `reserve` + --> $DIR/unnecessary_reserve.rs:58:9 + | +LL | vec_deque.reserve(1); + | -------------------- help: remove this line +LL | vec_deque.extend([1]) + | ^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors +