unnecessary_reserve: address review comments

Author: wowinter13

clippy_config/src/conf.rs

@@ -647,6 +647,7 @@ define_Conf! {
         unchecked_duration_subtraction,
         uninlined_format_args,
         unnecessary_lazy_evaluations,
+        unnecessary_reserve,
         unnested_or_patterns,
         unused_trait_names,
         use_self,

clippy_lints/src/unnecessary_reserve.rs

@@ -1,13 +1,13 @@
 use clippy_config::Conf;
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::ty::adt_def_id;
 use clippy_utils::visitors::for_each_expr;
 use clippy_utils::{SpanlessEq, get_enclosing_block, match_def_path, paths};
 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::impl_lint_pass;
 use rustc_span::sym;
 
@@ -16,7 +16,7 @@ declare_clippy_lint! {
     ///
     /// 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`
+    /// `extend` implicitly calls `reserve`
     ///
     /// ### Example
     /// ```rust
@@ -31,9 +31,9 @@ declare_clippy_lint! {
     /// let array: &[usize] = &[1, 2];
     /// vec.extend(array);
     /// ```
-    #[clippy::version = "1.64.0"]
+    #[clippy::version = "1.86.0"]
     pub UNNECESSARY_RESERVE,
-    pedantic,
+    complexity,
     "calling `reserve` before `extend` on a `Vec` or `VecDeque`, when it will be called implicitly"
 }
 
@@ -84,13 +84,11 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryReserve {
 }
 
 fn acceptable_type(cx: &LateContext<'_>, struct_calling_on: &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,
-        }
-    })
+    if let Some(did) = adt_def_id(cx.typeck_results().expr_ty_adjusted(struct_calling_on)) {
+        matches!(cx.tcx.get_diagnostic_name(did), Some(sym::Vec | sym::VecDeque))
+    } else {
+        false
+    }
 }
 
 #[must_use]
@@ -100,19 +98,26 @@ fn check_extend_method<'tcx>(
     struct_expr: &Expr<'tcx>,
     args_a: &Expr<'tcx>,
 ) -> Option<rustc_span::Span> {
-    let args_a_kind = &args_a.kind;
+    let mut found_reserve = false;
     let mut read_found = false;
     let mut spanless_eq = SpanlessEq::new(cx);
 
     let _: Option<!> = for_each_expr(cx, block, |expr: &Expr<'tcx>| {
+        if !found_reserve {
+            if expr.hir_id == args_a.hir_id {
+                found_reserve = true;
+            }
+            return ControlFlow::Continue(());
+        }
+
         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
+            ) = args_a.kind
             && method_call_a.name == sym::len
             && match_def_path(cx, expr_def_id, &paths::ITER_EXTEND)
             && acceptable_type(cx, struct_calling_on)

clippy_utils/src/ty/mod.rs

@@ -1352,3 +1352,8 @@ pub fn option_arg_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'t
         _ => None,
     }
 }
+
+/// Check if `ty` with references peeled is an `Adt` and return its `DefId`.
+pub fn adt_def_id(ty: Ty<'_>) -> Option<DefId> {
+    ty.peel_refs().ty_adt_def().map(AdtDef::did)
+}

clippy_utils/src/ty/type_certainty/mod.rs

@@ -11,13 +11,14 @@
 //! As a heuristic, `expr_type_is_certain` may produce false negatives, but a false positive should
 //! be considered a bug.
 
+use super::adt_def_id;
 use crate::def_path_res;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::{InferKind, Visitor, VisitorExt, walk_qpath, walk_ty};
 use rustc_hir::{self as hir, AmbigArg, Expr, ExprKind, GenericArgs, HirId, Node, PathSegment, QPath, TyKind};
 use rustc_lint::LateContext;
-use rustc_middle::ty::{self, AdtDef, GenericArgKind, Ty};
+use rustc_middle::ty::{self, GenericArgKind, Ty};
 use rustc_span::{Span, Symbol};
 
 mod certainty;
@@ -313,10 +314,6 @@ fn self_ty<'tcx>(cx: &LateContext<'tcx>, method_def_id: DefId) -> Ty<'tcx> {
     cx.tcx.fn_sig(method_def_id).skip_binder().inputs().skip_binder()[0]
 }
 
