Skip to content

Commit f70c73f

Browse files
committed
Auto merge of #8692 - kyoto7250:fixing_unnecessary_to_owned, r=giraffate
fix unnecessary_to_owned about msrv This PR fixes ``[`unnecessary_owned`]``. ## What ```rust # sample code fn _msrv_1_35() { #![clippy::msrv = "1.35"] let _ = &["x"][..].to_vec().into_iter(); } fn _msrv_1_36() { #![clippy::msrv = "1.36"] let _ = &["x"][..].to_vec().into_iter(); } ``` If we will check this code using clippy, ``[`unnecessary_owned`]`` will modify the code as follows. ```rust error: unnecessary use of `to_vec` --> $DIR/unnecessary_to_owned.rs:219:14 | LL | let _ = &["x"][..].to_vec().into_iter(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `["x"][..].iter().copied()` error: unnecessary use of `to_vec` --> $DIR/unnecessary_to_owned.rs:224:14 | LL | let _ = &["x"][..].to_vec().into_iter(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `["x"][..].iter().copied()` ``` This is incorrect. Because `Iterator::copied` was estabilished in 1.36. ## Why This bug was caused by not separating "copied" and "clone" by reference to msrv. https://github.com/rust-lang/rust-clippy/blob/89ee6aa6e32df974ef9dbb3c825afbdce441e856/clippy_lints/src/methods/unnecessary_to_owned.rs#L195 So, I added a conditional branch and described the corresponding test. Thank you in advance. changelog: fix wrong suggestions about msrv in [`unnecessary_to_owned`] r! `@giraffate`
2 parents 06b1695 + dfdc5ad commit f70c73f

File tree

5 files changed

+138
-87
lines changed

5 files changed

+138
-87
lines changed

