Change unnecessary_to_owned into_iter suggestions to MaybeIncorrect

This commit is contained in:
Samuel E. Moelius III 2021-12-30 18:43:34 -05:00
parent 0eff589afc
commit a4ebf6f78e
2 changed files with 25 additions and 5 deletions

View file

@ -18,7 +18,7 @@ pub fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, method_name: Symbol
if let Some(callee_def_id) = fn_def_id(cx, parent);
if is_into_iter(cx, callee_def_id);
then {
check_for_loop_iter(cx, parent, method_name, receiver)
check_for_loop_iter(cx, parent, method_name, receiver, false)
} else {
false
}
@ -34,6 +34,7 @@ pub fn check_for_loop_iter(
expr: &'tcx Expr<'tcx>,
method_name: Symbol,
receiver: &'tcx Expr<'tcx>,
cloned_before_iter: bool,
) -> bool {
if_chain! {
if let Some(grandparent) = get_parent_expr(cx, expr).and_then(|parent| get_parent_expr(cx, parent));
@ -70,12 +71,22 @@ pub fn check_for_loop_iter(
expr.span,
&format!("unnecessary use of `{}`", method_name),
|diag| {
diag.span_suggestion(expr.span, "use", snippet, Applicability::MachineApplicable);
// If `check_into_iter_call_arg` called `check_for_loop_iter` because a call to
// a `to_owned`-like function was removed, then the next suggestion may be
// incorrect. This is because the iterator that results from the call's removal
// could hold a reference to a resource that is used mutably. See
// https://github.com/rust-lang/rust-clippy/issues/8148.
let applicability = if cloned_before_iter {
Applicability::MaybeIncorrect
} else {
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 `&`", String::new(), Applicability::MachineApplicable);
diag.span_suggestion(span, "remove this `&`", String::new(), applicability);
}
_ => unreachable!(),
}

View file

@ -187,7 +187,13 @@ fn check_into_iter_call_arg(
if let Some(item_ty) = get_iterator_item_ty(cx, parent_ty);
if let Some(receiver_snippet) = snippet_opt(cx, receiver.span);
then {
if unnecessary_iter_cloned::check_for_loop_iter(cx, parent, method_name, receiver) {
if unnecessary_iter_cloned::check_for_loop_iter(
cx,
parent,
method_name,
receiver,
true,
) {
return true;
}
let cloned_or_copied = if is_copy(cx, item_ty) {
@ -195,6 +201,9 @@ fn check_into_iter_call_arg(
} else {
"cloned"
};
// The next suggestion may be incorrect because the removal of the `to_owned`-like
// function could cause the iterator to hold a reference to a resource that is used
// mutably. See https://github.com/rust-lang/rust-clippy/issues/8148.
span_lint_and_sugg(
cx,
UNNECESSARY_TO_OWNED,
@ -202,7 +211,7 @@ fn check_into_iter_call_arg(
&format!("unnecessary use of `{}`", method_name),
"use",
format!("{}.iter().{}()", receiver_snippet, cloned_or_copied),
Applicability::MachineApplicable,
Applicability::MaybeIncorrect,
);
return true;
}