-fn adt_def_id(ty: Ty<'_>) -> Option<DefId> {
-    ty.peel_refs().ty_adt_def().map(AdtDef::did)
-}
-
 fn contains_param(ty: Ty<'_>, index: u32) -> bool {
     ty.walk()
         .any(|arg| matches!(arg.unpack(), GenericArgKind::Type(ty) if ty.is_param(index)))

tests/ui/unnecessary_reserve.fixed

@@ -9,6 +9,7 @@ fn main() {
     vec_deque_reserve();
     hash_map_reserve();
     msrv_1_62();
+    box_vec_reserve();
 }
 
 fn vec_reserve() {
@@ -97,3 +98,12 @@ fn msrv_1_62() {
     vec_deque.reserve(1);
     vec_deque.extend([1]);
 }
+
+fn box_vec_reserve() {
+    let mut vec: Box<Vec<usize>> = Box::default();
+    let array: &[usize] = &[1, 2];
+
+    // do lint
+    ;
+    vec.extend(array);
+}

tests/ui/unnecessary_reserve.rs

@@ -9,6 +9,7 @@ fn main() {
     vec_deque_reserve();
     hash_map_reserve();
     msrv_1_62();
+    box_vec_reserve();
 }
 
 fn vec_reserve() {
@@ -98,3 +99,12 @@ fn msrv_1_62() {
     vec_deque.reserve(1);
     vec_deque.extend([1]);
 }
+
+fn box_vec_reserve() {
+    let mut vec: Box<Vec<usize>> = Box::default();
+    let array: &[usize] = &[1, 2];
+
+    // do lint
+    vec.reserve(array.len());
+    vec.extend(array);
+}

tests/ui/unnecessary_reserve.stderr

@@ -7,7 +7,7 @@ LL | #[allow(clippy::unnecessary_operation)]
    = note: `#[deny(clippy::useless_attribute)]` on by default
 
 error: unnecessary call to `reserve`
-  --> tests/ui/unnecessary_reserve.rs:14:18
+  --> tests/ui/unnecessary_reserve.rs:15:18
    |
 LL |   fn vec_reserve() {
    |  __________________^
@@ -26,7 +26,7 @@ LL | | }
    = help: to override `-D warnings` add `#[allow(clippy::unnecessary_reserve)]`
 
 error: unnecessary call to `reserve`
-  --> tests/ui/unnecessary_reserve.rs:14:18
+  --> tests/ui/unnecessary_reserve.rs:15:18
    |
 LL |   fn vec_reserve() {
    |  __________________^
@@ -42,7 +42,7 @@ LL | | }
    | |_^
 
 error: unnecessary call to `reserve`
-  --> tests/ui/unnecessary_reserve.rs:32:5
+  --> tests/ui/unnecessary_reserve.rs:33:5
    |
 LL | /     {
 LL | |         vec.reserve(array1.len());
@@ -52,7 +52,7 @@ LL | |     };
    | |_____^
 
 error: unnecessary call to `reserve`
-  --> tests/ui/unnecessary_reserve.rs:14:18
+  --> tests/ui/unnecessary_reserve.rs:15:18
    |
 LL |   fn vec_reserve() {
    |  __________________^
@@ -68,7 +68,7 @@ LL | | }
    | |_^
 
 error: call to `reserve` immediately after creation
-  --> tests/ui/unnecessary_reserve.rs:43:5
+  --> tests/ui/unnecessary_reserve.rs:44:5
    |
 LL | /     let mut other_vec: Vec<usize> = vec![];
 LL | |     other_vec.reserve(1);
@@ -78,7 +78,7 @@ LL | |     other_vec.reserve(1);
    = help: to override `-D warnings` add `#[allow(clippy::reserve_after_initialization)]`
 
 error: unnecessary call to `reserve`
-  --> tests/ui/unnecessary_reserve.rs:48:24
+  --> tests/ui/unnecessary_reserve.rs:49:24
    |
 LL |   fn vec_deque_reserve() {
    |  ________________________^
@@ -92,5 +92,19 @@ LL | |     vec_deque.extend([1])
 LL | | }
    | |_^
 
-error: aborting due to 7 previous errors
+error: unnecessary call to `reserve`
+  --> tests/ui/unnecessary_reserve.rs:103:22
+   |
+LL |   fn box_vec_reserve() {
+   |  ______________________^
+LL | |     let mut vec: Box<Vec<usize>> = Box::default();
+LL | |     let array: &[usize] = &[1, 2];
+...  |
+LL | |     vec.reserve(array.len());
+   | |     ------------------------ help: remove this line
+LL | |     vec.extend(array);
+LL | | }
+   | |_^
+
+error: aborting due to 8 previous errors