add arc_new_in_vec_from_slice lint

Author: yonip23

CHANGELOG.md

@@ -3278,6 +3278,7 @@ Released 2018-09-13
 [`allow_attributes_without_reason`]: https://rust-lang.github.io/rust-clippy/master/index.html#allow_attributes_without_reason
 [`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped
 [`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant
+[`arc_new_in_vec_from_elem`]: https://rust-lang.github.io/rust-clippy/master/index.html#arc_new_in_vec_from_elem
 [`as_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_conversions
 [`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants
 [`assign_op_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern

clippy_lints/src/arc_new_in_vec_from_elem.rs

@@ -0,0 +1,79 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::higher::VecArgs;
+use clippy_utils::last_path_segment;
+use clippy_utils::macros::{root_macro_call_first_node, MacroCall};
+use rustc_hir::{Expr, ExprKind, QPath, TyKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `Arc::new` in `vec![elem; len]`
+    ///
+    /// ### Why is this bad?
+    /// This will create `elem` once and clone it `len` times - doing so with `Arc`
+    /// is a bit misleading, as it will create references to the same pointer, rather
+    /// than different `Arc` instances.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let v = vec![std::sync::Arc::new("some data".to_string()); 100];
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let data = std::sync::Arc::new("some data".to_string());
+    /// let v = vec![data; 100];
+    /// ```
+    #[clippy::version = "1.62.0"]
+    pub ARC_NEW_IN_VEC_FROM_ELEM,
+    suspicious,
+    "calling `Arc::new` in `vec![elem; len]`"
+}
+declare_lint_pass!(ArcNewInVecFromElem => [ARC_NEW_IN_VEC_FROM_ELEM]);
+
+impl LateLintPass<'_> for ArcNewInVecFromElem {
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
+        let Some(macro_call) = root_macro_call_first_node(cx, expr) else { return; };
+
+        if let Some(VecArgs::Repeat(elem, _)) = VecArgs::hir(cx, expr) {
+            if !is_arc_new(cx, elem) {
+                return;
+            }
+
+            yield_lint(cx, &macro_call);
+        }
+    }
+}
+
+fn yield_lint(cx: &LateContext<'_>, macro_call: &MacroCall) {
+    span_lint_and_then(
+        cx,
+        ARC_NEW_IN_VEC_FROM_ELEM,
+        macro_call.span,
+        "calling `Arc::new` in `vec![elem; len]`",
+        |diag| {
+            diag.note("each `Arc` will point to the same allocation");
+            diag.help("if this is intentional, consider extracting the `Arc` initialization to a variable");
+        },
+    );
+}
+
+/// Checks whether the given `expr` is a call to `Arc::new`
+fn is_arc_new(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    if_chain! {
+        if let ExprKind::Call(func, _args) = expr.kind;
+        if let ExprKind::Path(ref func_path @ QPath::TypeRelative(ty, _)) = func.kind;
+        if let TyKind::Path(ref ty_path) = ty.kind;
+        if let Some(def_id) = cx.qpath_res(ty_path, ty.hir_id).opt_def_id();
+        if cx.tcx.is_diagnostic_item(sym::Arc, def_id);
+
+        then {
+            let func_segment = last_path_segment(func_path);
+
+            return func_segment.ident.name == sym::new;
+        }
+    }
+
+    false
+}

clippy_lints/src/lib.register_all.rs

@@ -5,6 +5,7 @@
 store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(absurd_extreme_comparisons::ABSURD_EXTREME_COMPARISONS),
     LintId::of(approx_const::APPROX_CONSTANT),
+    LintId::of(arc_new_in_vec_from_elem::ARC_NEW_IN_VEC_FROM_ELEM),
     LintId::of(assertions_on_constants::ASSERTIONS_ON_CONSTANTS),
     LintId::of(assign_ops::ASSIGN_OP_PATTERN),
     LintId::of(assign_ops::MISREFACTORED_ASSIGN_OP),

clippy_lints/src/lib.register_lints.rs

