Auto merge of #12702 - Luv-Ray:non_canonical_partial_ord_impl, r=Manishearth

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

fixes #12683

---

changelog: fix [`non_canonical_partial_ord_impl`] emitting warnings which conflict with `needless_return`

clippy_lints/src/non_canonical_impls.rs

@@ -182,17 +182,17 @@ fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) {
 
             if block.stmts.is_empty()
                 && let Some(expr) = block.expr
-                && let ExprKind::Call(
+                && expr_is_cmp(cx, &expr.kind, impl_item, &mut needs_fully_qualified)
-                        Expr {
+            {
-                            kind: ExprKind::Path(some_path),
+            }
-                            hir_id: some_hir_id,
+            // Fix #12683, allow [`needless_return`] here
-                            ..
+            else if block.expr.is_none()
-                        },
+                && let Some(stmt) = block.stmts.first()
-                        [cmp_expr],
+                && let rustc_hir::StmtKind::Semi(Expr {
-                    ) = expr.kind
+                    kind: ExprKind::Ret(Some(Expr { kind: ret_kind, .. })),
-                && is_res_lang_ctor(cx, cx.qpath_res(some_path, *some_hir_id), LangItem::OptionSome)
+                    ..
-                // Fix #11178, allow `Self::cmp(self, ..)` too
+                }) = stmt.kind
-                && self_cmp_call(cx, cmp_expr, impl_item.owner_id.def_id, &mut needs_fully_qualified)
+                && 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 @@ fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) {
     }
 }
 
+/// 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>,

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));
+    }
+}

tests/ui/non_canonical_partial_ord_impl.rs

@@ -146,3 +146,21 @@ fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
         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));
+    }
+}