[`non_canonical_partial_ord_impl`]: Fix emitting warnings which conflict with `needless_return`

3 files changed, 71 insertions, 11 deletions

diff --git a/clippy_lints/src/non_canonical_impls.rs b/clippy_lints/src/non_canonical_impls.rs
index fd3985a5daf..932d6fe54d6 100644
--- a/clippy_lints/src/non_canonical_impls.rs
+++ b/clippy_lints/src/non_canonical_impls.rs
@@ -182,17 +182,17 @@ impl LateLintPass<'_> for NonCanonicalImpls {
 
             if block.stmts.is_empty()
                 && let Some(expr) = block.expr
-                && let ExprKind::Call(
-                        Expr {
-                            kind: ExprKind::Path(some_path),
-                            hir_id: some_hir_id,
-                            ..
-                        },
-                        [cmp_expr],
-                    ) = expr.kind
-                && is_res_lang_ctor(cx, cx.qpath_res(some_path, *some_hir_id), LangItem::OptionSome)
-                // Fix #11178, allow `Self::cmp(self, ..)` too
-                && self_cmp_call(cx, cmp_expr, impl_item.owner_id.def_id, &mut needs_fully_qualified)
+                && expr_is_cmp(cx, &expr.kind, impl_item, &mut needs_fully_qualified)
+            {
+            }
+            // Fix #12683, allow [`needless_return`] here
+            else if block.expr.is_none()
+                && let Some(stmt) = block.stmts.first()
+                && let rustc_hir::StmtKind::Semi(Expr {
+                    kind: ExprKind::Ret(Some(Expr { kind: ret_kind, .. })),
+                    ..
+                }) = stmt.kind
+                && expr_is_cmp(cx, ret_kind, impl_item, &mut needs_fully_qualified)
             {
             } else {
                 // If `Self` and `Rhs` are not the same type, bail. This makes creating a valid
@@ -245,6 +245,30 @@ impl LateLintPass<'_> for NonCanonicalImpls {
     }
 }
 
+/// Return true if `expr_kind` is a `cmp` call.
+fn expr_is_cmp<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr_kind: &'tcx ExprKind<'tcx>,
+    impl_item: &ImplItem<'_>,
+    needs_fully_qualified: &mut bool,
+) -> bool {
+    if let ExprKind::Call(
+        Expr {
+            kind: ExprKind::Path(some_path),
+            hir_id: some_hir_id,
+            ..
+        },
+        [cmp_expr],
+    ) = expr_kind
+    {
+        is_res_lang_ctor(cx, cx.qpath_res(some_path, *some_hir_id), LangItem::OptionSome)
+            // Fix #11178, allow `Self::cmp(self, ..)` too
+            && self_cmp_call(cx, cmp_expr, impl_item.owner_id.def_id, needs_fully_qualified)
+    } else {
+        false
+    }
+}
+
 /// Returns whether this is any of `self.cmp(..)`, `Self::cmp(self, ..)` or `Ord::cmp(self, ..)`.
 fn self_cmp_call<'tcx>(
     cx: &LateContext<'tcx>,
diff --git a/tests/ui/non_canonical_partial_ord_impl.fixed b/tests/ui/non_canonical_partial_ord_impl.fixed
index db55cc094e3..d444a753697 100644
--- a/tests/ui/non_canonical_partial_ord_impl.fixed
+++ b/tests/ui/non_canonical_partial_ord_impl.fixed
@@ -142,3 +142,21 @@ impl PartialOrd for H {
         Some(Ord::cmp(self, other))
     }
 }
+
+// #12683, do not lint
+
+#[derive(Eq, PartialEq)]
+struct I(u32);
+
+impl Ord for I {
+    fn cmp(&self, other: &Self) -> Ordering {
+        todo!();
+    }
+}
+
+impl PartialOrd for I {
+    #[allow(clippy::needless_return)]
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        return Some(self.cmp(other));
+    }
+}
diff --git a/tests/ui/non_canonical_partial_ord_impl.rs b/tests/ui/non_canonical_partial_ord_impl.rs
index 52f4b85b917..dc6c4354604 100644
--- a/tests/ui/non_canonical_partial_ord_impl.rs
+++ b/tests/ui/non_canonical_partial_ord_impl.rs
@@ -146,3 +146,21 @@ impl PartialOrd for H {
         Some(Ord::cmp(self, other))
     }
 }
+
+// #12683, do not lint
+
+#[derive(Eq, PartialEq)]
+struct I(u32);
+
+impl Ord for I {
+    fn cmp(&self, other: &Self) -> Ordering {
+        todo!();
+    }
+}
+
+impl PartialOrd for I {
+    #[allow(clippy::needless_return)]
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        return Some(self.cmp(other));
+    }
+}