Skip to content

Commit 81643e2

Browse files
authored
unnecessary_to_owned: don't call iter() on a temporary object (#14243)
fix #14242 changelog: [`unnecessary_to_owned`]: don't call `iter` on a temporary object
2 parents 231bf45 + 06f797d commit 81643e2

File tree

6 files changed

+73
-112
lines changed

6 files changed

+73
-112
lines changed

clippy_lints/src/casts/borrow_as_ptr.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,10 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::msrvs::Msrv;
33
use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
44
use clippy_utils::sugg::has_enclosing_paren;
5-
use clippy_utils::{is_lint_allowed, msrvs, std_or_core};
5+
use clippy_utils::{is_expr_temporary_value, is_lint_allowed, msrvs, std_or_core};
66
use rustc_errors::Applicability;
77
use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, Ty, TyKind};
88
use rustc_lint::LateContext;
9-
use rustc_middle::ty::adjustment::Adjust;
109
use rustc_span::BytePos;
1110

1211
use super::BORROW_AS_PTR;
@@ -25,12 +24,7 @@ pub(super) fn check<'tcx>(
2524
let mut app = Applicability::MachineApplicable;
2625
let snip = snippet_with_context(cx, e.span, cast_expr.span.ctxt(), "..", &mut app).0;
2726
// Fix #9884
28-
if !e.is_place_expr(|base| {
29-
cx.typeck_results()
30-
.adjustments()
31-
.get(base.hir_id)
32-
.is_some_and(|x| x.iter().any(|adj| matches!(adj.kind, Adjust::Deref(_))))
33-
}) {
27+
if is_expr_temporary_value(cx, e) {
3428
return false;
3529
}
3630

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;

clippy_utils/src/lib.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2331,6 +2331,18 @@ pub fn is_expr_final_block_expr(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool {
23312331
matches!(tcx.parent_hir_node(expr.hir_id), Node::Block(..))
23322332
}
23332333

2334+
/// Checks if the expression is a temporary value.
2335+
// This logic is the same as the one used in rustc's `check_named_place_expr function`.
2336+
// https://github.com/rust-lang/rust/blob/3ed2a10d173d6c2e0232776af338ca7d080b1cd4/compiler/rustc_hir_typeck/src/expr.rs#L482-L499
2337+
pub fn is_expr_temporary_value(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
2338+
!expr.is_place_expr(|base| {
2339+
cx.typeck_results()
2340+
.adjustments()
2341+
.get(base.hir_id)
2342+
.is_some_and(|x| x.iter().any(|adj| matches!(adj.kind, Adjust::Deref(_))))
2343+
})
2344+
}
2345+
23342346
pub fn std_or_core(cx: &LateContext<'_>) -> Option<&'static str> {
23352347
if !is_no_std_crate(cx) {
23362348
Some("std")

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+
}

0 commit comments

Comments
 (0)