Skip to content

Commit

Permalink
Auto merge of #8299 - marekdownar:8214, r=Manishearth
Browse files Browse the repository at this point in the history
#8214 cmp_owned suggestion flips the comparison

changelog: ``[`cmp_owned`]`` fixes #8214 so that the suggestion does not flip the comparison
  • Loading branch information
bors committed Jan 17, 2022
2 parents 537a7f3 + 5b6ec8c commit d364d8a
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 2 deletions.
21 changes: 19 additions & 2 deletions clippy_lints/src/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
);
}
}

Expand Down
29 changes: 29 additions & 0 deletions tests/ui/cmp_owned/comparison_flip.fixed
Original file line number Diff line number Diff line change
@@ -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
}
}
29 changes: 29 additions & 0 deletions tests/ui/cmp_owned/comparison_flip.rs
Original file line number Diff line number Diff line change
@@ -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
}
}
18 changes: 18 additions & 0 deletions tests/ui/cmp_owned/comparison_flip.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit d364d8a

Please sign in to comment.