heavily refactor + bump clippy::version

CHANGELOG.md

@@ -4918,7 +4918,6 @@ Released 2018-09-13
 [`manual_next_back`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_next_back
 [`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
 [`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
-[`manual_partial_ord_and_ord_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_partial_ord_and_ord_impl
 [`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
 [`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid
 [`manual_retain`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain
@@ -5014,6 +5013,7 @@ 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

clippy_lints/src/declared_lints.rs

@@ -274,7 +274,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::manual_let_else::MANUAL_LET_ELSE_INFO,
     crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO,
     crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO,
-    crate::manual_partial_ord_and_ord_impl::MANUAL_PARTIAL_ORD_AND_ORD_IMPL_INFO,
     crate::manual_rem_euclid::MANUAL_REM_EUCLID_INFO,
     crate::manual_retain::MANUAL_RETAIN_INFO,
     crate::manual_slice_size_calculation::MANUAL_SLICE_SIZE_CALCULATION_INFO,
@@ -459,6 +458,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::needless_continue::NEEDLESS_CONTINUE_INFO,
     crate::needless_else::NEEDLESS_ELSE_INFO,
     crate::needless_for_each::NEEDLESS_FOR_EACH_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,

clippy_lints/src/lib.rs

@@ -185,7 +185,6 @@ mod manual_is_ascii_check;
 mod manual_let_else;
 mod manual_main_separator_str;
 mod manual_non_exhaustive;
-mod manual_partial_ord_and_ord_impl;
 mod manual_rem_euclid;
 mod manual_retain;
 mod manual_slice_size_calculation;
@@ -223,6 +222,7 @@ mod needless_borrowed_ref;
 mod needless_continue;
 mod needless_else;
 mod needless_for_each;
+mod needless_impls;
 mod needless_late_init;
 mod needless_parens_on_range_literals;
 mod needless_pass_by_value;
@@ -1022,7 +1022,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|_| Box::new(endian_bytes::EndianBytes));
     store.register_late_pass(|_| Box::new(redundant_type_annotations::RedundantTypeAnnotations));
     store.register_late_pass(|_| Box::new(arc_with_non_send_sync::ArcWithNonSendSync));
-    store.register_late_pass(|_| Box::new(manual_partial_ord_and_ord_impl::ManualPartialOrdAndOrdImpl));
+    store.register_late_pass(|_| Box::new(needless_impls::NeedlessImpls));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/needless_impls.rs

@@ -0,0 +1,138 @@
+use clippy_utils::{diagnostics::span_lint_and_then, get_parent_node, match_qpath, path_res, ty::implements_trait};
+use rustc_errors::Applicability;
+use rustc_hir::{def::Res, Expr, ExprKind, ImplItem, ImplItemKind, ItemKind, Node, PatKind};
+use rustc_hir_analysis::hir_ty_to_ty;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::EarlyBinder;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::sym;
+use std::borrow::Cow;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for manual implementations of both `PartialOrd` and `Ord` when only `Ord` is
+    /// necessary.
+    ///
+    /// ### Why is this bad?
+    /// If both `PartialOrd` and `Ord` are implemented, they must agree. This is commonly done by
+    /// wrapping the result of `cmp` in `Some` for `partial_cmp`. Not doing this may silently
+    /// introduce an error upon refactoring.
+    ///
+    /// ### Example
+    /// ```rust,ignore
+    /// #[derive(Eq, PartialEq)]
+    /// struct A(u32);
+    ///
+    /// impl Ord for A {
+    ///     fn cmp(&self, other: &Self) -> Ordering {
+    ///         todo!();
+    ///     }
+    /// }
+    ///
+    /// impl PartialOrd for A {
+    ///     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+    ///         todo!();
+    ///     }
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust,ignore
+    /// #[derive(Eq, PartialEq)]
+    /// struct A(u32);
+    ///
+    /// impl Ord for A {
+    ///     fn cmp(&self, other: &Self) -> Ordering {
+    ///         todo!();
+    ///     }
+    /// }
+    ///
+    /// impl PartialOrd for A {
+    ///     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+    ///         Some(self.cmp(other))
+    ///     }
+    /// }
+    /// ```
+    #[clippy::version = "1.72.0"]
+    pub NEEDLESS_PARTIAL_ORD_IMPL,
+    correctness,
+    "manual implementation of `PartialOrd` when `Ord` is already implemented"
+}
+declare_lint_pass!(NeedlessImpls => [NEEDLESS_PARTIAL_ORD_IMPL]);
+
+impl LateLintPass<'_> for NeedlessImpls {
+    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 {
+            return;
+        };
+        let ItemKind::Impl(imp) = item.kind else {
+            return;
+        };
+        let Some(trait_impl) = cx.tcx.impl_trait_ref(item.owner_id).map(EarlyBinder::skip_binder) else {
+            return;
+        };
+        let trait_impl_def_id = trait_impl.def_id;
+        if cx.tcx.is_automatically_derived(item.owner_id.to_def_id()) {
+            return;
+        }
+        let ImplItemKind::Fn(_, impl_item_id) = cx.tcx.hir().impl_item(impl_item.impl_item_id()).kind else {
+            return;
+        };
+        let body = cx.tcx.hir().body(impl_item_id);
+        let ExprKind::Block(block, ..) = body.value.kind else {
+            return;
+        };
+
+        if cx.tcx.is_diagnostic_item(sym::PartialOrd, trait_impl_def_id)
+            && impl_item.ident.name == sym::partial_cmp
+            && let Some(ord_def_id) = cx
+                .tcx
+                .diagnostic_items(trait_impl.def_id.krate)
+                .name_to_id
+                .get(&sym::Ord)
+            && implements_trait(
+                    cx,
+                    hir_ty_to_ty(cx.tcx, imp.self_ty),
+                    *ord_def_id,
+                    trait_impl.substs,
+                )
+        {
+            if block.stmts.is_empty()
+                && let Some(expr) = block.expr
+                && let ExprKind::Call(Expr { kind: ExprKind::Path(some_path), .. }, [cmp_expr]) = expr.kind
+                // TODO: We can likely use the `option_some_variant` lang item instead, but this may be tricky
+                // due to the fact that `expr` is the constructor, not `option_some_variant` itself.
+                && match_qpath(some_path, &["Some"])
+                && let ExprKind::MethodCall(cmp_path, _, [other_expr], ..) = cmp_expr.kind
+                && cmp_path.ident.name == sym::cmp
+                && let Res::Local(..) = path_res(cx, other_expr)
+            {} else {
+                span_lint_and_then(
+                    cx,
+                    NEEDLESS_PARTIAL_ORD_IMPL,
+                    item.span,
+                    "manual implementation of `PartialOrd` when `Ord` is already implemented",
+                    |diag| {
+                        let (help, app) = if let Some(other) = body.params.get(0)
+                            && let PatKind::Binding(_, _, other_ident, ..) = other.pat.kind
+                        {
+                            (
+                                Cow::Owned(format!("{{ Some(self.cmp({})) }}", other_ident.name)),
+                                Applicability::Unspecified,
+                            )
+                        } else {
+                            (Cow::Borrowed("{ Some(self.cmp(...)) }"), Applicability::HasPlaceholders)
+                        };
+
+                        diag.span_suggestion(
+                            block.span,
+                            "change this to",
+                            help,
+                            app,
+                        );
+                    }
+                );
+            }
+        }
+    }
+}

tests/ui/bool_comparison.fixed

@@ -1,7 +1,7 @@
 //@run-rustfix
 
 #![warn(clippy::bool_comparison)]
-#![allow(clippy::manual_partial_ord_and_ord_impl)]
+#![allow(clippy::needless_partial_ord_impl)]
 
 fn main() {
     let x = true;

tests/ui/bool_comparison.rs

@@ -1,7 +1,7 @@
 //@run-rustfix
 
 #![warn(clippy::bool_comparison)]
-#![allow(clippy::manual_partial_ord_and_ord_impl)]
+#![allow(clippy::needless_partial_ord_impl)]
 
 fn main() {
     let x = true;

tests/ui/derive.rs

@@ -1,4 +1,4 @@
-#![allow(dead_code)]
+#![allow(clippy::needless_partial_ord_impl, dead_code)]
 #![warn(clippy::expl_impl_clone_on_copy)]
 
 #[derive(Copy)]

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::manual_partial_ord_and_ord_impl)]
+#![allow(clippy::needless_partial_ord_impl)]
 
 use std::cmp::Ordering;