chore: starting to fix unnecessary_iter_cloned (#13848)

This will address https://github.com/rust-lang/rust-clippy/issues/13099
for the unnecessary_iter_cloned test.

changelog: [unnecessary_iter_cloned]: unnecessary_iter_cloned
manual_assert to use multipart_suggestions where appropriate

5 files changed, 226 insertions, 44 deletions

diff --git a/clippy_lints/src/methods/unnecessary_iter_cloned.rs b/clippy_lints/src/methods/unnecessary_iter_cloned.rs
index 029704882dd..671c189a98e 100644
--- a/clippy_lints/src/methods/unnecessary_iter_cloned.rs
+++ b/clippy_lints/src/methods/unnecessary_iter_cloned.rs
@@ -6,6 +6,7 @@ use clippy_utils::ty::{get_iterator_item_ty, implements_trait};
 use clippy_utils::visitors::for_each_expr_without_closures;
 use clippy_utils::{can_mut_borrow_both, fn_def_id, get_parent_expr, path_to_local};
 use core::ops::ControlFlow;
+use itertools::Itertools;
 use rustc_errors::Applicability;
 use rustc_hir::def_id::DefId;
 use rustc_hir::{BindingMode, Expr, ExprKind, Node, PatKind};
@@ -122,14 +123,13 @@ pub fn check_for_loop_iter(
                 } else {
                     Applicability::MachineApplicable
                 };
-                diag.span_suggestion(expr.span, "use", snippet.to_owned(), applicability);
-                if !references_to_binding.is_empty() {
-                    diag.multipart_suggestion(
-                        "remove any references to the binding",
-                        references_to_binding,
-                        applicability,
-                    );
-                }
+
+                let combined = references_to_binding
+                    .into_iter()
+                    .chain(vec![(expr.span, snippet.to_owned())])
+                    .collect_vec();
+
+                diag.multipart_suggestion("remove any references to the binding", combined, applicability);
             },
         );
         return true;
