#8214 cmp_owned suggestion flips the comparison

4 files changed, 95 insertions, 2 deletions

diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs
index 21b3f81d5d9..8db71d1e967 100644
--- a/clippy_lints/src/misc.rs
+++ b/clippy_lints/src/misc.rs
@@ -548,6 +548,7 @@ fn is_array(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
     matches!(&cx.typeck_results().expr_ty(expr).peel_refs().kind(), ty::Array(_, _))
 }
 
+#[allow(clippy::too_many_lines)]
 fn check_to_owned(cx: &LateContext<'_>, expr: &Expr<'_>, other: &Expr<'_>, left: bool) {
     #[derive(Default)]
     struct EqImpl {
@@ -644,10 +645,26 @@ fn check_to_owned(cx: &LateContext<'_>, expr: &Expr<'_>, other: &Expr<'_>, left:
                 hint = expr_snip;
             } else {
                 span = expr.span.to(other.span);
+
+                let cmp_span = if other.span < expr.span {
+                    other.span.between(expr.span)
+                } else {
+                    expr.span.between(other.span)
+                };
                 if eq_impl.ty_eq_other {
-                    hint = format!("{} == {}", expr_snip, snippet(cx, other.span, ".."));
+                    hint = format!(
+                        "{}{}{}",
+                        expr_snip,
+                        snippet(cx, cmp_span, ".."),
+                        snippet(cx, other.span, "..")
+                    );
                 } else {
-                    hint = format!("{} == {}", snippet(cx, other.span, ".."), expr_snip);
+                    hint = format!(
+                        "{}{}{}",
+                        snippet(cx, other.span, ".."),
+                        snippet(cx, cmp_span, ".."),
+                        expr_snip
+                    );
                 }
             }
 
diff --git a/tests/ui/cmp_owned/comparison_flip.fixed b/tests/ui/cmp_owned/comparison_flip.fixed
new file mode 100644
index 00000000000..44e41bdd114
--- /dev/null
+++ b/tests/ui/cmp_owned/comparison_flip.fixed
@@ -0,0 +1,29 @@
+// run-rustfix
+
+use std::fmt::{self, Display};
+
+fn main() {
+    let a = Foo;
+
+    if a != "bar" {
+        println!("foo");
+    }
+
+    if a != "bar" {
+        println!("foo");
+    }
+}
+
+struct Foo;
+
+impl Display for Foo {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "foo")
+    }
+}
+
+impl PartialEq<&str> for Foo {
+    fn eq(&self, other: &&str) -> bool {
+        "foo" == *other
+    }
+}
diff --git a/tests/ui/cmp_owned/comparison_flip.rs b/tests/ui/cmp_owned/comparison_flip.rs
new file mode 100644
index 00000000000..662673abb62
--- /dev/null
+++ b/tests/ui/cmp_owned/comparison_flip.rs
@@ -0,0 +1,29 @@
+// run-rustfix
+
+use std::fmt::{self, Display};
+
+fn main() {
+    let a = Foo;
+
+    if a.to_string() != "bar" {
+        println!("foo");
+    }
+
+    if "bar" != a.to_string() {
+        println!("foo");
+    }
+}
+
+struct Foo;
+
+impl Display for Foo {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "foo")
+    }
+}
+
+impl PartialEq<&str> for Foo {
+    fn eq(&self, other: &&str) -> bool {
+        "foo" == *other
+    }
+}
diff --git a/tests/ui/cmp_owned/comparison_flip.stderr b/tests/ui/cmp_owned/comparison_flip.stderr
new file mode 100644
index 00000000000..e4d0d822bb1
--- /dev/null
+++ b/tests/ui/cmp_owned/comparison_flip.stderr
@@ -0,0 +1,18 @@
+error: this creates an owned instance just for comparison
+  --> $DIR/comparison_flip.rs:8:8
+   |
+LL |     if a.to_string() != "bar" {
+   |        ^^^^^^^^^^^^^ help: try: `a`
+   |
+   = note: `-D clippy::cmp-owned` implied by `-D warnings`
+
+error: this creates an owned instance just for comparison
+  --> $DIR/comparison_flip.rs:12:17
+   |
+LL |     if "bar" != a.to_string() {
+   |        ---------^^^^^^^^^^^^^
+   |        |
+   |        help: try: `a != "bar"`
+
+error: aborting due to 2 previous errors
+