don't call iter() on a temporary object in unnecessary_to_owned

rust-lang · Feb 26, 2025 · ee7f334 · ee7f334
1 parent 33dd099
commit ee7f334
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 104 deletions.
diff --git a/clippy_lints/src/methods/unnecessary_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs
@@ -6,7 +6,8 @@ use clippy_utils::source::{SpanRangeExt, snippet};
 use clippy_utils::ty::{get_iterator_item_ty, implements_trait, is_copy, is_type_diagnostic_item, is_type_lang_item};
 use clippy_utils::visitors::find_all_ret_expressions;
 use clippy_utils::{
-    fn_def_id, get_parent_expr, is_diag_item_method, is_diag_trait_item, peel_middle_ty_refs, return_ty,
+    fn_def_id, get_parent_expr, is_diag_item_method, is_diag_trait_item, is_expr_temporary_value, peel_middle_ty_refs,
+    return_ty,
 };
 use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
@@ -219,6 +220,8 @@ fn check_into_iter_call_arg(
         && let Some(receiver_snippet) = receiver.span.get_source_text(cx)
         // If the receiver is a `Cow`, we can't remove the `into_owned` generally, see https://github.com/rust-lang/rust-clippy/issues/13624.
         && !is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(receiver), sym::Cow)
+        // Calling `iter()` on a temporary object can lead to false positives. #14242
+        && is_expr_temporary_value(cx, receiver)
     {
         if unnecessary_iter_cloned::check_for_loop_iter(cx, parent, method_name, receiver, true) {
             return true;

diff --git a/tests/ui/unnecessary_to_owned.fixed b/tests/ui/unnecessary_to_owned.fixed
@@ -195,19 +195,11 @@ fn main() {
     //~^ unnecessary_to_owned
     let _ = slice.iter().copied();
     //~^ unnecessary_to_owned
-    let _ = [std::path::PathBuf::new()][..].iter().cloned();
-    //~^ unnecessary_to_owned
-    let _ = [std::path::PathBuf::new()][..].iter().cloned();
-    //~^ unnecessary_to_owned
 
     let _ = slice.iter().copied();
     //~^ unnecessary_to_owned
     let _ = slice.iter().copied();
     //~^ unnecessary_to_owned
-    let _ = [std::path::PathBuf::new()][..].iter().cloned();
-    //~^ unnecessary_to_owned
-    let _ = [std::path::PathBuf::new()][..].iter().cloned();
-    //~^ unnecessary_to_owned
 
     let _ = check_files(&[FileType::Account]);
 
@@ -317,19 +309,6 @@ fn get_file_path(_file_type: &FileType) -> Result<std::path::PathBuf, std::io::E
 
 fn require_string(_: &String) {}
 
-#[clippy::msrv = "1.35"]
-fn _msrv_1_35() {
-    // `copied` was stabilized in 1.36, so clippy should use `cloned`.
-    let _ = &["x"][..].iter().cloned();
-    //~^ unnecessary_to_owned
-}
-
-#[clippy::msrv = "1.36"]
-fn _msrv_1_36() {
-    let _ = &["x"][..].iter().copied();
-    //~^ unnecessary_to_owned
-}
-
 // https://github.com/rust-lang/rust-clippy/issues/8507
 mod issue_8507 {
     #![allow(dead_code)]
@@ -680,3 +659,18 @@ fn issue13624() -> impl IntoIterator {
 
     cow.into_owned().into_iter()
 }
+
+mod issue_14242 {
+    use std::rc::Rc;
+
+    #[derive(Copy, Clone)]
+    struct Foo;
+
+    fn rc_slice_provider() -> Rc<[Foo]> {
+        Rc::from([Foo])
+    }
+
+    fn iterator_provider() -> impl Iterator<Item = Foo> {
+        rc_slice_provider().to_vec().into_iter()
+    }
+}
diff --git a/tests/ui/unnecessary_to_owned.rs b/tests/ui/unnecessary_to_owned.rs
@@ -195,19 +195,11 @@ fn main() {
     //~^ unnecessary_to_owned
     let _ = slice.to_owned().into_iter();
     //~^ unnecessary_to_owned
-    let _ = [std::path::PathBuf::new()][..].to_vec().into_iter();
-    //~^ unnecessary_to_owned
-    let _ = [std::path::PathBuf::new()][..].to_owned().into_iter();
-    //~^ unnecessary_to_owned
 
     let _ = IntoIterator::into_iter(slice.to_vec());
     //~^ unnecessary_to_owned
     let _ = IntoIterator::into_iter(slice.to_owned());
     //~^ unnecessary_to_owned
-    let _ = IntoIterator::into_iter([std::path::PathBuf::new()][..].to_vec());
-    //~^ unnecessary_to_owned
-    let _ = IntoIterator::into_iter([std::path::PathBuf::new()][..].to_owned());
-    //~^ unnecessary_to_owned
 
     let _ = check_files(&[FileType::Account]);
 
@@ -317,19 +309,6 @@ fn get_file_path(_file_type: &FileType) -> Result<std::path::PathBuf, std::io::E
 
 fn require_string(_: &String) {}
 
-#[clippy::msrv = "1.35"]
-fn _msrv_1_35() {
-    // `copied` was stabilized in 1.36, so clippy should use `cloned`.
-    let _ = &["x"][..].to_vec().into_iter();
-    //~^ unnecessary_to_owned
-}
-
-#[clippy::msrv = "1.36"]
-fn _msrv_1_36() {
-    let _ = &["x"][..].to_vec().into_iter();
-    //~^ unnecessary_to_owned
-}
-
 // https://github.com/rust-lang/rust-clippy/issues/8507
 mod issue_8507 {
     #![allow(dead_code)]
@@ -680,3 +659,18 @@ fn issue13624() -> impl IntoIterator {
 
     cow.into_owned().into_iter()
 }
+
+mod issue_14242 {
+    use std::rc::Rc;
+
+    #[derive(Copy, Clone)]
+    struct Foo;
+
+    fn rc_slice_provider() -> Rc<[Foo]> {
+        Rc::from([Foo])
+    }
+
+    fn iterator_provider() -> impl Iterator<Item = Foo> {
+        rc_slice_provider().to_vec().into_iter()
+    }
+}
diff --git a/tests/ui/unnecessary_to_owned.stderr b/tests/ui/unnecessary_to_owned.stderr
@@ -1,61 +1,61 @@
 error: redundant clone
-  --> tests/ui/unnecessary_to_owned.rs:225:64
+  --> tests/ui/unnecessary_to_owned.rs:217:64
    |
 LL |     require_c_str(&CString::from_vec_with_nul(vec![0]).unwrap().to_owned());
    |                                                                ^^^^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> tests/ui/unnecessary_to_owned.rs:225:20
+  --> tests/ui/unnecessary_to_owned.rs:217:20
    |
 LL |     require_c_str(&CString::from_vec_with_nul(vec![0]).unwrap().to_owned());
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: `-D clippy::redundant-clone` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::redundant_clone)]`
 
 error: redundant clone
-  --> tests/ui/unnecessary_to_owned.rs:227:40
+  --> tests/ui/unnecessary_to_owned.rs:219:40
    |
 LL |     require_os_str(&OsString::from("x").to_os_string());
    |                                        ^^^^^^^^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> tests/ui/unnecessary_to_owned.rs:227:21
+  --> tests/ui/unnecessary_to_owned.rs:219:21
    |
 LL |     require_os_str(&OsString::from("x").to_os_string());
    |                     ^^^^^^^^^^^^^^^^^^^
 
 error: redundant clone
-  --> tests/ui/unnecessary_to_owned.rs:229:48
+  --> tests/ui/unnecessary_to_owned.rs:221:48
    |
 LL |     require_path(&std::path::PathBuf::from("x").to_path_buf());
    |                                                ^^^^^^^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> tests/ui/unnecessary_to_owned.rs:229:19
+  --> tests/ui/unnecessary_to_owned.rs:221:19
    |
 LL |     require_path(&std::path::PathBuf::from("x").to_path_buf());
    |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: redundant clone
-  --> tests/ui/unnecessary_to_owned.rs:231:35
+  --> tests/ui/unnecessary_to_owned.rs:223:35
    |
 LL |     require_str(&String::from("x").to_string());
    |                                   ^^^^^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> tests/ui/unnecessary_to_owned.rs:231:18
+  --> tests/ui/unnecessary_to_owned.rs:223:18
    |
 LL |     require_str(&String::from("x").to_string());
    |                  ^^^^^^^^^^^^^^^^^
 
 error: redundant clone
-  --> tests/ui/unnecessary_to_owned.rs:233:39
+  --> tests/ui/unnecessary_to_owned.rs:225:39
    |
 LL |     require_slice(&[String::from("x")].to_owned());
    |                                       ^^^^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> tests/ui/unnecessary_to_owned.rs:233:20
+  --> tests/ui/unnecessary_to_owned.rs:225:20
    |
 LL |     require_slice(&[String::from("x")].to_owned());
    |                    ^^^^^^^^^^^^^^^^^^^
@@ -442,43 +442,19 @@ LL |     let _ = slice.to_owned().into_iter();
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `slice.iter().copied()`
 
 error: unnecessary use of `to_vec`
-  --> tests/ui/unnecessary_to_owned.rs:198:13
-   |
-LL |     let _ = [std::path::PathBuf::new()][..].to_vec().into_iter();
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[std::path::PathBuf::new()][..].iter().cloned()`
-
-error: unnecessary use of `to_owned`
-  --> tests/ui/unnecessary_to_owned.rs:200:13
-   |
-LL |     let _ = [std::path::PathBuf::new()][..].to_owned().into_iter();
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[std::path::PathBuf::new()][..].iter().cloned()`
-
-error: unnecessary use of `to_vec`
-  --> tests/ui/unnecessary_to_owned.rs:203:13
+  --> tests/ui/unnecessary_to_owned.rs:199:13
    |
 LL |     let _ = IntoIterator::into_iter(slice.to_vec());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `slice.iter().copied()`
 
 error: unnecessary use of `to_owned`
-  --> tests/ui/unnecessary_to_owned.rs:205:13
+  --> tests/ui/unnecessary_to_owned.rs:201:13
    |
 LL |     let _ = IntoIterator::into_iter(slice.to_owned());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `slice.iter().copied()`
 
-error: unnecessary use of `to_vec`
-  --> tests/ui/unnecessary_to_owned.rs:207:13
-   |
-LL |     let _ = IntoIterator::into_iter([std::path::PathBuf::new()][..].to_vec());
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[std::path::PathBuf::new()][..].iter().cloned()`
-
-error: unnecessary use of `to_owned`
-  --> tests/ui/unnecessary_to_owned.rs:209:13
-   |
-LL |     let _ = IntoIterator::into_iter([std::path::PathBuf::new()][..].to_owned());
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[std::path::PathBuf::new()][..].iter().cloned()`
-
 error: allocating a new `String` only to create a temporary `&str` from it
-  --> tests/ui/unnecessary_to_owned.rs:237:26
+  --> tests/ui/unnecessary_to_owned.rs:229:26
    |
 LL |     let _ref_str: &str = &String::from_utf8(slice.to_vec()).expect("not UTF-8");
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -490,7 +466,7 @@ LL +     let _ref_str: &str = core::str::from_utf8(&slice).expect("not UTF-8");
    |
 
 error: allocating a new `String` only to create a temporary `&str` from it
-  --> tests/ui/unnecessary_to_owned.rs:239:26
+  --> tests/ui/unnecessary_to_owned.rs:231:26
    |
 LL |     let _ref_str: &str = &String::from_utf8(b"foo".to_vec()).unwrap();
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -502,7 +478,7 @@ LL +     let _ref_str: &str = core::str::from_utf8(b"foo").unwrap();
    |
 
 error: allocating a new `String` only to create a temporary `&str` from it
-  --> tests/ui/unnecessary_to_owned.rs:241:26
+  --> tests/ui/unnecessary_to_owned.rs:233:26
    |
 LL |     let _ref_str: &str = &String::from_utf8(b"foo".as_slice().to_owned()).unwrap();
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -514,7 +490,7 @@ LL +     let _ref_str: &str = core::str::from_utf8(b"foo".as_slice()).unwrap();
    |
 
 error: unnecessary use of `to_vec`
-  --> tests/ui/unnecessary_to_owned.rs:299:14
+  --> tests/ui/unnecessary_to_owned.rs:291:14
    |
 LL |     for t in file_types.to_vec() {
    |              ^^^^^^^^^^^^^^^^^^^
@@ -526,65 +502,53 @@ LL |
 LL ~         let path = match get_file_path(t) {
    |
 
-error: unnecessary use of `to_vec`
-  --> tests/ui/unnecessary_to_owned.rs:323:14
-   |
-LL |     let _ = &["x"][..].to_vec().into_iter();
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `["x"][..].iter().cloned()`
-
-error: unnecessary use of `to_vec`
-  --> tests/ui/unnecessary_to_owned.rs:329:14
-   |
-LL |     let _ = &["x"][..].to_vec().into_iter();
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `["x"][..].iter().copied()`
-
 error: unnecessary use of `to_string`
-  --> tests/ui/unnecessary_to_owned.rs:378:24
+  --> tests/ui/unnecessary_to_owned.rs:357:24
    |
 LL |         Box::new(build(y.to_string()))
    |                        ^^^^^^^^^^^^^ help: use: `y`
 
 error: unnecessary use of `to_string`
-  --> tests/ui/unnecessary_to_owned.rs:488:12
+  --> tests/ui/unnecessary_to_owned.rs:467:12
    |
 LL |         id("abc".to_string())
    |            ^^^^^^^^^^^^^^^^^ help: use: `"abc"`
 
 error: unnecessary use of `to_vec`
-  --> tests/ui/unnecessary_to_owned.rs:632:37
+  --> tests/ui/unnecessary_to_owned.rs:611:37
    |
 LL |         IntoFuture::into_future(foo([].to_vec(), &0));
    |                                     ^^^^^^^^^^^ help: use: `[]`
 
 error: unnecessary use of `to_vec`
-  --> tests/ui/unnecessary_to_owned.rs:643:18
+  --> tests/ui/unnecessary_to_owned.rs:622:18
    |
 LL |         s.remove(&a.to_vec());
    |                  ^^^^^^^^^^^ help: replace it with: `a`
 
 error: unnecessary use of `to_owned`
-  --> tests/ui/unnecessary_to_owned.rs:648:14
+  --> tests/ui/unnecessary_to_owned.rs:627:14
    |
 LL |     s.remove(&"b".to_owned());
    |              ^^^^^^^^^^^^^^^ help: replace it with: `"b"`
 
 error: unnecessary use of `to_string`
-  --> tests/ui/unnecessary_to_owned.rs:650:14
+  --> tests/ui/unnecessary_to_owned.rs:629:14
    |
 LL |     s.remove(&"b".to_string());
    |              ^^^^^^^^^^^^^^^^ help: replace it with: `"b"`
 
 error: unnecessary use of `to_vec`
-  --> tests/ui/unnecessary_to_owned.rs:656:14
+  --> tests/ui/unnecessary_to_owned.rs:635:14
    |
 LL |     s.remove(&["b"].to_vec());
    |              ^^^^^^^^^^^^^^^ help: replace it with: `["b"].as_slice()`
 
 error: unnecessary use of `to_vec`
-  --> tests/ui/unnecessary_to_owned.rs:658:14
+  --> tests/ui/unnecessary_to_owned.rs:637:14
    |
 LL |     s.remove(&(&["b"]).to_vec());
    |              ^^^^^^^^^^^^^^^^^^ help: replace it with: `(&["b"]).as_slice()`
 
-error: aborting due to 88 previous errors
+error: aborting due to 82 previous errors