add suggestions to rc_clone_in_vec_init

This commit is contained in:
yonip23 2022-05-11 00:46:20 +03:00
parent d422baa30c
commit f3e01c4f6a
3 changed files with 152 additions and 18 deletions

View File

@ -1,11 +1,13 @@
use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::higher::VecArgs; use clippy_utils::higher::VecArgs;
use clippy_utils::last_path_segment; 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_hir::{Expr, ExprKind, QPath, TyKind};
use rustc_lint::{LateContext, LateLintPass}; use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{sym, Symbol}; use rustc_span::{sym, Span, Symbol};
declare_clippy_lint! { declare_clippy_lint! {
/// ### What it does /// ### What it does
@ -47,27 +49,107 @@
impl LateLintPass<'_> for RcCloneInVecInit { impl LateLintPass<'_> for RcCloneInVecInit {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
let Some(macro_call) = root_macro_call_first_node(cx, expr) else { return; }; 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 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, &macro_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<LintSuggestion> {
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(); let symbol_name = symbol.as_str();
span_lint_and_then( span_lint_and_then(
cx, cx,
RC_CLONE_IN_VEC_INIT, RC_CLONE_IN_VEC_INIT,
macro_call.span, lint_span,
&format!("calling `{symbol_name}::new` in `vec![elem; len]`"), &format!("calling `{symbol_name}::new` in `vec![elem; len]`"),
|diag| { |diag| {
diag.note(format!("each element will point to the same `{symbol_name}` instance")); diag.note(format!("each element will point to the same `{symbol_name}` instance"));
diag.help(format!( lint_suggestions
"if this is intentional, consider extracting the `{symbol_name}` initialization to a variable" .iter()
)); .for_each(|suggestion| suggestion.span_suggestion(diag));
diag.help("or if not, initialize each element individually");
}, },
); );
} }

View File

@ -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: `-D clippy::rc-clone-in-vec-init` implied by `-D warnings`
= note: each element will point to the same `Arc` instance = note: each element will point to the same `Arc` instance
= help: if this is intentional, consider extracting the `Arc` initialization to a variable help: consider initializing each `Arc` element individually
= help: or if not, initialize each 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]` error: calling `Arc::new` in `vec![elem; len]`
--> $DIR/arc.rs:11:13 --> $DIR/arc.rs:11:13
@ -23,8 +36,21 @@ LL | | ];
| |_____^ | |_____^
| |
= note: each element will point to the same `Arc` instance = note: each element will point to the same `Arc` instance
= help: if this is intentional, consider extracting the `Arc` initialization to a variable help: consider initializing each `Arc` element individually
= help: or if not, initialize each 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 error: aborting due to 2 previous errors

View File

@ -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: `-D clippy::rc-clone-in-vec-init` implied by `-D warnings`
= note: each element will point to the same `Rc` instance = note: each element will point to the same `Rc` instance
= help: if this is intentional, consider extracting the `Rc` initialization to a variable help: consider initializing each `Rc` element individually
= help: or if not, initialize each 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]` error: calling `Rc::new` in `vec![elem; len]`
--> $DIR/rc.rs:12:13 --> $DIR/rc.rs:12:13
@ -23,8 +36,21 @@ LL | | ];
| |_____^ | |_____^
| |
= note: each element will point to the same `Rc` instance = note: each element will point to the same `Rc` instance
= help: if this is intentional, consider extracting the `Rc` initialization to a variable help: consider initializing each `Rc` element individually
= help: or if not, initialize each 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 error: aborting due to 2 previous errors