From 9a0b598b73d93cf38905550dd9bd12e133e900aa Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Sat, 24 Aug 2019 09:23:06 +0200 Subject: [PATCH 1/2] Split up cmp_owned tests, add run-rustfix Some of the cmp_owned tests emitted non-machine-applicable suggestions, so I moved them to `tests/ui/cmp_owned/without_suggestion.rs` and added `// run-rustfix` to the other half. cc #3630 --- tests/ui/cmp_owned/with_suggestion.fixed | 72 +++++++++++++++++++ .../with_suggestion.rs} | 18 ++--- .../with_suggestion.stderr} | 32 ++------- tests/ui/cmp_owned/without_suggestion.rs | 52 ++++++++++++++ tests/ui/cmp_owned/without_suggestion.stderr | 22 ++++++ 5 files changed, 160 insertions(+), 36 deletions(-) create mode 100644 tests/ui/cmp_owned/with_suggestion.fixed rename tests/ui/{cmp_owned.rs => cmp_owned/with_suggestion.rs} (77%) rename tests/ui/{cmp_owned.stderr => cmp_owned/with_suggestion.stderr} (54%) create mode 100644 tests/ui/cmp_owned/without_suggestion.rs create mode 100644 tests/ui/cmp_owned/without_suggestion.stderr diff --git a/tests/ui/cmp_owned/with_suggestion.fixed b/tests/ui/cmp_owned/with_suggestion.fixed new file mode 100644 index 00000000000..0323b2905bf --- /dev/null +++ b/tests/ui/cmp_owned/with_suggestion.fixed @@ -0,0 +1,72 @@ +// run-rustfix + +#[warn(clippy::cmp_owned)] +#[allow(clippy::unnecessary_operation, clippy::no_effect, unused_must_use, clippy::eq_op)] +fn main() { + fn with_to_string(x: &str) { + x != "foo"; + + "foo" != x; + } + + let x = "oh"; + + with_to_string(x); + + x != "foo"; + + x != "foo"; + + 42.to_string() == "42"; + + Foo == Foo; + + "abc".chars().filter(|c| *c != 'X'); + + "abc".chars().filter(|c| *c != 'X'); +} + +struct Foo; + +impl PartialEq for Foo { + // Allow this here, because it emits the lint + // without a suggestion. This is tested in + // `tests/ui/cmp_owned_without_suggestion.rs` + #[allow(clippy::cmp_owned)] + fn eq(&self, other: &Self) -> bool { + self.to_owned() == *other + } +} + +impl ToOwned for Foo { + type Owned = Bar; + fn to_owned(&self) -> Bar { + Bar + } +} + +#[derive(PartialEq)] +struct Bar; + +impl PartialEq for Bar { + fn eq(&self, _: &Foo) -> bool { + true + } +} + +impl std::borrow::Borrow for Bar { + fn borrow(&self) -> &Foo { + static FOO: Foo = Foo; + &FOO + } +} + +#[derive(PartialEq)] +struct Baz; + +impl ToOwned for Baz { + type Owned = Baz; + fn to_owned(&self) -> Baz { + Baz + } +} diff --git a/tests/ui/cmp_owned.rs b/tests/ui/cmp_owned/with_suggestion.rs similarity index 77% rename from tests/ui/cmp_owned.rs rename to tests/ui/cmp_owned/with_suggestion.rs index a5f92b30bc2..ecfdffc8c97 100644 --- a/tests/ui/cmp_owned.rs +++ b/tests/ui/cmp_owned/with_suggestion.rs @@ -1,5 +1,7 @@ +// run-rustfix + #[warn(clippy::cmp_owned)] -#[allow(clippy::unnecessary_operation)] +#[allow(clippy::unnecessary_operation, clippy::no_effect, unused_must_use, clippy::eq_op)] fn main() { fn with_to_string(x: &str) { x != "foo".to_string(); @@ -22,21 +24,15 @@ fn main() { "abc".chars().filter(|c| c.to_owned() != 'X'); "abc".chars().filter(|c| *c != 'X'); - - let x = &Baz; - let y = &Baz; - - y.to_owned() == *x; - - let x = &&Baz; - let y = &Baz; - - y.to_owned() == **x; } struct Foo; impl PartialEq for Foo { + // Allow this here, because it emits the lint + // without a suggestion. This is tested in + // `tests/ui/cmp_owned_without_suggestion.rs` + #[allow(clippy::cmp_owned)] fn eq(&self, other: &Self) -> bool { self.to_owned() == *other } diff --git a/tests/ui/cmp_owned.stderr b/tests/ui/cmp_owned/with_suggestion.stderr similarity index 54% rename from tests/ui/cmp_owned.stderr rename to tests/ui/cmp_owned/with_suggestion.stderr index 9be749f8d04..2f333e6ea8e 100644 --- a/tests/ui/cmp_owned.stderr +++ b/tests/ui/cmp_owned/with_suggestion.stderr @@ -1,5 +1,5 @@ error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:5:14 + --> $DIR/with_suggestion.rs:7:14 | LL | x != "foo".to_string(); | ^^^^^^^^^^^^^^^^^ help: try: `"foo"` @@ -7,52 +7,34 @@ LL | x != "foo".to_string(); = note: `-D clippy::cmp-owned` implied by `-D warnings` error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:7:9 + --> $DIR/with_suggestion.rs:9:9 | LL | "foo".to_string() != x; | ^^^^^^^^^^^^^^^^^ help: try: `"foo"` error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:14:10 + --> $DIR/with_suggestion.rs:16:10 | LL | x != "foo".to_owned(); | ^^^^^^^^^^^^^^^^ help: try: `"foo"` error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:16:10 + --> $DIR/with_suggestion.rs:18:10 | LL | x != String::from("foo"); | ^^^^^^^^^^^^^^^^^^^ help: try: `"foo"` error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:20:5 + --> $DIR/with_suggestion.rs:22:5 | LL | Foo.to_owned() == Foo; | ^^^^^^^^^^^^^^ help: try: `Foo` error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:22:30 + --> $DIR/with_suggestion.rs:24:30 | LL | "abc".chars().filter(|c| c.to_owned() != 'X'); | ^^^^^^^^^^^^ help: try: `*c` -error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:29:5 - | -LL | y.to_owned() == *x; - | ^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating - -error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:34:5 - | -LL | y.to_owned() == **x; - | ^^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating - -error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:41:9 - | -LL | self.to_owned() == *other - | ^^^^^^^^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating - -error: aborting due to 9 previous errors +error: aborting due to 6 previous errors diff --git a/tests/ui/cmp_owned/without_suggestion.rs b/tests/ui/cmp_owned/without_suggestion.rs new file mode 100644 index 00000000000..9ab8795474c --- /dev/null +++ b/tests/ui/cmp_owned/without_suggestion.rs @@ -0,0 +1,52 @@ +#[allow(clippy::unnecessary_operation)] + +fn main() { + let x = &Baz; + let y = &Baz; + y.to_owned() == *x; + + let x = &&Baz; + let y = &Baz; + y.to_owned() == **x; +} + +struct Foo; + +impl PartialEq for Foo { + fn eq(&self, other: &Self) -> bool { + self.to_owned() == *other + } +} + +impl ToOwned for Foo { + type Owned = Bar; + fn to_owned(&self) -> Bar { + Bar + } +} + +#[derive(PartialEq)] +struct Baz; + +impl ToOwned for Baz { + type Owned = Baz; + fn to_owned(&self) -> Baz { + Baz + } +} + +#[derive(PartialEq)] +struct Bar; + +impl PartialEq for Bar { + fn eq(&self, _: &Foo) -> bool { + true + } +} + +impl std::borrow::Borrow for Bar { + fn borrow(&self) -> &Foo { + static FOO: Foo = Foo; + &FOO + } +} diff --git a/tests/ui/cmp_owned/without_suggestion.stderr b/tests/ui/cmp_owned/without_suggestion.stderr new file mode 100644 index 00000000000..6e8a5ad2a17 --- /dev/null +++ b/tests/ui/cmp_owned/without_suggestion.stderr @@ -0,0 +1,22 @@ +error: this creates an owned instance just for comparison + --> $DIR/without_suggestion.rs:6:5 + | +LL | y.to_owned() == *x; + | ^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating + | + = note: `-D clippy::cmp-owned` implied by `-D warnings` + +error: this creates an owned instance just for comparison + --> $DIR/without_suggestion.rs:10:5 + | +LL | y.to_owned() == **x; + | ^^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating + +error: this creates an owned instance just for comparison + --> $DIR/without_suggestion.rs:17:9 + | +LL | self.to_owned() == *other + | ^^^^^^^^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating + +error: aborting due to 3 previous errors + From 6d425a60a7e7d272a17449d85d7d590db1b2ce5e Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Mon, 26 Aug 2019 15:28:35 +0200 Subject: [PATCH 2/2] Use correct path in comment Co-Authored-By: Philipp Krones --- tests/ui/cmp_owned/with_suggestion.fixed | 2 +- tests/ui/cmp_owned/with_suggestion.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ui/cmp_owned/with_suggestion.fixed b/tests/ui/cmp_owned/with_suggestion.fixed index 0323b2905bf..05fb96339e3 100644 --- a/tests/ui/cmp_owned/with_suggestion.fixed +++ b/tests/ui/cmp_owned/with_suggestion.fixed @@ -31,7 +31,7 @@ struct Foo; impl PartialEq for Foo { // Allow this here, because it emits the lint // without a suggestion. This is tested in - // `tests/ui/cmp_owned_without_suggestion.rs` + // `tests/ui/cmp_owned/without_suggestion.rs` #[allow(clippy::cmp_owned)] fn eq(&self, other: &Self) -> bool { self.to_owned() == *other diff --git a/tests/ui/cmp_owned/with_suggestion.rs b/tests/ui/cmp_owned/with_suggestion.rs index ecfdffc8c97..0a02825ed82 100644 --- a/tests/ui/cmp_owned/with_suggestion.rs +++ b/tests/ui/cmp_owned/with_suggestion.rs @@ -31,7 +31,7 @@ struct Foo; impl PartialEq for Foo { // Allow this here, because it emits the lint // without a suggestion. This is tested in - // `tests/ui/cmp_owned_without_suggestion.rs` + // `tests/ui/cmp_owned/without_suggestion.rs` #[allow(clippy::cmp_owned)] fn eq(&self, other: &Self) -> bool { self.to_owned() == *other