unnecessary_to_owned should not suggest to remove & in macro expansion

This commit is contained in:
Marc Dominik Migge 2024-05-24 17:59:09 +02:00
parent ea535c97d5
commit 4a64180dd5
6 changed files with 117 additions and 19 deletions

View file

@ -38,7 +38,7 @@ pub fn check_for_loop_iter(
) -> bool {
if let Some(grandparent) = get_parent_expr(cx, expr).and_then(|parent| get_parent_expr(cx, parent))
&& let Some(ForLoop { pat, body, .. }) = ForLoop::hir(grandparent)
&& let (clone_or_copy_needed, addr_of_exprs) = clone_or_copy_needed(cx, pat, body)
&& let (clone_or_copy_needed, references_to_binding) = clone_or_copy_needed(cx, pat, body)
&& !clone_or_copy_needed
&& let Some(receiver_snippet) = snippet_opt(cx, receiver.span)
{
@ -123,14 +123,12 @@ pub fn check_for_loop_iter(
Applicability::MachineApplicable
};
diag.span_suggestion(expr.span, "use", snippet, applicability);
for addr_of_expr in addr_of_exprs {
match addr_of_expr.kind {
ExprKind::AddrOf(_, _, referent) => {
let span = addr_of_expr.span.with_hi(referent.span.lo());
diag.span_suggestion(span, "remove this `&`", "", applicability);
},
_ => unreachable!(),
}
if !references_to_binding.is_empty() {
diag.multipart_suggestion(
"remove any references to the binding",
references_to_binding,
applicability,
);
}
},
);

View file

@ -9,6 +9,7 @@ use rustc_lint::LateContext;
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::{self, Ty};
use rustc_span::symbol::sym;
use rustc_span::Span;
pub(super) fn derefs_to_slice<'tcx>(
cx: &LateContext<'tcx>,
@ -96,15 +97,15 @@ pub(super) fn clone_or_copy_needed<'tcx>(
cx: &LateContext<'tcx>,
pat: &Pat<'tcx>,
body: &'tcx Expr<'tcx>,
) -> (bool, Vec<&'tcx Expr<'tcx>>) {
) -> (bool, Vec<(Span, String)>) {
let mut visitor = CloneOrCopyVisitor {
cx,
binding_hir_ids: pat_bindings(pat),
clone_or_copy_needed: false,
addr_of_exprs: Vec::new(),
references_to_binding: Vec::new(),
};
visitor.visit_expr(body);
(visitor.clone_or_copy_needed, visitor.addr_of_exprs)
(visitor.clone_or_copy_needed, visitor.references_to_binding)
}
/// Returns a vector of all `HirId`s bound by the pattern.
@ -127,7 +128,7 @@ struct CloneOrCopyVisitor<'cx, 'tcx> {
cx: &'cx LateContext<'tcx>,
binding_hir_ids: Vec<HirId>,
clone_or_copy_needed: bool,
addr_of_exprs: Vec<&'tcx Expr<'tcx>>,
references_to_binding: Vec<(Span, String)>,
}
impl<'cx, 'tcx> Visitor<'tcx> for CloneOrCopyVisitor<'cx, 'tcx> {
@ -142,8 +143,11 @@ impl<'cx, 'tcx> Visitor<'tcx> for CloneOrCopyVisitor<'cx, 'tcx> {
if self.is_binding(expr) {
if let Some(parent) = get_parent_expr(self.cx, expr) {
match parent.kind {
ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, _) => {
self.addr_of_exprs.push(parent);
ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, referent) => {
if !parent.span.from_expansion() {
self.references_to_binding
.push((parent.span.until(referent.span), String::new()));
}
return;
},
ExprKind::MethodCall(.., args, _) => {

View file

@ -170,3 +170,32 @@ fn check_mut_iteratee_and_modify_inner_variable() {
}
}
}
mod issue_12821 {
fn foo() {
let v: Vec<_> = "hello".chars().collect();
for c in v.iter() {
//~^ ERROR: unnecessary use of `cloned`
println!("{c}"); // should not suggest to remove `&`
}
}
fn bar() {
let v: Vec<_> = "hello".chars().collect();
for c in v.iter() {
//~^ ERROR: unnecessary use of `cloned`
let ref_c = c; //~ HELP: remove any references to the binding
println!("{ref_c}");
}
}
fn baz() {
let v: Vec<_> = "hello".chars().enumerate().collect();
for (i, c) in v.iter() {
//~^ ERROR: unnecessary use of `cloned`
let ref_c = c; //~ HELP: remove any references to the binding
let ref_i = i;
println!("{i} {ref_c}"); // should not suggest to remove `&` from `i`
}
}
}

View file

@ -170,3 +170,32 @@ fn check_mut_iteratee_and_modify_inner_variable() {
}
}
}
mod issue_12821 {
fn foo() {
let v: Vec<_> = "hello".chars().collect();
for c in v.iter().cloned() {
//~^ ERROR: unnecessary use of `cloned`
println!("{c}"); // should not suggest to remove `&`
}
}
fn bar() {
let v: Vec<_> = "hello".chars().collect();
for c in v.iter().cloned() {
//~^ ERROR: unnecessary use of `cloned`
let ref_c = &c; //~ HELP: remove any references to the binding
println!("{ref_c}");
}
}
fn baz() {
let v: Vec<_> = "hello".chars().enumerate().collect();
for (i, c) in v.iter().cloned() {
//~^ ERROR: unnecessary use of `cloned`
let ref_c = &c; //~ HELP: remove any references to the binding
let ref_i = &i;
println!("{i} {ref_c}"); // should not suggest to remove `&` from `i`
}
}
}

View file

@ -10,7 +10,7 @@ help: use
|
LL | for (t, path) in files {
| ~~~~~
help: remove this `&`
help: remove any references to the binding
|
LL - let other = match get_file_path(&t) {
LL + let other = match get_file_path(t) {
@ -26,11 +26,49 @@ help: use
|
LL | for (t, path) in files.iter() {
| ~~~~~~~~~~~~
help: remove this `&`
help: remove any references to the binding
|
LL - let other = match get_file_path(&t) {
LL + let other = match get_file_path(t) {
|
error: aborting due to 2 previous errors
error: unnecessary use of `cloned`
--> tests/ui/unnecessary_iter_cloned.rs:177:18
|
LL | for c in v.iter().cloned() {
| ^^^^^^^^^^^^^^^^^ help: use: `v.iter()`
error: unnecessary use of `cloned`
--> tests/ui/unnecessary_iter_cloned.rs:185:18
|
LL | for c in v.iter().cloned() {
| ^^^^^^^^^^^^^^^^^
|
help: use
|
LL | for c in v.iter() {
| ~~~~~~~~
help: remove any references to the binding
|
LL - let ref_c = &c;
LL + let ref_c = c;
|
error: unnecessary use of `cloned`
--> tests/ui/unnecessary_iter_cloned.rs:194:23
|
LL | for (i, c) in v.iter().cloned() {
| ^^^^^^^^^^^^^^^^^
|
help: use
|
LL | for (i, c) in v.iter() {
| ~~~~~~~~
help: remove any references to the binding
|
LL ~ let ref_c = c;
LL ~ let ref_i = i;
|
error: aborting due to 5 previous errors

View file

@ -487,7 +487,7 @@ help: use
|
LL | for t in file_types {
| ~~~~~~~~~~
help: remove this `&`
help: remove any references to the binding
|
LL - let path = match get_file_path(&t) {
LL + let path = match get_file_path(t) {