diff --git a/tests/ui/unnecessary_iter_cloned.fixed b/tests/ui/unnecessary_iter_cloned.fixed
new file mode 100644
index 00000000000..dc5e163ff04
--- /dev/null
+++ b/tests/ui/unnecessary_iter_cloned.fixed
@@ -0,0 +1,201 @@
+#![allow(unused_assignments)]
+#![warn(clippy::unnecessary_to_owned)]
+
+#[allow(dead_code)]
+#[derive(Clone, Copy)]
+enum FileType {
+    Account,
+    PrivateKey,
+    Certificate,
+}
+
+fn main() {
+    let path = std::path::Path::new("x");
+
+    let _ = check_files(&[(FileType::Account, path)]);
+    let _ = check_files_vec(vec![(FileType::Account, path)]);
+
+    // negative tests
+    let _ = check_files_ref(&[(FileType::Account, path)]);
+    let _ = check_files_mut(&[(FileType::Account, path)]);
+    let _ = check_files_ref_mut(&[(FileType::Account, path)]);
+    let _ = check_files_self_and_arg(&[(FileType::Account, path)]);
+    let _ = check_files_mut_path_buf(&[(FileType::Account, std::path::PathBuf::new())]);
+
+    check_mut_iteratee_and_modify_inner_variable();
+}
+
+// `check_files` and its variants are based on:
+// https://github.com/breard-r/acmed/blob/1f0dcc32aadbc5e52de6d23b9703554c0f925113/acmed/src/storage.rs#L262
+fn check_files(files: &[(FileType, &std::path::Path)]) -> bool {
+    for (t, path) in files {
+        let other = match get_file_path(t) {
+            Ok(p) => p,
+            Err(_) => {
+                return false;
+            },
+        };
+        if !path.is_file() || !other.is_file() {
+            return false;
+        }
+    }
+    true
+}
+
+fn check_files_vec(files: Vec<(FileType, &std::path::Path)>) -> bool {
+    for (t, path) in files.iter() {
+        let other = match get_file_path(t) {
+            Ok(p) => p,
+            Err(_) => {
+                return false;
+            },
+        };
+        if !path.is_file() || !other.is_file() {
+            return false;
+        }
+    }
+    true
+}
+
+fn check_files_ref(files: &[(FileType, &std::path::Path)]) -> bool {
+    for (ref t, path) in files.iter().copied() {
+        let other = match get_file_path(t) {
+            Ok(p) => p,
+            Err(_) => {
+                return false;
+            },
+        };
+        if !path.is_file() || !other.is_file() {
+            return false;
+        }
+    }
+    true
+}
+
+#[allow(unused_assignments)]
+fn check_files_mut(files: &[(FileType, &std::path::Path)]) -> bool {
+    for (mut t, path) in files.iter().copied() {
+        t = FileType::PrivateKey;
+        let other = match get_file_path(&t) {
+            Ok(p) => p,
+            Err(_) => {
+                return false;
+            },
+        };
+        if !path.is_file() || !other.is_file() {
+            return false;
+        }
+    }
+    true
+}
+
+fn check_files_ref_mut(files: &[(FileType, &std::path::Path)]) -> bool {
+    for (ref mut t, path) in files.iter().copied() {
+        *t = FileType::PrivateKey;
+        let other = match get_file_path(t) {
+            Ok(p) => p,
+            Err(_) => {
+                return false;
+            },
+        };
+        if !path.is_file() || !other.is_file() {
+            return false;
+        }
+    }
+    true
+}
+
+fn check_files_self_and_arg(files: &[(FileType, &std::path::Path)]) -> bool {
+    for (t, path) in files.iter().copied() {
+        let other = match get_file_path(&t) {
+            Ok(p) => p,
+            Err(_) => {
+                return false;
+            },
+        };
+        if !path.join(path).is_file() || !other.is_file() {
+            return false;
+        }
+    }
+    true
+}
+
+#[allow(unused_assignments)]
+fn check_files_mut_path_buf(files: &[(FileType, std::path::PathBuf)]) -> bool {
+    for (mut t, path) in files.iter().cloned() {
+        t = FileType::PrivateKey;
+        let other = match get_file_path(&t) {
+            Ok(p) => p,
+            Err(_) => {
+                return false;
+            },
+        };
+        if !path.is_file() || !other.is_file() {
+            return false;
+        }
+    }
+    true
+}
+
+fn get_file_path(_file_type: &FileType) -> Result<std::path::PathBuf, std::io::Error> {
+    Ok(std::path::PathBuf::new())
+}
+
+// Issue 12098
+// https://github.com/rust-lang/rust-clippy/issues/12098
+// no message emits
+fn check_mut_iteratee_and_modify_inner_variable() {
+    struct Test {
+        list: Vec<String>,
+        mut_this: bool,
+    }
+
+    impl Test {
+        fn list(&self) -> &[String] {
+            &self.list
+        }
+    }
+
+    let mut test = Test {
+        list: vec![String::from("foo"), String::from("bar")],
+        mut_this: false,
+    };
+
+    for _item in test.list().to_vec() {
+        println!("{}", _item);
+
+        test.mut_this = true;
+        {
+            test.mut_this = true;
+        }
+    }
+}
+
+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`
+        }
+    }
+}
diff --git a/tests/ui/unnecessary_iter_cloned.rs b/tests/ui/unnecessary_iter_cloned.rs
index 331b7b25271..8f797ac717f 100644
--- a/tests/ui/unnecessary_iter_cloned.rs
+++ b/tests/ui/unnecessary_iter_cloned.rs
@@ -1,8 +1,6 @@
 #![allow(unused_assignments)]
 #![warn(clippy::unnecessary_to_owned)]
 
-//@no-rustfix: need to change the suggestion to a multipart suggestion
-
 #[allow(dead_code)]
 #[derive(Clone, Copy)]
 enum FileType {
diff --git a/tests/ui/unnecessary_iter_cloned.stderr b/tests/ui/unnecessary_iter_cloned.stderr
index e3592e3cbbd..6f2ae0ab1f3 100644
--- a/tests/ui/unnecessary_iter_cloned.stderr
+++ b/tests/ui/unnecessary_iter_cloned.stderr
@@ -1,71 +1,58 @@
 error: unnecessary use of `copied`
-  --> tests/ui/unnecessary_iter_cloned.rs:33:22
+  --> tests/ui/unnecessary_iter_cloned.rs:31:22
    |
 LL |     for (t, path) in files.iter().copied() {
    |                      ^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::unnecessary-to-owned` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::unnecessary_to_owned)]`
-help: use
-   |
-LL |     for (t, path) in files {
-   |                      ~~~~~
 help: remove any references to the binding
    |
-LL -         let other = match get_file_path(&t) {
-LL +         let other = match get_file_path(t) {
+LL ~     for (t, path) in files {
+LL ~         let other = match get_file_path(t) {
    |
 
 error: unnecessary use of `copied`
-  --> tests/ui/unnecessary_iter_cloned.rs:48:22
+  --> tests/ui/unnecessary_iter_cloned.rs:46:22
    |
 LL |     for (t, path) in files.iter().copied() {
    |                      ^^^^^^^^^^^^^^^^^^^^^
    |
-help: use
-   |
-LL |     for (t, path) in files.iter() {
-   |                      ~~~~~~~~~~~~
 help: remove any references to the binding
    |
-LL -         let other = match get_file_path(&t) {
-LL +         let other = match get_file_path(t) {
+LL ~     for (t, path) in files.iter() {
+LL ~         let other = match get_file_path(t) {
    |
 
 error: unnecessary use of `cloned`
-  --> tests/ui/unnecessary_iter_cloned.rs:179:18
+  --> tests/ui/unnecessary_iter_cloned.rs:177:18
    |
 LL |         for c in v.iter().cloned() {
-   |                  ^^^^^^^^^^^^^^^^^ help: use: `v.iter()`
+   |                  ^^^^^^^^^^^^^^^^^ help: remove any references to the binding: `v.iter()`
 
 error: unnecessary use of `cloned`
-  --> tests/ui/unnecessary_iter_cloned.rs:187:18
+  --> 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;
+LL ~         for c in v.iter() {
+LL |
+LL ~             let ref_c = c;
    |
 
 error: unnecessary use of `cloned`
-  --> tests/ui/unnecessary_iter_cloned.rs:196:23
+  --> 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 ~         for (i, c) in v.iter() {
+LL |
 LL ~             let ref_c = c;
 LL ~             let ref_i = i;
    |
diff --git a/tests/ui/unnecessary_to_owned.stderr b/tests/ui/unnecessary_to_owned.stderr
index 7ab1f667d9b..396802e16d5 100644
--- a/tests/ui/unnecessary_to_owned.stderr
+++ b/tests/ui/unnecessary_to_owned.stderr
@@ -519,14 +519,10 @@ error: unnecessary use of `to_vec`
 LL |     for t in file_types.to_vec() {
    |              ^^^^^^^^^^^^^^^^^^^
    |
-help: use
-   |
-LL |     for t in file_types {
-   |              ~~~~~~~~~~
 help: remove any references to the binding
    |
-LL -         let path = match get_file_path(&t) {
-LL +         let path = match get_file_path(t) {
+LL ~     for t in file_types {
+LL ~         let path = match get_file_path(t) {
    |
 
 error: unnecessary use of `to_vec`