From a4ebf6f78e8a3f586251e7d0e4c72af51b436de5 Mon Sep 17 00:00:00 2001 From: "Samuel E. Moelius III" Date: Thu, 30 Dec 2021 18:43:34 -0500 Subject: [PATCH] Change `unnecessary_to_owned` `into_iter` suggestions to `MaybeIncorrect` --- .../src/methods/unnecessary_iter_cloned.rs | 17 ++++++++++++++--- .../src/methods/unnecessary_to_owned.rs | 13 +++++++++++-- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_iter_cloned.rs b/clippy_lints/src/methods/unnecessary_iter_cloned.rs index 8300df03e99..4f589a6d318 100644 --- a/clippy_lints/src/methods/unnecessary_iter_cloned.rs +++ b/clippy_lints/src/methods/unnecessary_iter_cloned.rs @@ -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!(), } diff --git a/clippy_lints/src/methods/unnecessary_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs index c48bacfce0d..7e17f163b21 100644 --- a/clippy_lints/src/methods/unnecessary_to_owned.rs +++ b/clippy_lints/src/methods/unnecessary_to_owned.rs @@ -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; }