@@ -35,6 +35,7 @@ store.register_lints(&[
     utils::internal_lints::UNNECESSARY_SYMBOL_STR,
     absurd_extreme_comparisons::ABSURD_EXTREME_COMPARISONS,
     approx_const::APPROX_CONSTANT,
+    arc_new_in_vec_from_elem::ARC_NEW_IN_VEC_FROM_ELEM,
     arithmetic::FLOAT_ARITHMETIC,
     arithmetic::INTEGER_ARITHMETIC,
     as_conversions::AS_CONVERSIONS,

clippy_lints/src/lib.register_suspicious.rs

@@ -3,6 +3,7 @@
 // Manual edits will be overwritten.
 
 store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec![
+    LintId::of(arc_new_in_vec_from_elem::ARC_NEW_IN_VEC_FROM_ELEM),
     LintId::of(assign_ops::MISREFACTORED_ASSIGN_OP),
     LintId::of(attrs::BLANKET_CLIPPY_RESTRICTION_LINTS),
     LintId::of(await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE),

clippy_lints/src/lib.rs

@@ -166,6 +166,7 @@ mod utils;
 // begin lints modules, do not remove this comment, it’s used in `update_lints`
 mod absurd_extreme_comparisons;
 mod approx_const;
+mod arc_new_in_vec_from_elem;
 mod arithmetic;
 mod as_conversions;
 mod asm_syntax;
@@ -887,6 +888,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(bytes_count_to_len::BytesCountToLen));
     let max_include_file_size = conf.max_include_file_size;
     store.register_late_pass(move || Box::new(large_include_file::LargeIncludeFile::new(max_include_file_size)));
+    store.register_late_pass(|| Box::new(arc_new_in_vec_from_elem::ArcNewInVecFromElem));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

tests/ui/arc_new_in_vec_from_elem/complex_case.rs

@@ -0,0 +1,13 @@
+#![warn(clippy::arc_new_in_vec_from_elem)]
+use std::sync::Mutex;
+
+fn main() {
+    let v = vec![
+        std::sync::Arc::new(Mutex::new({
+            let x = 1;
+            dbg!(x);
+            x
+        }));
+        2
+    ];
+}

tests/ui/arc_new_in_vec_from_elem/complex_case.stderr

@@ -0,0 +1,19 @@
+error: calling `Arc::new` in `vec![elem; len]`
+  --> $DIR/complex_case.rs:5:13
+   |
+LL |       let v = vec![
+   |  _____________^
+LL | |         std::sync::Arc::new(Mutex::new({
+LL | |             let x = 1;
+LL | |             dbg!(x);
+...  |
+LL | |         2
+LL | |     ];
+   | |_____^
+   |
+   = note: `-D clippy::arc-new-in-vec-from-elem` implied by `-D warnings`
+   = note: each `Arc` will point to the same allocation
+   = help: if this is intentional, consider extracting the `Arc` initialization to a variable
+
+error: aborting due to previous error
+

tests/ui/arc_new_in_vec_from_elem/custom_arc_strucrt.rs

@@ -0,0 +1,14 @@
+#![warn(clippy::arc_new_in_vec_from_elem)]
+
+#[derive(Clone)]
+struct Arc;
+
+impl Arc {
+    fn new() -> Self {
+        Arc
+    }
+}
+
+fn main() {
+    let v = vec![Arc::new(); 2];
+}

tests/ui/arc_new_in_vec_from_elem/simple_case.rs

@@ -0,0 +1,6 @@
+#![warn(clippy::arc_new_in_vec_from_elem)]
+use std::sync::Arc;
+
+fn main() {
+    let v = vec![Arc::new("x".to_string()); 2];
+}

tests/ui/arc_new_in_vec_from_elem/simple_case.stderr

@@ -0,0 +1,12 @@
+error: calling `Arc::new` in `vec![elem; len]`
+  --> $DIR/simple_case.rs:5:13
+   |
+LL |     let v = vec![Arc::new("x".to_string()); 2];
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::arc-new-in-vec-from-elem` implied by `-D warnings`
+   = note: each `Arc` will point to the same allocation
+   = help: if this is intentional, consider extracting the `Arc` initialization to a variable
+
+error: aborting due to previous error
+

tests/ui/arc_new_in_vec_from_elem/vec_from_elem_but_not_arc.rs

@@ -0,0 +1,15 @@
+#![warn(clippy::arc_new_in_vec_from_elem)]
+use std::sync::{Arc, Mutex};
+
+fn main() {
+    let v = vec![String::new(); 2];
+    let v1 = vec![1; 2];
+    let v2 = vec![
+        Box::new(std::sync::Arc::new({
+            let y = 3;
+            dbg!(y);
+            y
+        }));
+        2
+    ];
+}

tests/ui/arc_new_in_vec_from_elem/vec_macro_but_not_from_elem.rs

@@ -0,0 +1,6 @@
+#![warn(clippy::arc_new_in_vec_from_elem)]
+use std::sync::Arc;
+
+fn main() {
+    let v = vec![Arc::new("x".to_string())];
+}