diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 2982460c9cfa..c6a0b84a4ee3 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -602,6 +602,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::unnamed_address::FN_ADDRESS_COMPARISONS_INFO, crate::unnamed_address::VTABLE_ADDRESS_COMPARISONS_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_wraps::UNNECESSARY_WRAPS_INFO, crate::unnested_or_patterns::UNNESTED_OR_PATTERNS_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index dee1ddb2c49a..ec1ba1eea8c5 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -909,7 +909,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(fn_null_check::FnNullCheck)); store.register_late_pass(|_| Box::new(permissions_set_readonly_false::PermissionsSetReadonlyFalse)); store.register_late_pass(|_| Box::new(size_of_ref::SizeOfRef)); - store.register_late_pass(move || Box::new(unnecessary_reserve::UnnecessaryReserve::new(msrv))); + 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 index 261945d39afb..fa222a4421b7 100644 --- a/clippy_lints/src/unnecessary_reserve.rs +++ b/clippy_lints/src/unnecessary_reserve.rs @@ -1,18 +1,21 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::{match_def_path, meets_msrv, msrvs, paths, visitors::expr_visitor_no_bodies}; +use clippy_utils::msrvs::{self, Msrv}; +use clippy_utils::{match_def_path, paths, visitors::for_each_expr, SpanlessEq}; +use core::ops::ControlFlow; use rustc_errors::Applicability; -use rustc_hir::{intravisit::Visitor, Block, ExprKind, QPath, StmtKind}; +use rustc_hir::{Block, ExprKind, PathSegment, 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![]; @@ -32,31 +35,27 @@ declare_clippy_lint! { "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: Option, + msrv: Msrv, } - impl UnnecessaryReserve { - #[must_use] - pub fn new(msrv: Option) -> Self { + pub fn new(msrv: Msrv) -> 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) { + if !self.msrv.meets(msrvs::CHECK_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)) + && let ExprKind::MethodCall(PathSegment { ident: method, .. }, struct_calling_on, _, _) = semi_expr.kind + && method.name.as_str() == "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() @@ -102,32 +101,32 @@ fn check_extend_method( ) -> Option { let mut read_found = false; let next_stmt_span; + let mut spanless_eq = SpanlessEq::new(cx); - let mut visitor = expr_visitor_no_bodies(|expr| { - if let ExprKind::MethodCall(_, [struct_calling_on, _], _) = expr.kind + 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) && match_def_path(cx, expr_def_id, &paths::ITER_EXTEND) && acceptable_type(cx, struct_calling_on) - && equal_ident(struct_calling_on, struct_expr) + // Check that both expr are equal + && spanless_eq.eq_expr(struct_calling_on, struct_expr) { read_found = true; } - !read_found + let _ = !read_found; + ControlFlow::Continue(()) }); 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); @@ -135,17 +134,3 @@ fn check_extend_method( 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 0f1f2acdfe1f..2dbbf6c040f9 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -20,7 +20,7 @@ macro_rules! msrv_aliases { // names may refer to stabilized feature flags or library items msrv_aliases! { 1,65,0 { LET_ELSE } - 1,62,0 { BOOL_THEN_SOME, UNNECESSARY_RESERVE } + 1,62,0 { BOOL_THEN_SOME, CHECK_UNNECESSARY_RESERVE } 1,58,0 { FORMAT_ARGS_CAPTURE, PATTERN_TRAIT_CHAR_ARRAY } 1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN, ARRAY_INTO_ITERATOR } 1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST } diff --git a/tests/ui/unnecessary_reserve.rs b/tests/ui/unnecessary_reserve.rs index 372c7f434ea5..f67d16e17bd3 100644 --- a/tests/ui/unnecessary_reserve.rs +++ b/tests/ui/unnecessary_reserve.rs @@ -6,27 +6,27 @@ use std::collections::VecDeque; fn main() { vec_reserve(); - vec_deque_reserve(); - hash_map_reserve(); - msrv_1_62(); + //vec_deque_reserve(); + //hash_map_reserve(); + //msrv_1_62(); } fn vec_reserve() { let mut vec: Vec = vec![]; let array: &[usize] = &[1, 2]; - // do lint + // do not lint vec.reserve(1); vec.extend([1]); - // do lint + //// do lint vec.reserve(array.len()); vec.extend(array); // do lint { - vec.reserve(1); - vec.extend([1]) + vec.reserve(array.len()); + vec.extend(array) }; // do not lint @@ -44,7 +44,7 @@ fn vec_deque_reserve() { let mut vec_deque: VecDeque = [1].into(); let array: &[usize] = &[1, 2]; - // do lint + // do not lint vec_deque.reserve(1); vec_deque.extend([1]); @@ -52,7 +52,7 @@ fn vec_deque_reserve() { vec_deque.reserve(array.len()); vec_deque.extend(array); - // do lint + // 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 index 75c7d58e7301..65f62d5d6d6f 100644 --- a/tests/ui/unnecessary_reserve.stderr +++ b/tests/ui/unnecessary_reserve.stderr @@ -16,13 +16,21 @@ LL | vec.reserve(array.len()); LL | vec.extend(array); | ^^^^^^^^^^^^^^^^^^ +error: unnecessary call to `reserve` + --> $DIR/unnecessary_reserve.rs:34:5 + | +LL | vec.reserve(array.len()); + | ------------------------ help: remove this line +LL | vec.push(1); + | ^^^^^^^^^^^^ + error: unnecessary call to `reserve` --> $DIR/unnecessary_reserve.rs:29:9 | -LL | vec.reserve(1); - | -------------- help: remove this line -LL | vec.extend([1]) - | ^^^^^^^^^^^^^^^ +LL | vec.reserve(array.len()); + | ------------------------ help: remove this line +LL | vec.extend(array) + | ^^^^^^^^^^^^^^^^^ error: unnecessary call to `reserve` --> $DIR/unnecessary_reserve.rs:49:5 @@ -40,6 +48,14 @@ LL | vec_deque.reserve(array.len()); LL | vec_deque.extend(array); | ^^^^^^^^^^^^^^^^^^^^^^^^ +error: unnecessary call to `reserve` + --> $DIR/unnecessary_reserve.rs:63:5 + | +LL | vec_deque.reserve(array.len() + 1); + | ---------------------------------- help: remove this line +LL | vec_deque.push_back(1); + | ^^^^^^^^^^^^^^^^^^^^^^^ + error: unnecessary call to `reserve` --> $DIR/unnecessary_reserve.rs:58:9 | @@ -48,5 +64,5 @@ LL | vec_deque.reserve(1); LL | vec_deque.extend([1]) | ^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 6 previous errors +error: aborting due to 8 previous errors