clippy_lints/src/methods/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2286,7 +2286,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
22862286
single_char_add_str::check(cx, expr, args);
22872287
into_iter_on_ref::check(cx, expr, method_span, method_call.ident.name, args);
22882288
single_char_pattern::check(cx, expr, method_call.ident.name, args);
2289-
unnecessary_to_owned::check(cx, expr, method_call.ident.name, args);
2289+
unnecessary_to_owned::check(cx, expr, method_call.ident.name, args, self.msrv.as_ref());
22902290
},
22912291
hir::ExprKind::Binary(op, lhs, rhs) if op.node == hir::BinOpKind::Eq || op.node == hir::BinOpKind::Ne => {
22922292
let mut info = BinaryExprInfo {

clippy_lints/src/methods/unnecessary_to_owned.rs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ use clippy_utils::source::snippet_opt;
55
use clippy_utils::ty::{
66
contains_ty, get_associated_type, get_iterator_item_ty, implements_trait, is_copy, peel_mid_ty_refs,
77
};
8+
use clippy_utils::{meets_msrv, msrvs};
9+
810
use clippy_utils::{fn_def_id, get_parent_expr, is_diag_item_method, is_diag_trait_item};
911
use rustc_errors::Applicability;
1012
use rustc_hir::{def_id::DefId, BorrowKind, Expr, ExprKind};
@@ -13,12 +15,19 @@ use rustc_middle::mir::Mutability;
1315
use rustc_middle::ty::adjustment::{Adjust, Adjustment, OverloadedDeref};
1416
use rustc_middle::ty::subst::{GenericArg, GenericArgKind, SubstsRef};
1517
use rustc_middle::ty::{self, PredicateKind, ProjectionPredicate, TraitPredicate, Ty};
18+
use rustc_semver::RustcVersion;
1619
use rustc_span::{sym, Symbol};
1720
use std::cmp::max;
1821

1922
use super::UNNECESSARY_TO_OWNED;
2023

21-
pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, method_name: Symbol, args: &'tcx [Expr<'tcx>]) {
24+
pub fn check<'tcx>(
25+
cx: &LateContext<'tcx>,
26+
expr: &'tcx Expr<'tcx>,
27+
method_name: Symbol,
28+
args: &'tcx [Expr<'tcx>],
29+
msrv: Option<&RustcVersion>,
30+
) {
2231
if_chain! {
2332
if let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
2433
if let [receiver] = args;
@@ -33,7 +42,7 @@ pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, method_name:
3342
if check_addr_of_expr(cx, expr, method_name, method_def_id, receiver) {
3443
return;
3544
}
36-
if check_into_iter_call_arg(cx, expr, method_name, receiver) {
45+
if check_into_iter_call_arg(cx, expr, method_name, receiver, msrv) {
3746
return;
3847
}
3948
check_other_call_arg(cx, expr, method_name, receiver);
@@ -178,7 +187,13 @@ fn check_addr_of_expr(
178187

179188
/// Checks whether `expr` is an argument in an `into_iter` call and, if so, determines whether its
180189
/// call of a `to_owned`-like function is unnecessary.
181-
fn check_into_iter_call_arg(cx: &LateContext<'_>, expr: &Expr<'_>, method_name: Symbol, receiver: &Expr<'_>) -> bool {
190+
fn check_into_iter_call_arg(
191+
cx: &LateContext<'_>,
192+
expr: &Expr<'_>,
193+
method_name: Symbol,
194+
receiver: &Expr<'_>,
195+
msrv: Option<&RustcVersion>,
196+
) -> bool {
182197
if_chain! {
183198
if let Some(parent) = get_parent_expr(cx, expr);
184199
if let Some(callee_def_id) = fn_def_id(cx, parent);
@@ -192,7 +207,7 @@ fn check_into_iter_call_arg(cx: &LateContext<'_>, expr: &Expr<'_>, method_name:
192207
if unnecessary_iter_cloned::check_for_loop_iter(cx, parent, method_name, receiver, true) {
193208
return true;
194209
}
195-
let cloned_or_copied = if is_copy(cx, item_ty) { "copied" } else { "cloned" };
210+
let cloned_or_copied = if is_copy(cx, item_ty) && meets_msrv(msrv, &msrvs::ITERATOR_COPIED) { "copied" } else { "cloned" };
196211
// The next suggestion may be incorrect because the removal of the `to_owned`-like
197212
// function could cause the iterator to hold a reference to a resource that is used
198213
// mutably. See https://github.com/rust-lang/rust-clippy/issues/8148.

tests/ui/unnecessary_to_owned.fixed

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#![allow(clippy::ptr_arg)]
44
#![warn(clippy::unnecessary_to_owned)]
5+
#![feature(custom_inner_attributes)]
56

67
use std::borrow::Cow;
78
use std::ffi::{CStr, CString, OsStr, OsString};
@@ -213,6 +214,17 @@ fn get_file_path(_file_type: &FileType) -> Result<std::path::PathBuf, std::io::E
213214

214215
fn require_string(_: &String) {}
215216

217+
fn _msrv_1_35() {
218+
#![clippy::msrv = "1.35"]
219+
// `copied` was stabilized in 1.36, so clippy should use `cloned`.
220+
let _ = &["x"][..].iter().cloned();
221+
}
222+
223+
fn _msrv_1_36() {
224+
#![clippy::msrv = "1.36"]
225+
let _ = &["x"][..].iter().copied();
226+
}
227+
216228
// https://github.com/rust-lang/rust-clippy/issues/8507
217229
mod issue_8507 {
218230
#![allow(dead_code)]

tests/ui/unnecessary_to_owned.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#![allow(clippy::ptr_arg)]
44
#![warn(clippy::unnecessary_to_owned)]
5+
#![feature(custom_inner_attributes)]
56

67
use std::borrow::Cow;
78
use std::ffi::{CStr, CString, OsStr, OsString};
@@ -213,6 +214,17 @@ fn get_file_path(_file_type: &FileType) -> Result<std::path::PathBuf, std::io::E
213214

214215
fn require_string(_: &String) {}
215216

217+
fn _msrv_1_35() {
218+
#![clippy::msrv = "1.35"]
219+
// `copied` was stabilized in 1.36, so clippy should use `cloned`.
220+
let _ = &["x"][..].to_vec().into_iter();
221+
}
222+
223+
fn _msrv_1_36() {
224+
#![clippy::msrv = "1.36"]
225+
let _ = &["x"][..].to_vec().into_iter();
226+
}
227+
216228
// https://github.com/rust-lang/rust-clippy/issues/8507
217229
mod issue_8507 {
218230
#![allow(dead_code)]

0 commit comments

Comments
 (0)