mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Address unnecessary_to_owned false positive

Browse Source
This commit is contained in:
Samuel E. Moelius III 2022-05-05 21:39:54 -04:00
parent 95f8b26002
commit 91a822c162
5 changed files with 160 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
clippy_lints/src/methods/unnecessary_to_owned.rs
View File

@ -65,13 +65,12 @@ fn check_addr_of_expr(
if let Some(parent) = get_parent_expr(cx, expr);
if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, _) = parent.kind;
let adjustments = cx.typeck_results().expr_adjustments(parent).iter().collect::<Vec<_>>();
if let Some(target_ty) = match adjustments[..]
{
if let
// For matching uses of `Cow::from`
[
Adjustment {
kind: Adjust::Deref(None),
..
target: referent_ty,
},
Adjustment {
kind: Adjust::Borrow(_),
@ -82,7 +81,7 @@ fn check_addr_of_expr(
| [
Adjustment {
kind: Adjust::Deref(None),
..
target: referent_ty,
},
Adjustment {
kind: Adjust::Borrow(_),
@ -97,7 +96,7 @@ fn check_addr_of_expr(
| [
Adjustment {
kind: Adjust::Deref(None),
..
target: referent_ty,
},
Adjustment {
kind: Adjust::Deref(Some(OverloadedDeref { .. })),
@ -107,17 +106,24 @@ fn check_addr_of_expr(
kind: Adjust::Borrow(_),
target: target_ty,
},
] => Some(target_ty),
_ => None,
};
] = adjustments[..];
let receiver_ty = cx.typeck_results().expr_ty(receiver);
// Only flag cases where the receiver is copyable or the method is `Cow::into_owned`. This
// restriction is to ensure there is not overlap between `redundant_clone` and this lint.
if is_copy(cx, receiver_ty) || is_cow_into_owned(cx, method_name, method_def_id);
if let Some(receiver_snippet) = snippet_opt(cx, receiver.span);
then {
let (target_ty, n_target_refs) = peel_mid_ty_refs(*target_ty);
let (receiver_ty, n_receiver_refs) = peel_mid_ty_refs(receiver_ty);
// Only flag cases satisfying at least one of the following three conditions:
// * the referent and receiver types are distinct
// * the referent/receiver type is a copyable array
// * the method is `Cow::into_owned`
// This restriction is to ensure there is no overlap between `redundant_clone` and this
// lint. It also avoids the following false positive:
// https://github.com/rust-lang/rust-clippy/issues/8759
// Arrays are a bit of a corner case. Non-copyable arrays are handled by
// `redundant_clone`, but copyable arrays are not.
if *referent_ty != receiver_ty
|| (matches!(referent_ty.kind(), ty::Array(..)) && is_copy(cx, *referent_ty))
|| is_cow_into_owned(cx, method_name, method_def_id);
if let Some(receiver_snippet) = snippet_opt(cx, receiver.span);
then {
if receiver_ty == target_ty && n_target_refs >= n_receiver_refs {
span_lint_and_sugg(
cx,
@ -207,7 +213,11 @@ fn check_into_iter_call_arg(
if unnecessary_iter_cloned::check_for_loop_iter(cx, parent, method_name, receiver, true) {
return true;
}
let cloned_or_copied = if is_copy(cx, item_ty) && meets_msrv(msrv, &msrvs::ITERATOR_COPIED) { "copied" } else { "cloned" };
let cloned_or_copied = if is_copy(cx, item_ty) && meets_msrv(msrv, &msrvs::ITERATOR_COPIED) {
"copied"
} else {
"cloned"
};
// The next suggestion may be incorrect because the removal of the `to_owned`-like
// function could cause the iterator to hold a reference to a resource that is used
// mutably. See https://github.com/rust-lang/rust-clippy/issues/8148.

11
tests/ui/recursive_format_impl.stderr
View File

@ -6,15 +6,6 @@ LL | write!(f, "{}", self.to_string())
|
= note: `-D clippy::recursive-format-impl` implied by `-D warnings`
error: unnecessary use of `to_string`
--> $DIR/recursive_format_impl.rs:61:50
|
LL | Self::E(string) => write!(f, "E {}", string.to_string()),
| ^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::unnecessary-to-owned` implied by `-D warnings`
= note: this error originates in the macro `$crate::format_args` (in Nightly builds, run with -Z macro-backtrace for more info)
error: using `self` as `Display` in `impl Display` will cause infinite recursion
--> $DIR/recursive_format_impl.rs:73:9
|
@ -87,5 +78,5 @@ LL | write!(f, "{}", &&**&&*self)
|
= note: this error originates in the macro `write` (in Nightly builds, run with -Z macro-backtrace for more info)
error: aborting due to 11 previous errors
error: aborting due to 10 previous errors

61
tests/ui/unnecessary_to_owned.fixed
View File

@ -78,10 +78,10 @@ fn main() {
require_slice(array.as_ref());
require_slice(array_ref.as_ref());
require_slice(slice);
require_slice(x_ref);
require_slice(&x_ref.to_owned()); // No longer flagged because of #8759.
require_x(&Cow::<X>::Owned(x.clone()));
require_x(x_ref);
require_x(&x_ref.to_owned()); // No longer flagged because of #8759.
require_deref_c_str(c_str);
require_deref_os_str(os_str);
@ -152,6 +152,7 @@ fn main() {
require_os_str(&OsString::from("x"));
require_path(&std::path::PathBuf::from("x"));
require_str(&String::from("x"));
require_slice(&[String::from("x")]);
}
fn require_c_str(_: &CStr) {}
@ -272,3 +273,59 @@ mod issue_8507 {
Box::new(build(y))
}
}
// https://github.com/rust-lang/rust-clippy/issues/8759
mod issue_8759 {
#![allow(dead_code)]
#[derive(Default)]
struct View {}
impl std::borrow::ToOwned for View {
type Owned = View;
fn to_owned(&self) -> Self::Owned {
View {}
}
}
#[derive(Default)]
struct RenderWindow {
default_view: View,
}
impl RenderWindow {
fn default_view(&self) -> &View {
&self.default_view
}
fn set_view(&mut self, _view: &View) {}
}
fn main() {
let mut rw = RenderWindow::default();
rw.set_view(&rw.default_view().to_owned());
}
}
mod issue_8759_variant {
#![allow(dead_code)]
#[derive(Clone, Default)]
struct View {}
#[derive(Default)]
struct RenderWindow {
default_view: View,
}
impl RenderWindow {
fn default_view(&self) -> &View {
&self.default_view
}
fn set_view(&mut self, _view: &View) {}
}
fn main() {
let mut rw = RenderWindow::default();
rw.set_view(&rw.default_view().to_owned());
}
}

61
tests/ui/unnecessary_to_owned.rs
View File

@ -78,10 +78,10 @@ fn main() {
require_slice(&array.to_owned());
require_slice(&array_ref.to_owned());
require_slice(&slice.to_owned());
require_slice(&x_ref.to_owned());
require_slice(&x_ref.to_owned()); // No longer flagged because of #8759.
require_x(&Cow::<X>::Owned(x.clone()).into_owned());
require_x(&x_ref.to_owned());
require_x(&x_ref.to_owned()); // No longer flagged because of #8759.
require_deref_c_str(c_str.to_owned());
require_deref_os_str(os_str.to_owned());
@ -152,6 +152,7 @@ fn main() {
require_os_str(&OsString::from("x").to_os_string());
require_path(&std::path::PathBuf::from("x").to_path_buf());
require_str(&String::from("x").to_string());
require_slice(&[String::from("x")].to_owned());
}
fn require_c_str(_: &CStr) {}
@ -272,3 +273,59 @@ fn test_y(y: Y) -> Box<dyn Abstracted> {
Box::new(build(y.to_string()))
}
}
// https://github.com/rust-lang/rust-clippy/issues/8759
mod issue_8759 {
#![allow(dead_code)]
#[derive(Default)]
struct View {}
impl std::borrow::ToOwned for View {
type Owned = View;
fn to_owned(&self) -> Self::Owned {
View {}
}
}
#[derive(Default)]
struct RenderWindow {
default_view: View,
}
impl RenderWindow {
fn default_view(&self) -> &View {
&self.default_view
}
fn set_view(&mut self, _view: &View) {}
}
fn main() {
let mut rw = RenderWindow::default();
rw.set_view(&rw.default_view().to_owned());
}
}
mod issue_8759_variant {
#![allow(dead_code)]
#[derive(Clone, Default)]
struct View {}
#[derive(Default)]
struct RenderWindow {
default_view: View,
}
impl RenderWindow {
fn default_view(&self) -> &View {
&self.default_view
}
fn set_view(&mut self, _view: &View) {}
}
fn main() {
let mut rw = RenderWindow::default();
rw.set_view(&rw.default_view().to_owned());
}
}

34
tests/ui/unnecessary_to_owned.stderr
View File

@ -47,6 +47,18 @@ note: this value is dropped without further use
LL | require_str(&String::from("x").to_string());
| ^^^^^^^^^^^^^^^^^
error: redundant clone
--> $DIR/unnecessary_to_owned.rs:155:39
|
LL | require_slice(&[String::from("x")].to_owned());
| ^^^^^^^^^^^ help: remove this
|
note: this value is dropped without further use
--> $DIR/unnecessary_to_owned.rs:155:20
|
LL | require_slice(&[String::from("x")].to_owned());
| ^^^^^^^^^^^^^^^^^^^
error: unnecessary use of `into_owned`
--> $DIR/unnecessary_to_owned.rs:60:36
|
@ -151,24 +163,12 @@ error: unnecessary use of `to_owned`
LL | require_slice(&slice.to_owned());
| ^^^^^^^^^^^^^^^^^ help: use: `slice`
error: unnecessary use of `to_owned`
--> $DIR/unnecessary_to_owned.rs:81:19
|
LL | require_slice(&x_ref.to_owned());
| ^^^^^^^^^^^^^^^^^ help: use: `x_ref`
error: unnecessary use of `into_owned`
--> $DIR/unnecessary_to_owned.rs:83:42
|
LL | require_x(&Cow::<X>::Owned(x.clone()).into_owned());
| ^^^^^^^^^^^^^ help: remove this
error: unnecessary use of `to_owned`
--> $DIR/unnecessary_to_owned.rs:84:15
|
LL | require_x(&x_ref.to_owned());
| ^^^^^^^^^^^^^^^^^ help: use: `x_ref`
error: unnecessary use of `to_owned`
--> $DIR/unnecessary_to_owned.rs:86:25
|
@ -476,7 +476,7 @@ LL | let _ = IntoIterator::into_iter([std::path::PathBuf::new()][..].to_owne
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[std::path::PathBuf::new()][..].iter().cloned()`
error: unnecessary use of `to_vec`
--> $DIR/unnecessary_to_owned.rs:197:14
--> $DIR/unnecessary_to_owned.rs:198:14
|
LL | for t in file_types.to_vec() {
| ^^^^^^^^^^^^^^^^^^^
@ -492,22 +492,22 @@ LL + let path = match get_file_path(t) {
|
error: unnecessary use of `to_vec`
--> $DIR/unnecessary_to_owned.rs:220:14
--> $DIR/unnecessary_to_owned.rs:221:14
|
LL | let _ = &["x"][..].to_vec().into_iter();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `["x"][..].iter().cloned()`
error: unnecessary use of `to_vec`
--> $DIR/unnecessary_to_owned.rs:225:14
--> $DIR/unnecessary_to_owned.rs:226:14
|
LL | let _ = &["x"][..].to_vec().into_iter();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `["x"][..].iter().copied()`
error: unnecessary use of `to_string`
--> $DIR/unnecessary_to_owned.rs:272:24
--> $DIR/unnecessary_to_owned.rs:273:24
|
LL | Box::new(build(y.to_string()))
| ^^^^^^^^^^^^^ help: use: `y`
error: aborting due to 79 previous errors
error: aborting due to 78 previous errors