From ad5c29a445692c0c074ee1f019fe1ff2056da23f Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Mon, 8 Oct 2018 19:04:29 -0700 Subject: [PATCH] Fixes #2925 cmp_owned false positive --- clippy_lints/src/misc.rs | 26 ++++++++++++++------------ tests/ui/cmp_owned.rs | 4 ++++ tests/ui/cmp_owned.stderr | 12 +++++++++--- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index a83fa75de69..4e6d5b72efc 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -334,11 +334,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { |db| { let sugg = if binop.node == BinOpKind::Or { !sugg } else { sugg }; db.span_suggestion_with_applicability( - s.span, + s.span, "replace it with", format!( "if {} {{ {}; }}", - sugg, + sugg, &snippet(cx, b.span, ".."), ), Applicability::MachineApplicable, // snippet @@ -520,16 +520,17 @@ fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr, other: &Expr) { None => return, }; - // *arg impls PartialEq - if !arg_ty + let deref_arg_impl_partial_eq_other = arg_ty .builtin_deref(true) - .map_or(false, |tam| implements_trait(cx, tam.ty, partial_eq_trait_id, &[other_ty.into()])) - // arg impls PartialEq<*other> - && !other_ty + .map_or(false, |tam| implements_trait(cx, tam.ty, partial_eq_trait_id, &[other_ty.into()])); + let arg_impl_partial_eq_deref_other = other_ty .builtin_deref(true) - .map_or(false, |tam| implements_trait(cx, arg_ty, partial_eq_trait_id, &[tam.ty.into()])) - // arg impls PartialEq - && !implements_trait(cx, arg_ty, partial_eq_trait_id, &[other_ty.into()]) + .map_or(false, |tam| implements_trait(cx, arg_ty, partial_eq_trait_id, &[tam.ty.into()])); + let arg_impl_partial_eq_other = implements_trait(cx, arg_ty, partial_eq_trait_id, &[other_ty.into()]); + + if !deref_arg_impl_partial_eq_other + && !arg_impl_partial_eq_deref_other + && !arg_impl_partial_eq_other { return; } @@ -559,10 +560,11 @@ fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr, other: &Expr) { } } } + let try_hint = if deref_arg_impl_partial_eq_other { format!("*{}", snip) } else { snip.to_string() }; db.span_suggestion_with_applicability( - expr.span, + expr.span, "try", - snip.to_string(), + try_hint, Applicability::MachineApplicable, // snippet ); }, diff --git a/tests/ui/cmp_owned.rs b/tests/ui/cmp_owned.rs index e937afc1a81..031809f5df5 100644 --- a/tests/ui/cmp_owned.rs +++ b/tests/ui/cmp_owned.rs @@ -31,6 +31,10 @@ fn main() { 42.to_string() == "42"; Foo.to_owned() == Foo; + + "abc".chars().filter(|c| c.to_owned() != 'X'); + + "abc".chars().filter(|c| *c != 'X'); } struct Foo; diff --git a/tests/ui/cmp_owned.stderr b/tests/ui/cmp_owned.stderr index 020ffe805cd..5434b68de9f 100644 --- a/tests/ui/cmp_owned.stderr +++ b/tests/ui/cmp_owned.stderr @@ -31,10 +31,16 @@ error: this creates an owned instance just for comparison | ^^^^^^^^^^^^^^ help: try: `Foo` error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:40:9 + --> $DIR/cmp_owned.rs:35:30 | -40 | self.to_owned() == *other +35 | "abc".chars().filter(|c| c.to_owned() != 'X'); + | ^^^^^^^^^^^^ help: try: `*c` + +error: this creates an owned instance just for comparison + --> $DIR/cmp_owned.rs:44:9 + | +44 | self.to_owned() == *other | ^^^^^^^^^^^^^^^ try calling implementing the comparison without allocating -error: aborting due to 6 previous errors +error: aborting due to 7 previous errors