Auto merge of #8807 - Jarcho:cmp_owned, r=giraffate

Fix `cmp_owned` on copy types

fixes #8803
fixes #7365

changelog: Don't lint `cmp_owned` on `From::from` for copy types

Author: bors

clippy_lints/src/misc.rs

@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then, span_lint_hir_and_then};
 use clippy_utils::source::{snippet, snippet_opt};
-use clippy_utils::ty::implements_trait;
+use clippy_utils::ty::{implements_trait, is_copy};
 use if_chain::if_chain;
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
@@ -20,8 +20,8 @@ use rustc_span::symbol::sym;
 use clippy_utils::consts::{constant, Constant};
 use clippy_utils::sugg::Sugg;
 use clippy_utils::{
-    get_item_name, get_parent_expr, in_constant, is_diag_trait_item, is_integer_const, iter_input_pats,
-    last_path_segment, match_any_def_paths, path_def_id, paths, unsext, SpanlessEq,
+    get_item_name, get_parent_expr, in_constant, is_integer_const, iter_input_pats, last_path_segment,
+    match_any_def_paths, path_def_id, paths, unsext, SpanlessEq,
 };
 
 declare_clippy_lint! {
@@ -569,33 +569,34 @@ fn check_to_owned(cx: &LateContext<'_>, expr: &Expr<'_>, other: &Expr<'_>, left:
         })
     }
 
-    let (arg_ty, snip) = match expr.kind {
-        ExprKind::MethodCall(.., args, _) if args.len() == 1 => {
-            if_chain!(
-                if let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
-                if is_diag_trait_item(cx, expr_def_id, sym::ToString)
-                    || is_diag_trait_item(cx, expr_def_id, sym::ToOwned);
-                then {
-                    (cx.typeck_results().expr_ty(&args[0]), snippet(cx, args[0].span, ".."))
-                } else {
-                    return;
-                }
-            )
+    let typeck = cx.typeck_results();
+    let (arg, arg_span) = match expr.kind {
+        ExprKind::MethodCall(.., [arg], _)
+            if typeck
+                .type_dependent_def_id(expr.hir_id)
+                .and_then(|id| cx.tcx.trait_of_item(id))
+                .map_or(false, |id| {
+                    matches!(cx.tcx.get_diagnostic_name(id), Some(sym::ToString | sym::ToOwned))
+                }) =>
+        {
+            (arg, arg.span)
         },
-        ExprKind::Call(path, [arg]) => {
+        ExprKind::Call(path, [arg])
             if path_def_id(cx, path)
                 .and_then(|id| match_any_def_paths(cx, id, &[&paths::FROM_STR_METHOD, &paths::FROM_FROM]))
-                .is_some()
-            {
-                (cx.typeck_results().expr_ty(arg), snippet(cx, arg.span, ".."))
-            } else {
-                return;
-            }
+                .map_or(false, |idx| match idx {
+                    0 => true,
+                    1 => !is_copy(cx, typeck.expr_ty(expr)),
+                    _ => false,
+                }) =>
+        {
+            (arg, arg.span)
         },
         _ => return,
     };
 
-    let other_ty = cx.typeck_results().expr_ty(other);
+    let arg_ty = typeck.expr_ty(arg);
+    let other_ty = typeck.expr_ty(other);
 
     let without_deref = symmetric_partial_eq(cx, arg_ty, other_ty).unwrap_or_default();
     let with_deref = arg_ty
@@ -627,13 +628,14 @@ fn check_to_owned(cx: &LateContext<'_>, expr: &Expr<'_>, other: &Expr<'_>, left:
                 return;
             }
 
+            let arg_snip = snippet(cx, arg_span, "..");
             let expr_snip;
             let eq_impl;
             if with_deref.is_implemented() {
-                expr_snip = format!("*{}", snip);
+                expr_snip = format!("*{}", arg_snip);
                 eq_impl = with_deref;
             } else {
-                expr_snip = snip.to_string();
+                expr_snip = arg_snip.to_string();
                 eq_impl = without_deref;
             };
 

tests/ui/cmp_owned/without_suggestion.rs

@@ -9,6 +9,10 @@ fn main() {
     let x = &&Baz;
     let y = &Baz;
     y.to_owned() == **x;
+
+    let x = 0u32;
+    let y = U32Wrapper(x);
+    let _ = U32Wrapper::from(x) == y;
 }
 
 struct Foo;
@@ -51,3 +55,21 @@ impl std::borrow::Borrow<Foo> for Bar {
         &FOO
     }
 }
+
+#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+struct U32Wrapper(u32);
+impl From<u32> for U32Wrapper {
+    fn from(x: u32) -> Self {
+        Self(x)
+    }
+}
+impl PartialEq<u32> for U32Wrapper {
+    fn eq(&self, other: &u32) -> bool {
+        self.0 == *other
+    }
+}
+impl PartialEq<U32Wrapper> for u32 {
+    fn eq(&self, other: &U32Wrapper) -> bool {
+        *self == other.0
+    }
+}

tests/ui/cmp_owned/without_suggestion.stderr

@@ -13,7 +13,7 @@ LL |     y.to_owned() == **x;
    |     ^^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating
 
 error: this creates an owned instance just for comparison
-  --> $DIR/without_suggestion.rs:18:9
+  --> $DIR/without_suggestion.rs:22:9
    |
 LL |         self.to_owned() == *other
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating