From f3e01c4f6a12ad1bd70813875ed84dbe8c830df7 Mon Sep 17 00:00:00 2001 From: yonip23 Date: Wed, 11 May 2022 00:46:20 +0300 Subject: [PATCH 1/5] add suggestions to rc_clone_in_vec_init --- clippy_lints/src/rc_clone_in_vec_init.rs | 102 ++++++++++++++++++++--- tests/ui/rc_clone_in_vec_init/arc.stderr | 34 +++++++- tests/ui/rc_clone_in_vec_init/rc.stderr | 34 +++++++- 3 files changed, 152 insertions(+), 18 deletions(-) diff --git a/clippy_lints/src/rc_clone_in_vec_init.rs b/clippy_lints/src/rc_clone_in_vec_init.rs index 6391890b0433..aa575d7e68bf 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 @@ declare_lint_pass!(RcCloneInVecInit => [RC_CLONE_IN_VEC_INIT]); 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 19e4727cb987..3de96c6f1758 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 50ffcca9676a..e05f024cf9de 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 From 344888a5820c2ae2d7c88ecf14bcaffb60ca2bc5 Mon Sep 17 00:00:00 2001 From: yonip23 Date: Wed, 11 May 2022 23:11:52 +0300 Subject: [PATCH 2/5] fix review comments --- clippy_lints/src/rc_clone_in_vec_init.rs | 72 ++++++++++-------------- tests/ui/rc_clone_in_vec_init/arc.rs | 9 +++ tests/ui/rc_clone_in_vec_init/arc.stderr | 36 +++++++++++- tests/ui/rc_clone_in_vec_init/rc.rs | 9 +++ tests/ui/rc_clone_in_vec_init/rc.stderr | 36 +++++++++++- 5 files changed, 115 insertions(+), 47 deletions(-) diff --git a/clippy_lints/src/rc_clone_in_vec_init.rs b/clippy_lints/src/rc_clone_in_vec_init.rs index aa575d7e68bf..7a7a3f558ca0 100644 --- a/clippy_lints/src/rc_clone_in_vec_init.rs +++ b/clippy_lints/src/rc_clone_in_vec_init.rs @@ -3,7 +3,7 @@ use clippy_utils::higher::VecArgs; use clippy_utils::last_path_segment; use clippy_utils::macros::root_macro_call_first_node; use clippy_utils::source::{indent_of, snippet}; -use rustc_errors::{Applicability, Diagnostic}; +use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, QPath, TyKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -51,70 +51,53 @@ impl LateLintPass<'_> for RcCloneInVecInit { let Some(macro_call) = root_macro_call_first_node(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, lint_span, &lint_suggestions); + + emit_lint(cx, symbol, macro_call.span, elem, len); } } 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); - } + snippet: String, } fn construct_lint_suggestions( + cx: &LateContext<'_>, span: Span, symbol_name: &str, - elem_snippet: &str, - len_snippet: &str, - indentation: usize, + elem: &Expr<'_>, + len: &Expr<'_>, ) -> Vec { + let len_snippet = snippet(cx, len.span, ".."); + let elem_snippet = elem_snippet(cx, elem, symbol_name); + let indentation = indent_of(cx, span).unwrap_or(0); 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); + let loop_init_suggestion = loop_init_suggestion(&elem_snippet, len_snippet.as_ref(), &indentation); + let extract_suggestion = extract_suggestion(&elem_snippet, len_snippet.as_ref(), &indentation); vec![ LintSuggestion { - span, message: format!("consider initializing each `{symbol_name}` element individually"), - suggestion: loop_init_suggestion, - applicability: Applicability::Unspecified, + snippet: loop_init_suggestion, }, LintSuggestion { - span, message: format!( "or if this is intentional, consider extracting the `{symbol_name}` initialization to a variable" ), - suggestion: extract_suggestion, - applicability: Applicability::Unspecified, + snippet: extract_suggestion, }, ] } fn elem_snippet(cx: &LateContext<'_>, elem: &Expr<'_>, symbol_name: &str) -> String { - let mut elem_snippet = snippet(cx, elem.span, "..").to_string(); + let 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.., ".."); + let reference_creation = format!("{symbol_name}::new"); + let (code_until_reference_creation, _right) = elem_snippet.split_once(&reference_creation).unwrap(); + + return format!("{code_until_reference_creation}{reference_creation}(..)"); } + elem_snippet } @@ -137,7 +120,7 @@ fn extract_suggestion(elem: &str, len: &str, indent: &str) -> String { ) } -fn emit_lint(cx: &LateContext<'_>, symbol: Symbol, lint_span: Span, lint_suggestions: &[LintSuggestion]) { +fn emit_lint(cx: &LateContext<'_>, symbol: Symbol, lint_span: Span, elem: &Expr<'_>, len: &Expr<'_>) { let symbol_name = symbol.as_str(); span_lint_and_then( @@ -146,10 +129,17 @@ fn emit_lint(cx: &LateContext<'_>, symbol: Symbol, lint_span: Span, lint_suggest lint_span, &format!("calling `{symbol_name}::new` in `vec![elem; len]`"), |diag| { + let suggestions = construct_lint_suggestions(cx, lint_span, symbol_name, elem, len); + diag.note(format!("each element will point to the same `{symbol_name}` instance")); - lint_suggestions - .iter() - .for_each(|suggestion| suggestion.span_suggestion(diag)); + suggestions.iter().for_each(|suggestion| { + diag.span_suggestion( + lint_span, + &suggestion.message, + &suggestion.snippet, + Applicability::Unspecified, + ); + }); }, ); } diff --git a/tests/ui/rc_clone_in_vec_init/arc.rs b/tests/ui/rc_clone_in_vec_init/arc.rs index 9f4e27dfa624..bef2c67a1a54 100644 --- a/tests/ui/rc_clone_in_vec_init/arc.rs +++ b/tests/ui/rc_clone_in_vec_init/arc.rs @@ -16,6 +16,15 @@ fn should_warn_complex_case() { })); 2 ]; + + let v1 = vec![ + Arc::new(Mutex::new({ + let x = 1; + dbg!(x); + x + })); + 2 + ]; } fn should_not_warn_custom_arc() { diff --git a/tests/ui/rc_clone_in_vec_init/arc.stderr b/tests/ui/rc_clone_in_vec_init/arc.stderr index 3de96c6f1758..387580c24310 100644 --- a/tests/ui/rc_clone_in_vec_init/arc.stderr +++ b/tests/ui/rc_clone_in_vec_init/arc.stderr @@ -40,17 +40,47 @@ 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 + (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 + let data = std::sync::Arc::new(..); LL + vec![data; 2] LL ~ }; | -error: aborting due to 2 previous errors +error: calling `Arc::new` in `vec![elem; len]` + --> $DIR/arc.rs:20:14 + | +LL | let v1 = vec![ + | ______________^ +LL | | Arc::new(Mutex::new({ +LL | | let x = 1; +LL | | dbg!(x); +... | +LL | | 2 +LL | | ]; + | |_____^ + | + = note: each element will point to the same `Arc` instance +help: consider initializing each `Arc` element individually + | +LL ~ let v1 = { +LL + let mut v = Vec::with_capacity(2); +LL + (0..2).for_each(|_| v.push(Arc::new(..))); +LL + v +LL ~ }; + | +help: or if this is intentional, consider extracting the `Arc` initialization to a variable + | +LL ~ let v1 = { +LL + let data = Arc::new(..); +LL + vec![data; 2] +LL ~ }; + | + +error: aborting due to 3 previous errors diff --git a/tests/ui/rc_clone_in_vec_init/rc.rs b/tests/ui/rc_clone_in_vec_init/rc.rs index 5e2834aa9023..79c23cafa2c1 100644 --- a/tests/ui/rc_clone_in_vec_init/rc.rs +++ b/tests/ui/rc_clone_in_vec_init/rc.rs @@ -17,6 +17,15 @@ fn should_warn_complex_case() { })); 2 ]; + + let v1 = vec![ + Rc::new(Mutex::new({ + let x = 1; + dbg!(x); + x + })); + 2 + ]; } fn should_not_warn_custom_arc() { diff --git a/tests/ui/rc_clone_in_vec_init/rc.stderr b/tests/ui/rc_clone_in_vec_init/rc.stderr index e05f024cf9de..4ce53eecbbd8 100644 --- a/tests/ui/rc_clone_in_vec_init/rc.stderr +++ b/tests/ui/rc_clone_in_vec_init/rc.stderr @@ -40,17 +40,47 @@ 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 + (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 + let data = std::rc::Rc::new(..); LL + vec![data; 2] LL ~ }; | -error: aborting due to 2 previous errors +error: calling `Rc::new` in `vec![elem; len]` + --> $DIR/rc.rs:21:14 + | +LL | let v1 = vec![ + | ______________^ +LL | | Rc::new(Mutex::new({ +LL | | let x = 1; +LL | | dbg!(x); +... | +LL | | 2 +LL | | ]; + | |_____^ + | + = note: each element will point to the same `Rc` instance +help: consider initializing each `Rc` element individually + | +LL ~ let v1 = { +LL + let mut v = Vec::with_capacity(2); +LL + (0..2).for_each(|_| v.push(Rc::new(..))); +LL + v +LL ~ }; + | +help: or if this is intentional, consider extracting the `Rc` initialization to a variable + | +LL ~ let v1 = { +LL + let data = Rc::new(..); +LL + vec![data; 2] +LL ~ }; + | + +error: aborting due to 3 previous errors From a791205e76fe9619f90d4869ab54e4213847b8fe Mon Sep 17 00:00:00 2001 From: yonip23 Date: Wed, 11 May 2022 23:16:49 +0300 Subject: [PATCH 3/5] fix clippy warning --- clippy_lints/src/rc_clone_in_vec_init.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/rc_clone_in_vec_init.rs b/clippy_lints/src/rc_clone_in_vec_init.rs index 7a7a3f558ca0..ee119e8c4a4f 100644 --- a/clippy_lints/src/rc_clone_in_vec_init.rs +++ b/clippy_lints/src/rc_clone_in_vec_init.rs @@ -92,6 +92,8 @@ fn construct_lint_suggestions( fn elem_snippet(cx: &LateContext<'_>, elem: &Expr<'_>, symbol_name: &str) -> String { let elem_snippet = snippet(cx, elem.span, "..").to_string(); if elem_snippet.contains('\n') { + // This string must be found in `elem_snippet`, otherwise we won't be constructing + // the snippet in the first place. let reference_creation = format!("{symbol_name}::new"); let (code_until_reference_creation, _right) = elem_snippet.split_once(&reference_creation).unwrap(); @@ -132,14 +134,14 @@ fn emit_lint(cx: &LateContext<'_>, symbol: Symbol, lint_span: Span, elem: &Expr< let suggestions = construct_lint_suggestions(cx, lint_span, symbol_name, elem, len); diag.note(format!("each element will point to the same `{symbol_name}` instance")); - suggestions.iter().for_each(|suggestion| { + for suggestion in suggestions { diag.span_suggestion( lint_span, &suggestion.message, &suggestion.snippet, Applicability::Unspecified, ); - }); + } }, ); } From dc23b5d661f828ddace3ccc65c5a53b0f43c1dd4 Mon Sep 17 00:00:00 2001 From: yonip23 Date: Thu, 12 May 2022 10:06:15 +0300 Subject: [PATCH 4/5] fix indentation + test --- clippy_lints/src/rc_clone_in_vec_init.rs | 10 ++++---- tests/ui/rc_clone_in_vec_init/arc.rs | 10 ++++++++ tests/ui/rc_clone_in_vec_init/arc.stderr | 29 +++++++++++++++++++++--- tests/ui/rc_clone_in_vec_init/rc.rs | 10 ++++++++ tests/ui/rc_clone_in_vec_init/rc.stderr | 29 +++++++++++++++++++++--- 5 files changed, 77 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/rc_clone_in_vec_init.rs b/clippy_lints/src/rc_clone_in_vec_init.rs index ee119e8c4a4f..ca12509bdc7f 100644 --- a/clippy_lints/src/rc_clone_in_vec_init.rs +++ b/clippy_lints/src/rc_clone_in_vec_init.rs @@ -106,9 +106,9 @@ fn elem_snippet(cx: &LateContext<'_>, elem: &Expr<'_>, symbol_name: &str) -> Str 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} let mut v = Vec::with_capacity({len}); +{indent} (0..{len}).for_each(|_| v.push({elem})); +{indent} v {indent}}}"# ) } @@ -116,8 +116,8 @@ fn loop_init_suggestion(elem: &str, len: &str, indent: &str) -> String { fn extract_suggestion(elem: &str, len: &str, indent: &str) -> String { format!( "{{ -{indent}{indent}let data = {elem}; -{indent}{indent}vec![data; {len}] +{indent} let data = {elem}; +{indent} vec![data; {len}] {indent}}}" ) } diff --git a/tests/ui/rc_clone_in_vec_init/arc.rs b/tests/ui/rc_clone_in_vec_init/arc.rs index bef2c67a1a54..384060e6eae5 100644 --- a/tests/ui/rc_clone_in_vec_init/arc.rs +++ b/tests/ui/rc_clone_in_vec_init/arc.rs @@ -7,6 +7,16 @@ fn should_warn_simple_case() { let v = vec![Arc::new("x".to_string()); 2]; } +fn should_warn_simple_case_with_big_indentation() { + if true { + let k = 1; + dbg!(k); + if true { + let v = vec![Arc::new("x".to_string()); 2]; + } + } +} + fn should_warn_complex_case() { let v = vec![ std::sync::Arc::new(Mutex::new({ diff --git a/tests/ui/rc_clone_in_vec_init/arc.stderr b/tests/ui/rc_clone_in_vec_init/arc.stderr index 387580c24310..ce84186c8e30 100644 --- a/tests/ui/rc_clone_in_vec_init/arc.stderr +++ b/tests/ui/rc_clone_in_vec_init/arc.stderr @@ -23,7 +23,30 @@ LL ~ }; | error: calling `Arc::new` in `vec![elem; len]` - --> $DIR/arc.rs:11:13 + --> $DIR/arc.rs:15:21 + | +LL | let v = vec![Arc::new("x".to_string()); 2]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: each element will point to the same `Arc` instance +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:21:13 | LL | let v = vec![ | _____________^ @@ -53,7 +76,7 @@ LL ~ }; | error: calling `Arc::new` in `vec![elem; len]` - --> $DIR/arc.rs:20:14 + --> $DIR/arc.rs:30:14 | LL | let v1 = vec![ | ______________^ @@ -82,5 +105,5 @@ LL + vec![data; 2] LL ~ }; | -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors diff --git a/tests/ui/rc_clone_in_vec_init/rc.rs b/tests/ui/rc_clone_in_vec_init/rc.rs index 79c23cafa2c1..0394457fe170 100644 --- a/tests/ui/rc_clone_in_vec_init/rc.rs +++ b/tests/ui/rc_clone_in_vec_init/rc.rs @@ -8,6 +8,16 @@ fn should_warn_simple_case() { let v = vec![Rc::new("x".to_string()); 2]; } +fn should_warn_simple_case_with_big_indentation() { + if true { + let k = 1; + dbg!(k); + if true { + let v = vec![Rc::new("x".to_string()); 2]; + } + } +} + fn should_warn_complex_case() { let v = vec![ std::rc::Rc::new(Mutex::new({ diff --git a/tests/ui/rc_clone_in_vec_init/rc.stderr b/tests/ui/rc_clone_in_vec_init/rc.stderr index 4ce53eecbbd8..0f5cc0cf98fe 100644 --- a/tests/ui/rc_clone_in_vec_init/rc.stderr +++ b/tests/ui/rc_clone_in_vec_init/rc.stderr @@ -23,7 +23,30 @@ LL ~ }; | error: calling `Rc::new` in `vec![elem; len]` - --> $DIR/rc.rs:12:13 + --> $DIR/rc.rs:16:21 + | +LL | let v = vec![Rc::new("x".to_string()); 2]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: each element will point to the same `Rc` instance +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:22:13 | LL | let v = vec![ | _____________^ @@ -53,7 +76,7 @@ LL ~ }; | error: calling `Rc::new` in `vec![elem; len]` - --> $DIR/rc.rs:21:14 + --> $DIR/rc.rs:31:14 | LL | let v1 = vec![ | ______________^ @@ -82,5 +105,5 @@ LL + vec![data; 2] LL ~ }; | -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors From ed3744b957d318f7b22b1a2d53b867f7da42746d Mon Sep 17 00:00:00 2001 From: yonip23 Date: Fri, 13 May 2022 02:20:28 +0300 Subject: [PATCH 5/5] inline construct_lint_suggestions --- clippy_lints/src/rc_clone_in_vec_init.rs | 61 ++++++++---------------- 1 file changed, 19 insertions(+), 42 deletions(-) diff --git a/clippy_lints/src/rc_clone_in_vec_init.rs b/clippy_lints/src/rc_clone_in_vec_init.rs index ca12509bdc7f..110f58f3734d 100644 --- a/clippy_lints/src/rc_clone_in_vec_init.rs +++ b/clippy_lints/src/rc_clone_in_vec_init.rs @@ -56,39 +56,6 @@ impl LateLintPass<'_> for RcCloneInVecInit { } } -struct LintSuggestion { - message: String, - snippet: String, -} - -fn construct_lint_suggestions( - cx: &LateContext<'_>, - span: Span, - symbol_name: &str, - elem: &Expr<'_>, - len: &Expr<'_>, -) -> Vec { - let len_snippet = snippet(cx, len.span, ".."); - let elem_snippet = elem_snippet(cx, elem, symbol_name); - let indentation = indent_of(cx, span).unwrap_or(0); - let indentation = " ".repeat(indentation); - let loop_init_suggestion = loop_init_suggestion(&elem_snippet, len_snippet.as_ref(), &indentation); - let extract_suggestion = extract_suggestion(&elem_snippet, len_snippet.as_ref(), &indentation); - - vec![ - LintSuggestion { - message: format!("consider initializing each `{symbol_name}` element individually"), - snippet: loop_init_suggestion, - }, - LintSuggestion { - message: format!( - "or if this is intentional, consider extracting the `{symbol_name}` initialization to a variable" - ), - snippet: extract_suggestion, - }, - ] -} - fn elem_snippet(cx: &LateContext<'_>, elem: &Expr<'_>, symbol_name: &str) -> String { let elem_snippet = snippet(cx, elem.span, "..").to_string(); if elem_snippet.contains('\n') { @@ -131,17 +98,27 @@ fn emit_lint(cx: &LateContext<'_>, symbol: Symbol, lint_span: Span, elem: &Expr< lint_span, &format!("calling `{symbol_name}::new` in `vec![elem; len]`"), |diag| { - let suggestions = construct_lint_suggestions(cx, lint_span, symbol_name, elem, len); + let len_snippet = snippet(cx, len.span, ".."); + let elem_snippet = elem_snippet(cx, elem, symbol_name); + let indentation = " ".repeat(indent_of(cx, lint_span).unwrap_or(0)); + let loop_init_suggestion = loop_init_suggestion(&elem_snippet, len_snippet.as_ref(), &indentation); + let extract_suggestion = extract_suggestion(&elem_snippet, len_snippet.as_ref(), &indentation); diag.note(format!("each element will point to the same `{symbol_name}` instance")); - for suggestion in suggestions { - diag.span_suggestion( - lint_span, - &suggestion.message, - &suggestion.snippet, - Applicability::Unspecified, - ); - } + diag.span_suggestion( + lint_span, + format!("consider initializing each `{symbol_name}` element individually"), + loop_init_suggestion, + Applicability::Unspecified, + ); + diag.span_suggestion( + lint_span, + format!( + "or if this is intentional, consider extracting the `{symbol_name}` initialization to a variable" + ), + extract_suggestion, + Applicability::Unspecified, + ); }, ); }