use other instead of self

Centri3 · Centri3 · commit c5c1a77001f8 · 2023-06-15T08:44:32.000-05:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4833,6 +4833,7 @@ Released 2018-09-13
 [`imprecise_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#imprecise_flops
 [`inconsistent_digit_grouping`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_digit_grouping
 [`inconsistent_struct_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_struct_constructor
+[`incorrect_partial_ord_impl_on_ord_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#incorrect_partial_ord_impl_on_ord_type
 [`index_refutable_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#index_refutable_slice
 [`indexing_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing
 [`ineffective_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_bit_mask
@@ -5017,7 +5018,6 @@ Released 2018-09-13
 [`needless_option_as_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_option_as_deref
 [`needless_option_take`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_option_take
 [`needless_parens_on_range_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_parens_on_range_literals
-[`needless_partial_ord_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_partial_ord_impl
 [`needless_pass_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value
 [`needless_question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_question_mark
 [`needless_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
@@ -206,6 +206,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::implicit_saturating_add::IMPLICIT_SATURATING_ADD_INFO,
     crate::implicit_saturating_sub::IMPLICIT_SATURATING_SUB_INFO,
     crate::inconsistent_struct_constructor::INCONSISTENT_STRUCT_CONSTRUCTOR_INFO,
+    crate::incorrect_impls::INCORRECT_PARTIAL_ORD_IMPL_ON_ORD_TYPE_INFO,
     crate::index_refutable_slice::INDEX_REFUTABLE_SLICE_INFO,
     crate::indexing_slicing::INDEXING_SLICING_INFO,
     crate::indexing_slicing::OUT_OF_BOUNDS_INDEXING_INFO,
@@ -463,7 +464,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::needless_else::NEEDLESS_ELSE_INFO,
     crate::needless_for_each::NEEDLESS_FOR_EACH_INFO,
     crate::needless_if::NEEDLESS_IF_INFO,
-    crate::needless_impls::NEEDLESS_PARTIAL_ORD_IMPL_INFO,
     crate::needless_late_init::NEEDLESS_LATE_INIT_INFO,
     crate::needless_parens_on_range_literals::NEEDLESS_PARENS_ON_RANGE_LITERALS_INFO,
     crate::needless_pass_by_value::NEEDLESS_PASS_BY_VALUE_INFO,
diff --git a/clippy_lints/src/incorrect_impls.rs b/clippy_lints/src/incorrect_impls.rs
@@ -55,13 +55,13 @@ declare_clippy_lint! {
     /// }
     /// ```
     #[clippy::version = "1.72.0"]
-    pub NEEDLESS_PARTIAL_ORD_IMPL,
+    pub INCORRECT_PARTIAL_ORD_IMPL_ON_ORD_TYPE,
     correctness,
     "manual implementation of `PartialOrd` when `Ord` is already implemented"
 }
-declare_lint_pass!(NeedlessImpls => [NEEDLESS_PARTIAL_ORD_IMPL]);
+declare_lint_pass!(IncorrectImpls => [INCORRECT_PARTIAL_ORD_IMPL_ON_ORD_TYPE]);
 
-impl LateLintPass<'_> for NeedlessImpls {
+impl LateLintPass<'_> for IncorrectImpls {
     fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) {
         let node = get_parent_node(cx.tcx, impl_item.hir_id());
         let Some(Node::Item(item)) = node else {
@@ -114,13 +114,22 @@ impl LateLintPass<'_> for NeedlessImpls {
                 && cmp_path.ident.name == sym::cmp
                 && let Res::Local(..) = path_res(cx, other_expr)
             {} else {
+                // If lhs and rhs are not the same type, bail. This makes creating a valid
+                // suggestion tons more complex.
+                if let Some(lhs) = trait_impl.substs.get(0)
+                    && let Some(rhs) = trait_impl.substs.get(1)
+                    && lhs != rhs
+                {
+                    return;
+                }
+
                 span_lint_and_then(
                     cx,
-                    NEEDLESS_PARTIAL_ORD_IMPL,
+                    INCORRECT_PARTIAL_ORD_IMPL_ON_ORD_TYPE,
                     item.span,
-                    "manual implementation of `PartialOrd` when `Ord` is already implemented",
+                    "incorrect implementation of `partial_cmp` on an `Ord` type",
                     |diag| {
-                        let (help, app) = if let Some(other) = body.params.get(0)
+                        let (help, app) = if let Some(other) = body.params.get(1)
                             && let PatKind::Binding(_, _, other_ident, ..) = other.pat.kind
                         {
                             (
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -150,6 +150,7 @@ mod implicit_return;
 mod implicit_saturating_add;
 mod implicit_saturating_sub;
 mod inconsistent_struct_constructor;
+mod incorrect_impls;
 mod index_refutable_slice;
 mod indexing_slicing;
 mod infinite_iter;
@@ -226,7 +227,6 @@ mod needless_continue;
 mod needless_else;
 mod needless_for_each;
 mod needless_if;
-mod needless_impls;
 mod needless_late_init;
 mod needless_parens_on_range_literals;
 mod needless_pass_by_value;
@@ -1048,6 +1048,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     let stack_size_threshold = conf.stack_size_threshold;
     store.register_late_pass(move |_| Box::new(large_stack_frames::LargeStackFrames::new(stack_size_threshold)));
     store.register_late_pass(|_| Box::new(single_range_in_vec_init::SingleRangeInVecInit));
+    store.register_late_pass(|_| Box::new(incorrect_impls::IncorrectImpls));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 
diff --git a/tests/ui/bool_comparison.fixed b/tests/ui/bool_comparison.fixed
@@ -2,7 +2,7 @@
 
 #![allow(clippy::needless_if)]
 #![warn(clippy::bool_comparison)]
-#![allow(clippy::needless_partial_ord_impl)]
+#![allow(clippy::incorrect_partial_ord_impl_on_ord_type)]
 
 fn main() {
     let x = true;
diff --git a/tests/ui/bool_comparison.rs b/tests/ui/bool_comparison.rs
@@ -2,7 +2,7 @@
 
 #![allow(clippy::needless_if)]
 #![warn(clippy::bool_comparison)]
-#![allow(clippy::needless_partial_ord_impl)]
+#![allow(clippy::incorrect_partial_ord_impl_on_ord_type)]
 
 fn main() {
     let x = true;
diff --git a/tests/ui/derive.rs b/tests/ui/derive.rs
@@ -1,4 +1,4 @@
-#![allow(clippy::needless_partial_ord_impl, dead_code)]
+#![allow(clippy::incorrect_partial_ord_impl_on_ord_type, dead_code)]
 #![warn(clippy::expl_impl_clone_on_copy)]
 
 #[derive(Copy)]
diff --git a/tests/ui/derive_ord_xor_partial_ord.rs b/tests/ui/derive_ord_xor_partial_ord.rs
@@ -1,6 +1,6 @@
 #![warn(clippy::derive_ord_xor_partial_ord)]
 #![allow(clippy::unnecessary_wraps)]
-#![allow(clippy::needless_partial_ord_impl)]
+#![allow(clippy::incorrect_partial_ord_impl_on_ord_type)]
 
 use std::cmp::Ordering;
 
diff --git a/tests/ui/incorrect_partial_ord_impl_on_ord_type.rs b/tests/ui/incorrect_partial_ord_impl_on_ord_type.rs
@@ -1,4 +1,3 @@
-//@run-rustfix
 #![allow(unused)]
 #![no_main]
 
@@ -51,7 +50,7 @@ impl Ord for C {
 
 impl PartialOrd for C {
     fn partial_cmp(&self, _: &Self) -> Option<Ordering> {
-        todo!();
+        todo!(); // don't run rustfix, or else this will cause it to fail to compile
     }
 }
 
@@ -87,3 +86,32 @@ impl<A: Ord + PartialOrd> PartialOrd for Uwu<A> {
         todo!();
     }
 }
+
+// do not lint since `Rhs` is not `Self`
+
+#[derive(Eq, PartialEq)]
+struct F(u32);
+
+impl Ord for F {
+    fn cmp(&self, other: &Self) -> Ordering {
+        todo!();
+    }
+}
+
+impl PartialOrd for F {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl PartialEq<u32> for F {
+    fn eq(&self, other: &u32) -> bool {
+        todo!();
+    }
+}
+
+impl PartialOrd<u32> for F {
+    fn partial_cmp(&self, other: &u32) -> Option<Ordering> {
+        todo!();
+    }
+}
diff --git a/tests/ui/incorrect_partial_ord_impl_on_ord_type.stderr b/tests/ui/incorrect_partial_ord_impl_on_ord_type.stderr
@@ -0,0 +1,28 @@
+error: incorrect implementation of `partial_cmp` on an `Ord` type
+  --> $DIR/incorrect_partial_ord_impl_on_ord_type.rs:17:1
+   |
+LL | /  impl PartialOrd for A {
+LL | |      fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+   | | _____________________________________________________________-
+LL | ||         todo!();
+LL | ||     }
+   | ||_____- help: change this to: `{ Some(self.cmp(other)) }`
+LL | |  }
+   | |__^
+   |
+   = note: `#[deny(clippy::incorrect_partial_ord_impl_on_ord_type)]` on by default
+
+error: incorrect implementation of `partial_cmp` on an `Ord` type
+  --> $DIR/incorrect_partial_ord_impl_on_ord_type.rs:51:1
+   |
+LL | /  impl PartialOrd for C {
+LL | |      fn partial_cmp(&self, _: &Self) -> Option<Ordering> {
+   | | _________________________________________________________-
+LL | ||         todo!(); // don't run rustfix, or else this will cause it to fail to compile
+LL | ||     }
+   | ||_____- help: change this to: `{ Some(self.cmp(...)) }`
+LL | |  }
+   | |__^
+
+error: aborting due to 2 previous errors
+
diff --git a/tests/ui/needless_partial_ord_impl.fixed b/tests/ui/needless_partial_ord_impl.fixed
diff --git a/tests/ui/needless_partial_ord_impl.stderr b/tests/ui/needless_partial_ord_impl.stderr

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-#![allow(clippy::needless_partial_ord_impl, dead_code)]`
	`1`	`+#![allow(clippy::incorrect_partial_ord_impl_on_ord_type, dead_code)]`
`2`	`2`	`#![warn(clippy::expl_impl_clone_on_copy)]`
`3`	`3`
`4`	`4`	`#[derive(Copy)]`