Add lint

rust-lang · m-ou-se · Dec 15, 2020 · Dec 28, 2020 · Dec 28, 2020 · Dec 28, 2020
commit 1853f8b228298d5b91e3d03ee677da91574554b4
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2152,6 +2152,7 @@ Released 2018-09-13
 [`useless_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_transmute
 [`useless_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_vec
 [`vec_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_box
+[`vec_init_then_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_init_then_push
 [`vec_resize_to_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_resize_to_zero
 [`verbose_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_bit_mask
 [`verbose_file_reads`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_file_reads

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -341,6 +341,7 @@ mod unwrap_in_result;
 mod use_self;
 mod useless_conversion;
 mod vec;
+mod vec_init_then_push;
 mod vec_resize_to_zero;
 mod verbose_file_reads;
 mod wildcard_dependencies;
@@ -935,6 +936,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &use_self::USE_SELF,
         &useless_conversion::USELESS_CONVERSION,
         &vec::USELESS_VEC,
+        &vec_init_then_push::VEC_INIT_THEN_PUSH,
         &vec_resize_to_zero::VEC_RESIZE_TO_ZERO,
         &verbose_file_reads::VERBOSE_FILE_READS,
         &wildcard_dependencies::WILDCARD_DEPENDENCIES,
@@ -1215,6 +1217,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box strings::StrToString);
     store.register_late_pass(|| box strings::StringToString);
     store.register_late_pass(|| box zero_sized_map_values::ZeroSizedMapValues);
+    store.register_late_pass(|| box vec_init_then_push::VecInitThenPush::default());
 
     store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
         LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1636,6 +1639,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&unwrap::UNNECESSARY_UNWRAP),
         LintId::of(&useless_conversion::USELESS_CONVERSION),
         LintId::of(&vec::USELESS_VEC),
+        LintId::of(&vec_init_then_push::VEC_INIT_THEN_PUSH),
         LintId::of(&vec_resize_to_zero::VEC_RESIZE_TO_ZERO),
         LintId::of(&write::PRINTLN_EMPTY_STRING),
         LintId::of(&write::PRINT_LITERAL),
@@ -1935,6 +1939,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&types::BOX_VEC),
         LintId::of(&types::REDUNDANT_ALLOCATION),
         LintId::of(&vec::USELESS_VEC),
+        LintId::of(&vec_init_then_push::VEC_INIT_THEN_PUSH),
     ]);
 
     store.register_group(true, "clippy::cargo", Some("clippy_cargo"), vec![

diff --git a/clippy_lints/src/vec_init_then_push.rs b/clippy_lints/src/vec_init_then_push.rs
@@ -0,0 +1,186 @@
+use crate::utils::{is_type_diagnostic_item, match_def_path, paths, snippet, span_lint_and_sugg};
+use if_chain::if_chain;
+use rustc_ast::ast::LitKind;
+use rustc_errors::Applicability;
+use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, Local, PatKind, QPath, 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, Symbol};
+
+declare_clippy_lint! {
+    /// **What it does:** Checks for calls to `push` immediately after creating a new `Vec`.
+    ///
+    /// **Why is this bad?** The `vec![]` macro is both more performant and easier to read than
+    /// multiple `push` calls.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// let mut v: Vec<u32> = Vec::new();
+    /// v.push(0);
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let v: Vec<u32> = vec![0];
+    /// ```
+    pub VEC_INIT_THEN_PUSH,
+    perf,
+    "`push` immediately after `Vec` creation"
+}
+
+impl_lint_pass!(VecInitThenPush => [VEC_INIT_THEN_PUSH]);
+
+#[derive(Default)]
+pub struct VecInitThenPush {
+    searcher: Option<VecPushSearcher>,
+}
+
+#[derive(Clone, Copy)]
+enum VecInitKind {
+    New,
+    WithCapacity(u64),
+}
+struct VecPushSearcher {
+    init: VecInitKind,
+    name: Symbol,
+    lhs_is_local: bool,
+    lhs_span: Span,
+    err_span: Span,
+    found: u64,
+}
+impl VecPushSearcher {
+    fn display_err(&self, cx: &LateContext<'_>) {
+        match self.init {
+            _ if self.found == 0 => return,
+            VecInitKind::WithCapacity(x) if x > self.found => return,
+            _ => (),
+        };
+
+        let mut s = if self.lhs_is_local {
+            String::from("let ")
+        } else {
+            String::new()
+        };
+        s.push_str(&snippet(cx, self.lhs_span, ".."));
+        s.push_str(" = vec![..];");
+
+        span_lint_and_sugg(
+            cx,
+            VEC_INIT_THEN_PUSH,
+            self.err_span,
+            "calls to `push` immediately after creation",
+            "consider using the `vec![]` macro",
+            s,
+            Applicability::HasPlaceholders,
+        );
+    }
+}
+
+impl LateLintPass<'_> for VecInitThenPush {
+    fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) {
+        self.searcher = None;
+        if_chain! {
+            if !in_external_macro(cx.sess(), local.span);
+            if let Some(init) = local.init;
+            if let PatKind::Binding(BindingAnnotation::Mutable, _, ident, None) = local.pat.kind;
+            if let Some(init_kind) = get_vec_init_kind(cx, init);
+            then {
+                self.searcher = Some(VecPushSearcher {
+                        init: init_kind,
+                        name: ident.name,
+                        lhs_is_local: true,
+                        lhs_span: local.ty.map(|t| local.pat.span.to(t.span)).unwrap_or(local.pat.span),
+                        err_span: local.span,
+                        found: 0,
+                    });
+            }
+        }
+    }
+
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        if self.searcher.is_none() {
+            if_chain! {
+                if !in_external_macro(cx.sess(), expr.span);
+                if let ExprKind::Assign(left, right, _) = expr.kind;
+                if let ExprKind::Path(QPath::Resolved(_, path)) = left.kind;
+                if let Some(name) = path.segments.get(0);
+                if let Some(init_kind) = get_vec_init_kind(cx, right);
+                then {
+                    self.searcher = Some(VecPushSearcher {
+                        init: init_kind,
+                        name: name.ident.name,
+                        lhs_is_local: false,
+                        lhs_span: left.span,
+                        err_span: expr.span,
+                        found: 0,
+                    });
+                }
+            }
+        }
+    }
+
+    fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
+        if let Some(searcher) = self.searcher.take() {
+            if_chain! {
+                if let StmtKind::Expr(expr) | StmtKind::Semi(expr) = stmt.kind;
+                if let ExprKind::MethodCall(path, _, [self_arg, _], _) = expr.kind;
+                if path.ident.name.as_str() == "push";
+                if let ExprKind::Path(QPath::Resolved(_, self_path)) = self_arg.kind;
+                if let [self_name] = self_path.segments;
+                if self_name.ident.name == searcher.name;
+                then {
+                    self.searcher = Some(VecPushSearcher {
+                        found: searcher.found + 1,
+                        err_span: searcher.err_span.to(stmt.span),
+                        .. searcher
+                    });
+                } else {
+                    searcher.display_err(cx);
+                }
+            }
+        }
+    }
+
+    fn check_block_post(&mut self, cx: &LateContext<'tcx>, _: &'tcx Block<'tcx>) {
+        if let Some(searcher) = self.searcher.take() {
+            searcher.display_err(cx);
+        }
+    }
+}
+
+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_type) =>
+            {
+                if name.ident.name.as_str() == "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 as u64))
+                            } 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_type) =>
+            {
+                return Some(VecInitKind::New);
+            }
+            _ => (),
+        }
+    }
+    None
+}
diff --git a/tests/ui/vec_init_then_push.rs b/tests/ui/vec_init_then_push.rs
@@ -0,0 +1,21 @@
+#![allow(unused_variables)]
+#![warn(clippy::vec_init_then_push)]
+
+fn main() {
+    let mut def_err: Vec<u32> = Default::default();
+    def_err.push(0);
+
+    let mut new_err = Vec::<u32>::new();
+    new_err.push(1);
+
+    let mut cap_err = Vec::with_capacity(2);
+    cap_err.push(0);
+    cap_err.push(1);
+    cap_err.push(2);
+
+    let mut cap_ok = Vec::with_capacity(10);
+    cap_ok.push(0);
+
+    new_err = Vec::new();
+    new_err.push(0);
+}
diff --git a/tests/ui/vec_init_then_push.stderr b/tests/ui/vec_init_then_push.stderr
@@ -0,0 +1,34 @@
+error: calls to `push` immediately after creation
+  --> $DIR/vec_init_then_push.rs:5:5
+   |
+LL | /     let mut def_err: Vec<u32> = Default::default();
+LL | |     def_err.push(0);
+   | |____________________^ help: consider using the `vec![]` macro: `let mut def_err: Vec<u32> = vec![..];`
+   |
+   = note: `-D clippy::vec-init-then-push` implied by `-D warnings`
+
+error: calls to `push` immediately after creation
+  --> $DIR/vec_init_then_push.rs:8:5
+   |
+LL | /     let mut new_err = Vec::<u32>::new();
+LL | |     new_err.push(1);
+   | |____________________^ help: consider using the `vec![]` macro: `let mut new_err = vec![..];`
+
+error: calls to `push` immediately after creation
+  --> $DIR/vec_init_then_push.rs:11:5
+   |
+LL | /     let mut cap_err = Vec::with_capacity(2);
+LL | |     cap_err.push(0);
+LL | |     cap_err.push(1);
+LL | |     cap_err.push(2);
+   | |____________________^ help: consider using the `vec![]` macro: `let mut cap_err = vec![..];`
+
+error: calls to `push` immediately after creation
+  --> $DIR/vec_init_then_push.rs:19:5
+   |
+LL | /     new_err = Vec::new();
+LL | |     new_err.push(0);
+   | |____________________^ help: consider using the `vec![]` macro: `new_err = vec![..];`
+
+error: aborting due to 4 previous errors
+