From: Josh Mcguigan <joshmcg88@gmail.com>
Date: Fri, 12 Oct 2018 11:48:54 +0000 (-0700)
Subject: cmp_owned refactor
X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=352863065cb644a4f59fa5655601960e34bf77e7;p=rust.git

cmp_owned refactor
---

diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs
index 0a65953313e..1cf7345e8df 100644
--- a/clippy_lints/src/misc.rs
+++ b/clippy_lints/src/misc.rs
@@ -21,7 +21,7 @@
             iter_input_pats, last_path_segment, match_qpath, match_trait_method, paths, snippet, span_lint,
             span_lint_and_then, walk_ptrs_ty, SpanlessEq};
 use crate::utils::sugg::Sugg;
-use crate::syntax::ast::{LitKind, CRATE_NODE_ID};
+use crate::syntax::ast::LitKind;
 use crate::consts::{constant, Constant};
 use crate::rustc_errors::Applicability;
 
@@ -540,18 +540,10 @@ fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr, other: &Expr) {
         _ => false,
     };
 
-    let (lint_span, try_hint) = if deref_arg_impl_partial_eq_other {
-        // suggest deref on the left
-        (expr.span, format!("*{}", snip))
-    } else if other_gets_derefed {
-        // suggest dropping the to_owned on the left and the deref on the right
-        let other_snippet = snippet(cx, other.span, "..").into_owned();
-        let other_without_deref = other_snippet.replacen('*', "", 1);
-
-        (expr.span.to(other.span), format!("{} == {}", snip.to_string(), other_without_deref))
+    let lint_span = if other_gets_derefed {
+        expr.span.to(other.span)
     } else {
-        // suggest dropping the to_owned on the left
-        (expr.span, snip.to_string())
+        expr.span
     };
 
     span_lint_and_then(
@@ -560,29 +552,20 @@ fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr, other: &Expr) {
         lint_span,
         "this creates an owned instance just for comparison",
         |db| {
-            // this is as good as our recursion check can get, we can't prove that the
-            // current function is
-            // called by
-            // PartialEq::eq, but we can at least ensure that this code is not part of it
-            let parent_fn = cx.tcx.hir.get_parent(expr.id);
-            let parent_impl = cx.tcx.hir.get_parent(parent_fn);
-            if parent_impl != CRATE_NODE_ID {
-                if let Node::Item(item) = cx.tcx.hir.get(parent_impl) {
-                    if let ItemKind::Impl(.., Some(ref trait_ref), _, _) = item.node {
-                        if trait_ref.path.def.def_id() == partial_eq_trait_id {
-                            // we are implementing PartialEq, don't suggest not doing `to_owned`, otherwise
-                            // we go into
-                            // recursion
-                            db.span_label(lint_span, "try implementing the comparison without allocating");
-                            return;
-                        }
-                    }
-                }
-            }
+            // this also catches PartialEq implementations that call to_owned
             if other_gets_derefed {
                 db.span_label(lint_span, "try implementing the comparison without allocating");
                 return;
             }
+
+            let try_hint = if deref_arg_impl_partial_eq_other {
+                // suggest deref on the left
+                format!("*{}", snip)
+            } else {
+                // suggest dropping the to_owned on the left
+                snip.to_string()
+            };
+
             db.span_suggestion_with_applicability(
                 lint_span,
                 "try",