Skip to content

Commit 06f797d

Browse files
lapla-cogitolapla-cogito
committed
don't call iter() on a temporary object in unnecessary_to_owned 
1 parent 27e69a8 commit 06f797d

File tree

4 files changed

+59
-104
lines changed

4 files changed

+59
-104
lines changed

‎clippy_lints/src/methods/unnecessary_to_owned.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ use clippy_utils::source::{SpanRangeExt, snippet};
66
use clippy_utils::ty::{get_iterator_item_ty, implements_trait, is_copy, is_type_diagnostic_item, is_type_lang_item};
77
use clippy_utils::visitors::find_all_ret_expressions;
88
use clippy_utils::{
9-
fn_def_id, get_parent_expr, is_diag_item_method, is_diag_trait_item, peel_middle_ty_refs, return_ty,
9+
fn_def_id, get_parent_expr, is_diag_item_method, is_diag_trait_item, is_expr_temporary_value, peel_middle_ty_refs,
10+
return_ty,
1011
};
1112
use rustc_errors::Applicability;
1213
use rustc_hir::def::{DefKind, Res};
@@ -219,6 +220,8 @@ fn check_into_iter_call_arg(
219220
&& let Some(receiver_snippet) = receiver.span.get_source_text(cx)
220221
// If the receiver is a `Cow`, we can't remove the `into_owned` generally, see https://github.com/rust-lang/rust-clippy/issues/13624.
221222
&& !is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(receiver), sym::Cow)
223+
// Calling `iter()` on a temporary object can lead to false positives. #14242
224+
&& !is_expr_temporary_value(cx, receiver)
222225
{
223226
if unnecessary_iter_cloned::check_for_loop_iter(cx, parent, method_name, receiver, true) {
224227
return true;

‎tests/ui/unnecessary_to_owned.fixed

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -195,19 +195,11 @@ fn main() {
195195
//~^ unnecessary_to_owned
196196
let _ = slice.iter().copied();
197197
//~^ unnecessary_to_owned
198-
let _ = [std::path::PathBuf::new()][..].iter().cloned();
199-
//~^ unnecessary_to_owned
200-
let _ = [std::path::PathBuf::new()][..].iter().cloned();
201-
//~^ unnecessary_to_owned
202198

203199
let _ = slice.iter().copied();
204200
//~^ unnecessary_to_owned
205201
let _ = slice.iter().copied();
206202
//~^ unnecessary_to_owned
207-
let _ = [std::path::PathBuf::new()][..].iter().cloned();
208-
//~^ unnecessary_to_owned
209-
let _ = [std::path::PathBuf::new()][..].iter().cloned();
210-
//~^ unnecessary_to_owned
211203

212204
let _ = check_files(&[FileType::Account]);
213205

@@ -317,19 +309,6 @@ fn get_file_path(_file_type: &FileType) -> Result<std::path::PathBuf, std::io::E
317309

318310
fn require_string(_: &String) {}
319311

320-
#[clippy::msrv = "1.35"]
321-
fn _msrv_1_35() {
322-
// `copied` was stabilized in 1.36, so clippy should use `cloned`.
323-
let _ = &["x"][..].iter().cloned();
324-
//~^ unnecessary_to_owned
325-
}
326-
327-
#[clippy::msrv = "1.36"]
328-
fn _msrv_1_36() {
329-
let _ = &["x"][..].iter().copied();
330-
//~^ unnecessary_to_owned
331-
}
332-
333312
// https://github.com/rust-lang/rust-clippy/issues/8507
334313
mod issue_8507 {
335314
#![allow(dead_code)]
@@ -680,3 +659,18 @@ fn issue13624() -> impl IntoIterator {
680659

681660
cow.into_owned().into_iter()
682661
}
662+
663+
mod issue_14242 {
664+
use std::rc::Rc;
665+
666+
#[derive(Copy, Clone)]
667+
struct Foo;
668+
669+
fn rc_slice_provider() -> Rc<[Foo]> {
670+
Rc::from([Foo])
671+
}
672+
673+
fn iterator_provider() -> impl Iterator<Item = Foo> {
674+
rc_slice_provider().to_vec().into_iter()
675+
}
676+
}

‎tests/ui/unnecessary_to_owned.rs

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -195,19 +195,11 @@ fn main() {
195195
//~^ unnecessary_to_owned
196196
let _ = slice.to_owned().into_iter();
197197
//~^ unnecessary_to_owned
198-
let _ = [std::path::PathBuf::new()][..].to_vec().into_iter();
199-
//~^ unnecessary_to_owned
200-
let _ = [std::path::PathBuf::new()][..].to_owned().into_iter();
201-
//~^ unnecessary_to_owned
202198

203199
let _ = IntoIterator::into_iter(slice.to_vec());
204200
//~^ unnecessary_to_owned
205201
let _ = IntoIterator::into_iter(slice.to_owned());
206202
//~^ unnecessary_to_owned
207-
let _ = IntoIterator::into_iter([std::path::PathBuf::new()][..].to_vec());
208-
//~^ unnecessary_to_owned
209-
let _ = IntoIterator::into_iter([std::path::PathBuf::new()][..].to_owned());
210-
//~^ unnecessary_to_owned
211203

212204
let _ = check_files(&[FileType::Account]);
213205

@@ -317,19 +309,6 @@ fn get_file_path(_file_type: &FileType) -> Result<std::path::PathBuf, std::io::E
317309

318310
fn require_string(_: &String) {}
319311

320-
#[clippy::msrv = "1.35"]
321-
fn _msrv_1_35() {
322-
// `copied` was stabilized in 1.36, so clippy should use `cloned`.
323-
let _ = &["x"][..].to_vec().into_iter();
324-
//~^ unnecessary_to_owned
325-
}
326-
327-
#[clippy::msrv = "1.36"]
328-
fn _msrv_1_36() {
329-
let _ = &["x"][..].to_vec().into_iter();
330-
//~^ unnecessary_to_owned
331-
}
332-
333312
// https://github.com/rust-lang/rust-clippy/issues/8507
334313
mod issue_8507 {
335314
#![allow(dead_code)]
@@ -680,3 +659,18 @@ fn issue13624() -> impl IntoIterator {
680659

681660
cow.into_owned().into_iter()
682661
}
662+
663+
mod issue_14242 {
664+
use std::rc::Rc;
665+
666+
#[derive(Copy, Clone)]
667+
struct Foo;
668+
669+
fn rc_slice_provider() -> Rc<[Foo]> {
670+
Rc::from([Foo])
671+
}
672+
673+
fn iterator_provider() -> impl Iterator<Item = Foo> {
674+
rc_slice_provider().to_vec().into_iter()
675+
}
676+
}

‎tests/ui/unnecessary_to_owned.stderr

Lines changed: 25 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,61 +1,61 @@
11
error: redundant clone
2-
--> tests/ui/unnecessary_to_owned.rs:225:64
2+
--> tests/ui/unnecessary_to_owned.rs:217:64
33
|
44
LL | require_c_str(&CString::from_vec_with_nul(vec![0]).unwrap().to_owned());
55
| ^^^^^^^^^^^ help: remove this
66
|
77
note: this value is dropped without further use
8-
--> tests/ui/unnecessary_to_owned.rs:225:20
8+
--> tests/ui/unnecessary_to_owned.rs:217:20
99
|
1010
LL | require_c_str(&CString::from_vec_with_nul(vec![0]).unwrap().to_owned());
1111
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1212
= note: `-D clippy::redundant-clone` implied by `-D warnings`
1313
= help: to override `-D warnings` add `#[allow(clippy::redundant_clone)]`
1414

1515
error: redundant clone
16-
--> tests/ui/unnecessary_to_owned.rs:227:40
16+
--> tests/ui/unnecessary_to_owned.rs:219:40
1717
|
1818
LL | require_os_str(&OsString::from("x").to_os_string());
1919
| ^^^^^^^^^^^^^^^ help: remove this
2020
|
2121
note: this value is dropped without further use
22-
--> tests/ui/unnecessary_to_owned.rs:227:21
22+
--> tests/ui/unnecessary_to_owned.rs:219:21
2323
|
2424
LL | require_os_str(&OsString::from("x").to_os_string());
2525
| ^^^^^^^^^^^^^^^^^^^
2626

2727
error: redundant clone
28-
--> tests/ui/unnecessary_to_owned.rs:229:48
28+
--> tests/ui/unnecessary_to_owned.rs:221:48
2929
|
3030
LL | require_path(&std::path::PathBuf::from("x").to_path_buf());
3131
| ^^^^^^^^^^^^^^ help: remove this
3232
|
3333
note: this value is dropped without further use
34-
--> tests/ui/unnecessary_to_owned.rs:229:19
34+
--> tests/ui/unnecessary_to_owned.rs:221:19
3535
|
3636
LL | require_path(&std::path::PathBuf::from("x").to_path_buf());
3737
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3838

3939
error: redundant clone
40-
--> tests/ui/unnecessary_to_owned.rs:231:35
40+
--> tests/ui/unnecessary_to_owned.rs:223:35
4141
|
4242
LL | require_str(&String::from("x").to_string());
4343
| ^^^^^^^^^^^^ help: remove this
4444
|
4545
note: this value is dropped without further use
46-
--> tests/ui/unnecessary_to_owned.rs:231:18
46+
--> tests/ui/unnecessary_to_owned.rs:223:18
4747
|
4848
LL | require_str(&String::from("x").to_string());
4949
| ^^^^^^^^^^^^^^^^^
5050

5151
error: redundant clone
52-
--> tests/ui/unnecessary_to_owned.rs:233:39
52+
--> tests/ui/unnecessary_to_owned.rs:225:39
5353
|
5454
LL | require_slice(&[String::from("x")].to_owned());
5555
| ^^^^^^^^^^^ help: remove this
5656
|
5757
note: this value is dropped without further use
58-
--> tests/ui/unnecessary_to_owned.rs:233:20
58+
--> tests/ui/unnecessary_to_owned.rs:225:20
5959
|
6060
LL | require_slice(&[String::from("x")].to_owned());
6161
| ^^^^^^^^^^^^^^^^^^^
@@ -442,43 +442,19 @@ LL | let _ = slice.to_owned().into_iter();
442442
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `slice.iter().copied()`
443443

444444
error: unnecessary use of `to_vec`
445-
--> tests/ui/unnecessary_to_owned.rs:198:13
446-
|
447-
LL | let _ = [std::path::PathBuf::new()][..].to_vec().into_iter();
448-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[std::path::PathBuf::new()][..].iter().cloned()`
449-
450-
error: unnecessary use of `to_owned`
451-
--> tests/ui/unnecessary_to_owned.rs:200:13
452-
|
453-
LL | let _ = [std::path::PathBuf::new()][..].to_owned().into_iter();
454-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[std::path::PathBuf::new()][..].iter().cloned()`
455-
456-
error: unnecessary use of `to_vec`
457-
--> tests/ui/unnecessary_to_owned.rs:203:13
445+
--> tests/ui/unnecessary_to_owned.rs:199:13
458446
|
459447
LL | let _ = IntoIterator::into_iter(slice.to_vec());
460448
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `slice.iter().copied()`
461449

462450
error: unnecessary use of `to_owned`
463-
--> tests/ui/unnecessary_to_owned.rs:205:13
451+
--> tests/ui/unnecessary_to_owned.rs:201:13
464452
|
465453
LL | let _ = IntoIterator::into_iter(slice.to_owned());
466454
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `slice.iter().copied()`
467455

468-
error: unnecessary use of `to_vec`
469-
--> tests/ui/unnecessary_to_owned.rs:207:13
470-
|
471-
LL | let _ = IntoIterator::into_iter([std::path::PathBuf::new()][..].to_vec());
472-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[std::path::PathBuf::new()][..].iter().cloned()`
473-
474-
error: unnecessary use of `to_owned`
475-
--> tests/ui/unnecessary_to_owned.rs:209:13
476-
|
477-
LL | let _ = IntoIterator::into_iter([std::path::PathBuf::new()][..].to_owned());
478-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[std::path::PathBuf::new()][..].iter().cloned()`
479-
480456
error: allocating a new `String` only to create a temporary `&str` from it
481-
--> tests/ui/unnecessary_to_owned.rs:237:26
457+
--> tests/ui/unnecessary_to_owned.rs:229:26
482458
|
483459
LL | let _ref_str: &str = &String::from_utf8(slice.to_vec()).expect("not UTF-8");
484460
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -490,7 +466,7 @@ LL + let _ref_str: &str = core::str::from_utf8(&slice).expect("not UTF-8");
490466
|
491467

492468
error: allocating a new `String` only to create a temporary `&str` from it
493-
--> tests/ui/unnecessary_to_owned.rs:239:26
469+
--> tests/ui/unnecessary_to_owned.rs:231:26
494470
|
495471
LL | let _ref_str: &str = &String::from_utf8(b"foo".to_vec()).unwrap();
496472
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -502,7 +478,7 @@ LL + let _ref_str: &str = core::str::from_utf8(b"foo").unwrap();
502478
|
503479

504480
error: allocating a new `String` only to create a temporary `&str` from it
505-
--> tests/ui/unnecessary_to_owned.rs:241:26
481+
--> tests/ui/unnecessary_to_owned.rs:233:26
506482
|
507483
LL | let _ref_str: &str = &String::from_utf8(b"foo".as_slice().to_owned()).unwrap();
508484
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -514,7 +490,7 @@ LL + let _ref_str: &str = core::str::from_utf8(b"foo".as_slice()).unwrap();
514490
|
515491

516492
error: unnecessary use of `to_vec`
517-
--> tests/ui/unnecessary_to_owned.rs:299:14
493+
--> tests/ui/unnecessary_to_owned.rs:291:14
518494
|
519495
LL | for t in file_types.to_vec() {
520496
| ^^^^^^^^^^^^^^^^^^^
@@ -526,65 +502,53 @@ LL |
526502
LL ~ let path = match get_file_path(t) {
527503
|
528504

529-
error: unnecessary use of `to_vec`
530-
--> tests/ui/unnecessary_to_owned.rs:323:14
531-
|
532-
LL | let _ = &["x"][..].to_vec().into_iter();
533-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `["x"][..].iter().cloned()`
534-
535-
error: unnecessary use of `to_vec`
536-
--> tests/ui/unnecessary_to_owned.rs:329:14
537-
|
538-
LL | let _ = &["x"][..].to_vec().into_iter();
539-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `["x"][..].iter().copied()`
540-
541505
error: unnecessary use of `to_string`
542-
--> tests/ui/unnecessary_to_owned.rs:378:24
506+
--> tests/ui/unnecessary_to_owned.rs:357:24
543507
|
544508
LL | Box::new(build(y.to_string()))
545509
| ^^^^^^^^^^^^^ help: use: `y`
546510

547511
error: unnecessary use of `to_string`
548-
--> tests/ui/unnecessary_to_owned.rs:488:12
512+
--> tests/ui/unnecessary_to_owned.rs:467:12
549513
|
550514
LL | id("abc".to_string())
551515
| ^^^^^^^^^^^^^^^^^ help: use: `"abc"`
552516

553517
error: unnecessary use of `to_vec`
554-
--> tests/ui/unnecessary_to_owned.rs:632:37
518+
--> tests/ui/unnecessary_to_owned.rs:611:37
555519
|
556520
LL | IntoFuture::into_future(foo([].to_vec(), &0));
557521
| ^^^^^^^^^^^ help: use: `[]`
558522

559523
error: unnecessary use of `to_vec`
560-
--> tests/ui/unnecessary_to_owned.rs:643:18
524+
--> tests/ui/unnecessary_to_owned.rs:622:18
561525
|
562526
LL | s.remove(&a.to_vec());
563527
| ^^^^^^^^^^^ help: replace it with: `a`
564528

565529
error: unnecessary use of `to_owned`
566-
--> tests/ui/unnecessary_to_owned.rs:648:14
530+
--> tests/ui/unnecessary_to_owned.rs:627:14
567531
|
568532
LL | s.remove(&"b".to_owned());
569533
| ^^^^^^^^^^^^^^^ help: replace it with: `"b"`
570534

571535
error: unnecessary use of `to_string`
572-
--> tests/ui/unnecessary_to_owned.rs:650:14
536+
--> tests/ui/unnecessary_to_owned.rs:629:14
573537
|
574538
LL | s.remove(&"b".to_string());
575539
| ^^^^^^^^^^^^^^^^ help: replace it with: `"b"`
576540

577541
error: unnecessary use of `to_vec`
578-
--> tests/ui/unnecessary_to_owned.rs:656:14
542+
--> tests/ui/unnecessary_to_owned.rs:635:14
579543
|
580544
LL | s.remove(&["b"].to_vec());
581545
| ^^^^^^^^^^^^^^^ help: replace it with: `["b"].as_slice()`
582546

583547
error: unnecessary use of `to_vec`
584-
--> tests/ui/unnecessary_to_owned.rs:658:14
548+
--> tests/ui/unnecessary_to_owned.rs:637:14
585549
|
586550
LL | s.remove(&(&["b"]).to_vec());
587551
| ^^^^^^^^^^^^^^^^^^ help: replace it with: `(&["b"]).as_slice()`
588552

589-
error: aborting due to 88 previous errors
553+
error: aborting due to 82 previous errors
590554

0 commit comments

Comments
 (0)