diff --git a/clippy_lints/src/rc_clone_in_vec_init.rs b/clippy_lints/src/rc_clone_in_vec_init.rs index 6391890b043..aa575d7e68b 100644 --- a/clippy_lints/src/rc_clone_in_vec_init.rs +++ b/clippy_lints/src/rc_clone_in_vec_init.rs @@ -1,11 +1,13 @@ 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 clippy_utils::macros::root_macro_call_first_node; +use clippy_utils::source::{indent_of, snippet}; +use rustc_errors::{Applicability, Diagnostic}; 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, Symbol}; +use rustc_span::{sym, Span, Symbol}; declare_clippy_lint! { /// ### What it does @@ -47,27 +49,107 @@ impl LateLintPass<'_> for RcCloneInVecInit { fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { let Some(macro_call) = root_macro_call_first_node(cx, expr) else { return; }; - let Some(VecArgs::Repeat(elem, _)) = VecArgs::hir(cx, expr) else { return; }; + let Some(VecArgs::Repeat(elem, len)) = VecArgs::hir(cx, expr) else { return; }; let Some(symbol) = new_reference_call(cx, elem) else { return; }; + let lint_span = macro_call.span; + let symbol_name = symbol.as_str(); + let len_snippet = snippet(cx, len.span, ".."); + let elem_snippet = elem_snippet(cx, elem, symbol_name); + let indentation = indent_of(cx, lint_span).unwrap_or(0); + let lint_suggestions = + construct_lint_suggestions(lint_span, symbol_name, &elem_snippet, len_snippet.as_ref(), indentation); - emit_lint(cx, symbol, ¯o_call); + emit_lint(cx, symbol, lint_span, &lint_suggestions); } } -fn emit_lint(cx: &LateContext<'_>, symbol: Symbol, macro_call: &MacroCall) { +struct LintSuggestion { + span: Span, + message: String, + suggestion: String, + applicability: Applicability, +} + +impl LintSuggestion { + fn span_suggestion(&self, diag: &mut Diagnostic) { + diag.span_suggestion(self.span, &self.message, &self.suggestion, self.applicability); + } +} + +fn construct_lint_suggestions( + span: Span, + symbol_name: &str, + elem_snippet: &str, + len_snippet: &str, + indentation: usize, +) -> Vec { + let indentation = " ".repeat(indentation); + let loop_init_suggestion = loop_init_suggestion(elem_snippet, len_snippet, &indentation); + let extract_suggestion = extract_suggestion(elem_snippet, len_snippet, &indentation); + + vec![ + LintSuggestion { + span, + message: format!("consider initializing each `{symbol_name}` element individually"), + suggestion: loop_init_suggestion, + applicability: Applicability::Unspecified, + }, + LintSuggestion { + span, + message: format!( + "or if this is intentional, consider extracting the `{symbol_name}` initialization to a variable" + ), + suggestion: extract_suggestion, + applicability: Applicability::Unspecified, + }, + ] +} + +fn elem_snippet(cx: &LateContext<'_>, elem: &Expr<'_>, symbol_name: &str) -> String { + let mut elem_snippet = snippet(cx, elem.span, "..").to_string(); + if elem_snippet.contains('\n') { + let reference_initialization = format!("{symbol_name}::new"); + // This string must be found in `elem_snippet`, otherwise we won't be constructing the snippet in + // the first place. + let reference_initialization_end = + elem_snippet.find(&reference_initialization).unwrap() + reference_initialization.len(); + elem_snippet.replace_range(reference_initialization_end.., ".."); + } + elem_snippet +} + +fn loop_init_suggestion(elem: &str, len: &str, indent: &str) -> String { + format!( + r#"{{ +{indent}{indent}let mut v = Vec::with_capacity({len}); +{indent}{indent}(0..{len}).for_each(|_| v.push({elem})); +{indent}{indent}v +{indent}}}"# + ) +} + +fn extract_suggestion(elem: &str, len: &str, indent: &str) -> String { + format!( + "{{ +{indent}{indent}let data = {elem}; +{indent}{indent}vec![data; {len}] +{indent}}}" + ) +} + +fn emit_lint(cx: &LateContext<'_>, symbol: Symbol, lint_span: Span, lint_suggestions: &[LintSuggestion]) { let symbol_name = symbol.as_str(); span_lint_and_then( cx, RC_CLONE_IN_VEC_INIT, - macro_call.span, + lint_span, &format!("calling `{symbol_name}::new` in `vec![elem; len]`"), |diag| { diag.note(format!("each element will point to the same `{symbol_name}` instance")); - diag.help(format!( - "if this is intentional, consider extracting the `{symbol_name}` initialization to a variable" - )); - diag.help("or if not, initialize each element individually"); + lint_suggestions + .iter() + .for_each(|suggestion| suggestion.span_suggestion(diag)); }, ); } diff --git a/tests/ui/rc_clone_in_vec_init/arc.stderr b/tests/ui/rc_clone_in_vec_init/arc.stderr index 19e4727cb98..3de96c6f175 100644 --- a/tests/ui/rc_clone_in_vec_init/arc.stderr +++ b/tests/ui/rc_clone_in_vec_init/arc.stderr @@ -6,8 +6,21 @@ LL | let v = vec![Arc::new("x".to_string()); 2]; | = note: `-D clippy::rc-clone-in-vec-init` implied by `-D warnings` = note: each element will point to the same `Arc` instance - = help: if this is intentional, consider extracting the `Arc` initialization to a variable - = help: or if not, initialize each element individually +help: consider initializing each `Arc` element individually + | +LL ~ let v = { +LL + let mut v = Vec::with_capacity(2); +LL + (0..2).for_each(|_| v.push(Arc::new("x".to_string()))); +LL + v +LL ~ }; + | +help: or if this is intentional, consider extracting the `Arc` initialization to a variable + | +LL ~ let v = { +LL + let data = Arc::new("x".to_string()); +LL + vec![data; 2] +LL ~ }; + | error: calling `Arc::new` in `vec![elem; len]` --> $DIR/arc.rs:11:13 @@ -23,8 +36,21 @@ LL | | ]; | |_____^ | = note: each element will point to the same `Arc` instance - = help: if this is intentional, consider extracting the `Arc` initialization to a variable - = help: or if not, initialize each element individually +help: consider initializing each `Arc` element individually + | +LL ~ let v = { +LL + let mut v = Vec::with_capacity(2); +LL + (0..2).for_each(|_| v.push(std::sync::Arc::new..)); +LL + v +LL ~ }; + | +help: or if this is intentional, consider extracting the `Arc` initialization to a variable + | +LL ~ let v = { +LL + let data = std::sync::Arc::new..; +LL + vec![data; 2] +LL ~ }; + | error: aborting due to 2 previous errors diff --git a/tests/ui/rc_clone_in_vec_init/rc.stderr b/tests/ui/rc_clone_in_vec_init/rc.stderr index 50ffcca9676..e05f024cf9d 100644 --- a/tests/ui/rc_clone_in_vec_init/rc.stderr +++ b/tests/ui/rc_clone_in_vec_init/rc.stderr @@ -6,8 +6,21 @@ LL | let v = vec![Rc::new("x".to_string()); 2]; | = note: `-D clippy::rc-clone-in-vec-init` implied by `-D warnings` = note: each element will point to the same `Rc` instance - = help: if this is intentional, consider extracting the `Rc` initialization to a variable - = help: or if not, initialize each element individually +help: consider initializing each `Rc` element individually + | +LL ~ let v = { +LL + let mut v = Vec::with_capacity(2); +LL + (0..2).for_each(|_| v.push(Rc::new("x".to_string()))); +LL + v +LL ~ }; + | +help: or if this is intentional, consider extracting the `Rc` initialization to a variable + | +LL ~ let v = { +LL + let data = Rc::new("x".to_string()); +LL + vec![data; 2] +LL ~ }; + | error: calling `Rc::new` in `vec![elem; len]` --> $DIR/rc.rs:12:13 @@ -23,8 +36,21 @@ LL | | ]; | |_____^ | = note: each element will point to the same `Rc` instance - = help: if this is intentional, consider extracting the `Rc` initialization to a variable - = help: or if not, initialize each element individually +help: consider initializing each `Rc` element individually + | +LL ~ let v = { +LL + let mut v = Vec::with_capacity(2); +LL + (0..2).for_each(|_| v.push(std::rc::Rc::new..)); +LL + v +LL ~ }; + | +help: or if this is intentional, consider extracting the `Rc` initialization to a variable + | +LL ~ let v = { +LL + let data = std::rc::Rc::new..; +LL + vec![data; 2] +LL ~ }; + | error: aborting due to 2 previous errors