Auto merge of #8201 - smoelius:master, r=camsteffen

Change `unnecessary_to_owned` `into_iter` suggestions to `MaybeIncorrect`

I am having a hard time finding a good solution for #8148, so I am wondering if is enough to just change the suggestion's applicability to `MaybeIncorrect`?

I apologize, as I realize this is a bit of a cop out.

changelog: none

3 files changed, 30 insertions, 5 deletions

diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 8a2a468c852..3741a1cdc5e 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -1895,6 +1895,11 @@ declare_clippy_lint! {
     /// ### Why is this bad?
     /// The unnecessary calls result in useless allocations.
     ///
+    /// ### Known problems
+    /// `unnecessary_to_owned` can falsely trigger if `IntoIterator::into_iter` is applied to an
+    /// owned copy of a resource and the resource is later used mutably. See
+    /// [#8148](https://github.com/rust-lang/rust-clippy/issues/8148).
+    ///
     /// ### Example
     /// ```rust
     /// let path = std::path::Path::new("x");
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;
         }