Extract get_vec_init_kind and share it

Author: Qwaz

clippy_lints/src/uninit_vec.rs

@@ -1,41 +1,50 @@
 use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::get_vec_init_kind;
 use clippy_utils::ty::is_type_diagnostic_item;
-use clippy_utils::{
-    match_def_path, path_to_local_id, paths, peel_hir_expr_while, ty::is_uninit_value_valid_for_ty, SpanlessEq,
-};
-use rustc_hir::def::Res;
-use rustc_hir::{Block, Expr, ExprKind, HirId, PatKind, Stmt, StmtKind};
+use clippy_utils::{path_to_local_id, peel_hir_expr_while, ty::is_uninit_value_valid_for_ty, SpanlessEq};
+use rustc_hir::{Block, Expr, ExprKind, HirId, PatKind, PathSegment, Stmt, StmtKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::{sym, Span};
 
+// TODO: add `ReadBuf` (RFC 2930) in "How to fix" once it is available in std
 declare_clippy_lint! {
     /// ### What it does
-    /// Checks for the creation of uninitialized `Vec<T>` by calling `set_len()`
-    /// immediately after `with_capacity()` or `reserve()`.
+    /// Checks for `set_len()` call that creates `Vec` with uninitialized elements.
+    /// This is commonly caused by calling `set_len()` right after after calling
+    /// `with_capacity()` or `reserve()`.
     ///
     /// ### Why is this bad?
-    /// It creates `Vec<T>` that contains uninitialized data, which leads to an
+    /// It creates a `Vec` with uninitialized data, which leads to an
     /// undefined behavior with most safe operations.
-    /// Notably, using uninitialized `Vec<u8>` with generic `Read` is unsound.
+    /// Notably, uninitialized `Vec<u8>` must not be used with generic `Read`.
     ///
     /// ### Example
     /// ```rust,ignore
     /// let mut vec: Vec<u8> = Vec::with_capacity(1000);
     /// unsafe { vec.set_len(1000); }
     /// reader.read(&mut vec); // undefined behavior!
     /// ```
-    /// Use an initialized buffer:
-    /// ```rust,ignore
-    /// let mut vec: Vec<u8> = vec![0; 1000];
-    /// reader.read(&mut vec);
-    /// ```
-    /// Or, wrap the content in `MaybeUninit`:
-    /// ```rust,ignore
-    /// let mut vec: Vec<MaybeUninit<T>> = Vec::with_capacity(1000);
-    /// unsafe { vec.set_len(1000); }
-    /// ```
+    ///
+    /// ### How to fix?
+    /// 1. Use an initialized buffer:
+    ///    ```rust,ignore
+    ///    let mut vec: Vec<u8> = vec![0; 1000];
+    ///    reader.read(&mut vec);
+    ///    ```
+    /// 2. Wrap the content in `MaybeUninit`:
+    ///    ```rust,ignore
+    ///    let mut vec: Vec<MaybeUninit<T>> = Vec::with_capacity(1000);
+    ///    vec.set_len(1000);  // `MaybeUninit` can be uninitialized
+    ///    ```
+    /// 3. If you are on nightly, `Vec::spare_capacity_mut()` is available:
+    ///    ```rust,ignore
+    ///    let mut vec: Vec<u8> = Vec::with_capacity(1000);
+    ///    let remaining = vec.spare_capacity_mut();  // `&mut [MaybeUninit<u8>]`
+    ///    // perform initialization with `remaining`
+    ///    vec.set_len(...);  // Safe to call `set_len()` on initialized part
+    ///    ```
     pub UNINIT_VEC,
     correctness,
     "Vec with uninitialized data"
@@ -59,24 +68,24 @@ impl<'tcx> LateLintPass<'tcx> for UninitVec {
 
 fn handle_uninit_vec_pair(
     cx: &LateContext<'tcx>,
-    maybe_with_capacity_or_reserve: &'tcx Stmt<'tcx>,
+    maybe_init_or_reserve: &'tcx Stmt<'tcx>,
     maybe_set_len: &'tcx Expr<'tcx>,
 ) {
     if_chain! {
-        if let Some(vec) = extract_with_capacity_or_reserve_target(cx, maybe_with_capacity_or_reserve);
+        if let Some(vec) = extract_init_or_reserve_target(cx, maybe_init_or_reserve);
         if let Some((set_len_self, call_span)) = extract_set_len_self(cx, maybe_set_len);
         if vec.eq_expr(cx, set_len_self);
         if let ty::Ref(_, vec_ty, _) = cx.typeck_results().expr_ty_adjusted(set_len_self).kind();
         if let ty::Adt(_, substs) = vec_ty.kind();
         // Check T of Vec<T>
         if !is_uninit_value_valid_for_ty(cx, substs.type_at(0));
         then {
-            // FIXME: false positive #7698
+            // FIXME: #7698, false positive of the internal lints
             #[allow(clippy::collapsible_span_lint_calls)]
             span_lint_and_then(
                 cx,
                 UNINIT_VEC,
-                vec![call_span, maybe_with_capacity_or_reserve.span],
+                vec![call_span, maybe_init_or_reserve.span],
                 "calling `set_len()` immediately after reserving a buffer creates uninitialized values",
                 |diag| {
                     diag.help("initialize the buffer or wrap the content in `MaybeUninit`");
@@ -101,56 +110,36 @@ impl<'tcx> LocalOrExpr<'tcx> {
     }
 }
 
-/// Returns the target vec of `Vec::with_capacity()` or `Vec::reserve()`
-fn extract_with_capacity_or_reserve_target(cx: &LateContext<'_>, stmt: &'tcx Stmt<'_>) -> Option<LocalOrExpr<'tcx>> {
+/// Finds the target location where the result of `Vec` initialization is stored
+/// or `self` expression for `Vec::reserve()`.
+fn extract_init_or_reserve_target<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'tcx>) -> Option<LocalOrExpr<'tcx>> {
     match stmt.kind {
         StmtKind::Local(local) => {
-            // let mut x = Vec::with_capacity()
             if_chain! {
                 if let Some(init_expr) = local.init;
                 if let PatKind::Binding(_, hir_id, _, None) = local.pat.kind;
-                if is_with_capacity(cx, init_expr);
+                if get_vec_init_kind(cx, init_expr).is_some();
                 then {
                     Some(LocalOrExpr::Local(hir_id))
                 } else {
                     None
                 }
             }
         },
-        StmtKind::Expr(expr) | StmtKind::Semi(expr) => {
-            match expr.kind {
-                ExprKind::Assign(lhs, rhs, _span) if is_with_capacity(cx, rhs) => {
-                    // self.vec = Vec::with_capacity()
-                    Some(LocalOrExpr::Expr(lhs))
-                },
-                ExprKind::MethodCall(path, _, [self_expr, _], _) => {
-                    // self.vec.reserve()
-                    if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_expr).peel_refs(), sym::vec_type)
-                        && path.ident.name.as_str() == "reserve"
-                    {
-                        Some(LocalOrExpr::Expr(self_expr))
-                    } else {
-                        None
-                    }
-                },
-                _ => None,
-            }
+        StmtKind::Expr(expr) | StmtKind::Semi(expr) => match expr.kind {
+            ExprKind::Assign(lhs, rhs, _span) if get_vec_init_kind(cx, rhs).is_some() => Some(LocalOrExpr::Expr(lhs)),
+            ExprKind::MethodCall(path, _, [self_expr, _], _) if is_reserve(cx, path, self_expr) => {
+                Some(LocalOrExpr::Expr(self_expr))
+            },
+            _ => None,
         },
         StmtKind::Item(_) => None,
     }
 }
 
-fn is_with_capacity(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> bool {
-    if_chain! {
-        if let ExprKind::Call(path_expr, _) = &expr.kind;
-        if let ExprKind::Path(qpath) = &path_expr.kind;
-        if let Res::Def(_, def_id) = cx.qpath_res(qpath, path_expr.hir_id);
-        then {
-            match_def_path(cx, def_id, &paths::VEC_WITH_CAPACITY)
-        } else {
-            false
-        }
-    }
+fn is_reserve(cx: &LateContext<'_>, path: &PathSegment<'_>, self_expr: &Expr<'_>) -> bool {
+    is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_expr).peel_refs(), sym::Vec)
+        && path.ident.name.as_str() == "reserve"
 }
 
 /// Returns self if the expression is `Vec::set_len()`
@@ -169,14 +158,13 @@ fn extract_set_len_self(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> Option<(&
         }
     });
     match expr.kind {
-        ExprKind::MethodCall(_, _, [vec_expr, _], _) => {
-            cx.typeck_results().type_dependent_def_id(expr.hir_id).and_then(|id| {
-                if match_def_path(cx, id, &paths::VEC_SET_LEN) {
-                    Some((vec_expr, expr.span))
-                } else {
-                    None
-                }
-            })
+        ExprKind::MethodCall(path, _, [self_expr, _], _) => {
+            let self_type = cx.typeck_results().expr_ty(self_expr).peel_refs();
+            if is_type_diagnostic_item(cx, self_type, sym::Vec) && path.ident.name.as_str() == "set_len" {
+                Some((self_expr, expr.span))
+            } else {
+                None
+            }
         },
         _ => None,
     }

clippy_lints/src/vec_init_then_push.rs

@@ -1,16 +1,13 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet;
-use clippy_utils::ty::is_type_diagnostic_item;
-use clippy_utils::{match_def_path, path_to_local, path_to_local_id, paths};
+use clippy_utils::{get_vec_init_kind, path_to_local, path_to_local_id, VecInitKind};
 use if_chain::if_chain;
-use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
-use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, HirId, Local, PatKind, QPath, Stmt, StmtKind};
+use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, HirId, Local, PatKind, 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::{symbol::sym, Span};
-use std::convert::TryInto;
+use rustc_span::Span;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -41,11 +38,6 @@ pub struct VecInitThenPush {
     searcher: Option<VecPushSearcher>,
 }
 
-#[derive(Clone, Copy)]
-enum VecInitKind {
-    New,
-    WithCapacity(u64),
-}
 struct VecPushSearcher {
     local_id: HirId,
     init: VecInitKind,
@@ -58,7 +50,8 @@ impl VecPushSearcher {
     fn display_err(&self, cx: &LateContext<'_>) {
         match self.init {
             _ if self.found == 0 => return,
-            VecInitKind::WithCapacity(x) if x > self.found => return,
+            VecInitKind::WithLiteralCapacity(x) if x > self.found => return,
+            VecInitKind::WithExprCapacity(_) => return,
             _ => (),
         };
 
@@ -152,37 +145,3 @@ impl LateLintPass<'_> for VecInitThenPush {
         }
     }
 }
-
-fn get_vec_init_kind<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<VecInitKind> {
-    if let ExprKind::Call(func, args) = expr.kind {
-        match func.kind {
-            ExprKind::Path(QPath::TypeRelative(ty, name))
-                if is_type_diagnostic_item(cx, cx.typeck_results().node_type(ty.hir_id), sym::Vec) =>
-            {
-                if name.ident.name == sym::new {
-                    return Some(VecInitKind::New);
-                } else if name.ident.name.as_str() == "with_capacity" {
-                    return args.get(0).and_then(|arg| {
-                        if_chain! {
-                            if let ExprKind::Lit(lit) = &arg.kind;
-                            if let LitKind::Int(num, _) = lit.node;
-                            then {
-                                Some(VecInitKind::WithCapacity(num.try_into().ok()?))
-                            } else {
-                                None
-                            }
-                        }
-                    });
-                }
-            }
-            ExprKind::Path(QPath::Resolved(_, path))
-                if match_def_path(cx, path.res.opt_def_id()?, &paths::DEFAULT_TRAIT_METHOD)
-                    && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::Vec) =>
-            {
-                return Some(VecInitKind::New);
-            }
-            _ => (),
-        }
-    }
-    None
-}

clippy_utils/src/lib.rs

@@ -93,7 +93,7 @@ use rustc_span::{Span, DUMMY_SP};
 use rustc_target::abi::Integer;
 
 use crate::consts::{constant, Constant};
-use crate::ty::{can_partially_move_ty, is_copy, is_recursively_primitive_type};
+use crate::ty::{can_partially_move_ty, is_copy, is_recursively_primitive_type, is_type_diagnostic_item};
 
 pub fn parse_msrv(msrv: &str, sess: Option<&Session>, span: Option<Span>) -> Option<RustcVersion> {
     if let Ok(version) = RustcVersion::parse(msrv) {
@@ -1789,6 +1789,53 @@ pub fn is_expr_identity_function(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool
     }
 }
 
+#[derive(Clone, Copy)]
+pub enum VecInitKind {
+    /// `Vec::new()`
+    New,
+    /// `Vec::default()` or `Default::default()`
+    Default,
+    /// `Vec::with_capacity(123)`
+    WithLiteralCapacity(u64),
+    /// `Vec::with_capacity(slice.len())`
+    WithExprCapacity(HirId),
+}
+
+/// Checks if given expression is an initialization of `Vec` and returns its kind.
+pub fn get_vec_init_kind<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<VecInitKind> {
+    if let ExprKind::Call(func, args) = expr.kind {
+        match func.kind {
+            ExprKind::Path(QPath::TypeRelative(ty, name))
+                if is_type_diagnostic_item(cx, cx.typeck_results().node_type(ty.hir_id), sym::Vec) =>
+            {
+                if name.ident.name == sym::new {
+                    return Some(VecInitKind::New);
+                } else if name.ident.name.as_str() == "with_capacity" {
+                    return args.get(0).and_then(|arg| {
+                        if_chain! {
+                            if let ExprKind::Lit(lit) = &arg.kind;
+                            if let LitKind::Int(num, _) = lit.node;
+                            then {
+                                Some(VecInitKind::WithLiteralCapacity(num.try_into().ok()?))
+                            } else {
+                                Some(VecInitKind::WithExprCapacity(arg.hir_id))
+                            }
+                        }
+                    });
+                }
+            }
+            ExprKind::Path(QPath::Resolved(_, path))
+                if match_def_path(cx, path.res.opt_def_id()?, &paths::DEFAULT_TRAIT_METHOD)
+                    && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::Vec) =>
+            {
+                return Some(VecInitKind::Default);
+            }
+            _ => (),
+        }
+    }
+    None
+}
+
 /// Gets the node where an expression is either used, or it's type is unified with another branch.
 pub fn get_expr_use_or_unification_node(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> Option<Node<'tcx>> {
     let mut child_id = expr.hir_id;

clippy_utils/src/paths.rs

@@ -183,7 +183,6 @@ pub const VEC_AS_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_slice"];
 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_WITH_CAPACITY: [&str; 4] = ["alloc", "vec", "Vec", "with_capacity"];
 pub const VEC_SET_LEN: [&str; 4] = ["alloc", "vec", "Vec", "set_len"];
 pub const WEAK_ARC: [&str; 3] = ["alloc", "sync", "Weak"];
 pub const WEAK_RC: [&str; 3] = ["alloc", "rc", "Weak"];