Skip to content

Commit cbebf82

Browse files
committed
fix pr comments
1 parent eae4137 commit cbebf82

File tree

3 files changed

+32
-24
lines changed

3 files changed

+32
-24
lines changed

clippy_lints/src/arc_new_in_vec_from_slice.rs

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,20 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
2+
use clippy_utils::higher::VecArgs;
3+
use clippy_utils::last_path_segment;
24
use clippy_utils::macros::{root_macro_call_first_node, MacroCall};
3-
use clippy_utils::{last_path_segment, match_def_path, paths};
45
use rustc_hir::{Expr, ExprKind, QPath, TyKind};
56
use rustc_lint::{LateContext, LateLintPass};
67
use rustc_session::{declare_lint_pass, declare_tool_lint};
78
use rustc_span::sym;
89

910
declare_clippy_lint! {
1011
/// ### What it does
11-
/// Checks for `Arc::new` calls in vector initialization using the slice macro `vec![elem; len]`
12+
/// Checks for `Arc::new` in `vec![elem; len]`
1213
///
1314
/// ### Why is this bad?
14-
/// This vector initialization creates `elem` once and clones it `len` times - doing so with `Arc`
15-
/// is a bit missleading, because the vector will contain references to the same pointer, although
16-
/// it can look like it'll contain different `Arc` instances.
15+
/// This will create `elem` once and clone it `len` times - doing so with `Arc`
16+
/// is a bit misleading, as it will create references to the same pointer, rather
17+
/// than different `Arc` instances.
1718
///
1819
/// ### Example
1920
/// ```rust
@@ -38,41 +39,42 @@ impl LateLintPass<'_> for ArcNewInVecFromSlice {
3839
return;
3940
}
4041

41-
if_chain! {
42-
if let ExprKind::Call(func, args) = expr.peel_drop_temps().kind;
43-
if let ExprKind::Path(QPath::Resolved(_ty, path)) = func.kind;
44-
if let Some(did) = path.res.opt_def_id();
45-
then {
46-
if !(match_def_path(cx, did, &paths::VEC_FROM_ELEM) && first_arg_is_arc_new(args)) { return; }
47-
48-
span_lint_and_help(
49-
cx,
50-
ARC_NEW_IN_VEC_FROM_SLICE,
51-
macro_call.span,
52-
"calling `Arc::new` in `vec![elem; len]`",
53-
None,
54-
"consider extracting `Arc` initialization to a variable",
55-
);
42+
if let Some(VecArgs::Repeat(elem, _)) = VecArgs::hir(cx, expr) {
43+
if !is_arc_new(elem) {
44+
return;
5645
}
46+
47+
yield_lint(cx, &macro_call);
5748
}
5849
}
5950
}
6051

52+
fn yield_lint(cx: &LateContext<'_>, macro_call: &MacroCall) {
53+
span_lint_and_help(
54+
cx,
55+
ARC_NEW_IN_VEC_FROM_SLICE,
56+
macro_call.span,
57+
"calling `Arc::new` in `vec![elem; len]`",
58+
None,
59+
"consider extracting `Arc` initialization to a variable",
60+
);
61+
}
62+
6163
fn macro_is_vec(cx: &LateContext<'_>, macro_call: &MacroCall) -> bool {
6264
cx.tcx.is_diagnostic_item(sym::vec_macro, macro_call.def_id)
6365
}
6466

65-
fn first_arg_is_arc_new(args: &[Expr<'_>]) -> bool {
66-
let Some(first_arg) = args.get(0) else { return false; };
67+
/// Checks whether the given `expr` is a call to `Arc::new`
68+
fn is_arc_new(expr: &Expr<'_>) -> bool {
6769
if_chain! {
68-
if let ExprKind::Call(func, _args) = first_arg.kind;
70+
if let ExprKind::Call(func, _args) = expr.kind;
6971
if let ExprKind::Path(ref func_path) = func.kind;
7072
if let ExprKind::Path(QPath::TypeRelative(ty, _)) = func.kind;
7173
if let TyKind::Path(ref ty_path) = ty.kind;
7274

7375
then {
74-
let func_segment = last_path_segment(func_path);
7576
let ty_segment = last_path_segment(ty_path);
77+
let func_segment = last_path_segment(func_path);
7678

7779
return ty_segment.ident.name == sym::Arc && func_segment.ident.name == sym::new;
7880
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#![warn(clippy::arc_new_in_vec_from_slice)]
2+
use std::sync::Arc;
3+
4+
fn main() {
5+
let v = vec![Arc::new("x".to_string())];
6+
}

0 commit comments

Comments
 (0)