diff --git a/CHANGELOG.md b/CHANGELOG.md index 559b560dde4b..abb5376f1783 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4990,6 +4990,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_safety_comment`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_comment [`unnecessary_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_doc [`unnecessary_self_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_self_imports diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index f24dab627809..9836b752a645 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -624,6 +624,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::unnamed_address::VTABLE_ADDRESS_COMPARISONS_INFO, crate::unnecessary_box_returns::UNNECESSARY_BOX_RETURNS_INFO, crate::unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO, + crate::unnecessary_reserve::UNNECESSARY_RESERVE_INFO, crate::unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS_INFO, crate::unnecessary_struct_initialization::UNNECESSARY_STRUCT_INITIALIZATION_INFO, crate::unnecessary_wraps::UNNECESSARY_WRAPS_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index b0ec14855e71..e0b1351b830e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -305,6 +305,7 @@ mod unit_types; mod unnamed_address; mod unnecessary_box_returns; mod unnecessary_owned_empty_strings; +mod unnecessary_reserve; mod unnecessary_self_imports; mod unnecessary_struct_initialization; mod unnecessary_wraps; @@ -959,6 +960,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(tests_outside_test_module::TestsOutsideTestModule)); store.register_late_pass(|_| Box::new(manual_slice_size_calculation::ManualSliceSizeCalculation)); store.register_early_pass(|| Box::new(suspicious_doc_comments::SuspiciousDocComments)); + 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..03fb7c31d64d --- /dev/null +++ b/clippy_lints/src/unnecessary_reserve.rs @@ -0,0 +1,121 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::msrvs::{self, Msrv}; +use clippy_utils::{get_enclosing_block, match_def_path, paths, visitors::for_each_expr, SpanlessEq}; +use core::ops::ControlFlow; +use rustc_errors::Applicability; +use rustc_hir::{Block, Expr, ExprKind, PathSegment}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +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" +} + +impl_lint_pass!(UnnecessaryReserve => [UNNECESSARY_RESERVE]); + +pub struct UnnecessaryReserve { + msrv: Msrv, +} +impl UnnecessaryReserve { + pub fn new(msrv: Msrv) -> Self { + Self { msrv } + } +} + +impl<'tcx> LateLintPass<'tcx> for UnnecessaryReserve { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { + if !self.msrv.meets(msrvs::EXTEND_IMPLICIT_RESERVE) { + return; + } + + if let ExprKind::MethodCall(PathSegment { ident: method, .. }, struct_calling_on, args_a, _) = expr.kind + && method.name.as_str() == "reserve" + && acceptable_type(cx, struct_calling_on) + && let Some(block) = get_enclosing_block(cx, expr.hir_id) + && let Some(next_stmt_span) = check_extend_method(cx, block, struct_calling_on, &args_a[0]) + && !next_stmt_span.from_expansion() + { + span_lint_and_then( + cx, + UNNECESSARY_RESERVE, + next_stmt_span, + "unnecessary call to `reserve`", + |diag| { + diag.span_suggestion( + 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<'_>, + struct_expr: &rustc_hir::Expr<'_>, + args_a: &rustc_hir::Expr<'_>, +) -> Option { + let args_a_kind = &args_a.kind; + let mut read_found = false; + let mut spanless_eq = SpanlessEq::new(cx); + + let _: Option = for_each_expr(block, |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) + && let ExprKind::MethodCall(PathSegment { ident: method_call_a, .. },..) = args_a_kind + && method_call_a.name == rustc_span::sym::len + && match_def_path(cx, expr_def_id, &paths::ITER_EXTEND) + && acceptable_type(cx, struct_calling_on) + // Check that both expr are equal + && spanless_eq.eq_expr(struct_calling_on, struct_expr) + { + read_found = true; + } + let _: bool = !read_found; + ControlFlow::Continue(()) + }); + + if read_found { Some(block.span) } else { None } +} diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index e05de2dc99c0..b22deaea6ec2 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -21,7 +21,7 @@ macro_rules! msrv_aliases { msrv_aliases! { 1,68,0 { PATH_MAIN_SEPARATOR_STR } 1,65,0 { LET_ELSE } - 1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE } + 1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE, EXTEND_IMPLICIT_RESERVE } 1,58,0 { FORMAT_ARGS_CAPTURE, PATTERN_TRAIT_CHAR_ARRAY } 1,55,0 { SEEK_REWIND } 1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN, ARRAY_INTO_ITERATOR } diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 9be2d0eae80a..1a875d3f7b50 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -49,6 +49,7 @@ pub const IDENT: [&str; 3] = ["rustc_span", "symbol", "Ident"]; pub const IDENT_AS_STR: [&str; 4] = ["rustc_span", "symbol", "Ident", "as_str"]; pub const INSERT_STR: [&str; 4] = ["alloc", "string", "String", "insert_str"]; 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 ITERTOOLS_NEXT_TUPLE: [&str; 3] = ["itertools", "Itertools", "next_tuple"]; #[cfg(feature = "internal")] pub const KW_MODULE: [&str; 3] = ["rustc_span", "symbol", "kw"]; @@ -154,6 +155,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..0bf826e181e9 --- /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 not lint + vec.reserve(1); + vec.extend([1]); + + //// do lint + vec.reserve(array.len()); + vec.extend(array); + + // do lint + { + vec.reserve(array.len()); + vec.extend(array) + }; + + // 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 not lint + vec_deque.reserve(1); + vec_deque.extend([1]); + + // do lint + vec_deque.reserve(array.len()); + vec_deque.extend(array); + + // do not 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..23cb227924cc --- /dev/null +++ b/tests/ui/unnecessary_reserve.stderr @@ -0,0 +1,62 @@ +error: unnecessary call to `reserve` + --> $DIR/unnecessary_reserve.rs:14:18 + | +LL | fn vec_reserve() { + | __________________^ +LL | | let mut vec: Vec = vec![]; +LL | | let array: &[usize] = &[1, 2]; +LL | | +... | +LL | | vec.reserve(array.len()); + | | ------------------------ help: remove this line +... | +LL | | vec.extend([1]) +LL | | } + | |_^ + | + = note: `-D clippy::unnecessary-reserve` implied by `-D warnings` + +error: unnecessary call to `reserve` + --> $DIR/unnecessary_reserve.rs:27:5 + | +LL | / { +LL | | vec.reserve(array.len()); + | | ------------------------ help: remove this line +LL | | vec.extend(array) +LL | | }; + | |_____^ + +error: unnecessary call to `reserve` + --> $DIR/unnecessary_reserve.rs:14:18 + | +LL | fn vec_reserve() { + | __________________^ +LL | | let mut vec: Vec = vec![]; +LL | | let array: &[usize] = &[1, 2]; +LL | | +... | +LL | | vec.reserve(array.len()); + | | ------------------------ help: remove this line +... | +LL | | vec.extend([1]) +LL | | } + | |_^ + +error: unnecessary call to `reserve` + --> $DIR/unnecessary_reserve.rs:43:24 + | +LL | fn vec_deque_reserve() { + | ________________________^ +LL | | let mut vec_deque: VecDeque = [1].into(); +LL | | let array: &[usize] = &[1, 2]; +LL | | +... | +LL | | vec_deque.reserve(array.len()); + | | ------------------------------ help: remove this line +... | +LL | | vec_deque.extend([1]) +LL | | } + | |_^ + +error: aborting due to 4 previous errors +