diff options
| -rw-r--r-- | clippy_lints/src/non_canonical_impls.rs | 25 | ||||
| -rw-r--r-- | tests/ui/non_canonical_partial_ord_impl.fixed | 33 | ||||
| -rw-r--r-- | tests/ui/non_canonical_partial_ord_impl.rs | 35 | ||||
| -rw-r--r-- | tests/ui/non_canonical_partial_ord_impl.stderr | 15 |
4 files changed, 99 insertions, 9 deletions
diff --git a/clippy_lints/src/non_canonical_impls.rs b/clippy_lints/src/non_canonical_impls.rs index 329415e2dc9..93865197ec9 100644 --- a/clippy_lints/src/non_canonical_impls.rs +++ b/clippy_lints/src/non_canonical_impls.rs @@ -1,6 +1,8 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use clippy_utils::ty::implements_trait; -use clippy_utils::{is_from_proc_macro, is_res_lang_ctor, last_path_segment, path_res, std_or_core}; +use clippy_utils::{ + is_diag_trait_item, is_from_proc_macro, is_res_lang_ctor, last_path_segment, path_res, std_or_core, +}; use rustc_errors::Applicability; use rustc_hir::def_id::LocalDefId; use rustc_hir::{Expr, ExprKind, ImplItem, ImplItemKind, LangItem, Node, UnOp}; @@ -98,7 +100,7 @@ declare_clippy_lint! { /// /// impl PartialOrd for A { /// fn partial_cmp(&self, other: &Self) -> Option<Ordering> { - /// Some(self.cmp(other)) + /// Some(self.cmp(other)) // or self.cmp(other).into() /// } /// } /// ``` @@ -185,7 +187,7 @@ impl LateLintPass<'_> for NonCanonicalImpls { if block.stmts.is_empty() && let Some(expr) = block.expr - && expr_is_cmp(cx, &expr.kind, impl_item, &mut needs_fully_qualified) + && expr_is_cmp(cx, expr, impl_item, &mut needs_fully_qualified) { return; } @@ -193,10 +195,10 @@ impl LateLintPass<'_> for NonCanonicalImpls { 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, .. })), + kind: ExprKind::Ret(Some(ret)), .. }) = stmt.kind - && expr_is_cmp(cx, ret_kind, impl_item, &mut needs_fully_qualified) + && expr_is_cmp(cx, ret, impl_item, &mut needs_fully_qualified) { return; } @@ -252,10 +254,11 @@ 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>, + expr: &'tcx Expr<'tcx>, impl_item: &ImplItem<'_>, needs_fully_qualified: &mut bool, ) -> bool { + let impl_item_did = impl_item.owner_id.def_id; if let ExprKind::Call( Expr { kind: ExprKind::Path(some_path), @@ -263,11 +266,17 @@ fn expr_is_cmp<'tcx>( .. }, [cmp_expr], - ) = expr_kind + ) = 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) + && self_cmp_call(cx, cmp_expr, impl_item_did, needs_fully_qualified) + } else if let ExprKind::MethodCall(_, recv, [], _) = expr.kind { + cx.tcx + .typeck(impl_item_did) + .type_dependent_def_id(expr.hir_id) + .is_some_and(|def_id| is_diag_trait_item(cx, def_id, sym::Into)) + && self_cmp_call(cx, recv, impl_item_did, needs_fully_qualified) } else { false } diff --git a/tests/ui/non_canonical_partial_ord_impl.fixed b/tests/ui/non_canonical_partial_ord_impl.fixed index 8774c666db1..23dbee5a084 100644 --- a/tests/ui/non_canonical_partial_ord_impl.fixed +++ b/tests/ui/non_canonical_partial_ord_impl.fixed @@ -162,3 +162,36 @@ impl PartialOrd for I { return Some(self.cmp(other)); } } + +// #13640, do not lint + +#[derive(Eq, PartialEq)] +struct J(u32); + +impl Ord for J { + fn cmp(&self, other: &Self) -> Ordering { + todo!(); + } +} + +impl PartialOrd for J { + fn partial_cmp(&self, other: &Self) -> Option<Ordering> { + self.cmp(other).into() + } +} + +// #13640, check that a simple `.into()` does not obliterate the lint + +#[derive(Eq, PartialEq)] +struct K(u32); + +impl Ord for K { + fn cmp(&self, other: &Self) -> Ordering { + todo!(); + } +} + +impl PartialOrd for K { + //~^ non_canonical_partial_ord_impl + fn partial_cmp(&self, other: &Self) -> Option<Ordering> { 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 568b97c8fff..12f055a542b 100644 --- a/tests/ui/non_canonical_partial_ord_impl.rs +++ b/tests/ui/non_canonical_partial_ord_impl.rs @@ -166,3 +166,38 @@ impl PartialOrd for I { return Some(self.cmp(other)); } } + +// #13640, do not lint + +#[derive(Eq, PartialEq)] +struct J(u32); + +impl Ord for J { + fn cmp(&self, other: &Self) -> Ordering { + todo!(); + } +} + +impl PartialOrd for J { + fn partial_cmp(&self, other: &Self) -> Option<Ordering> { + self.cmp(other).into() + } +} + +// #13640, check that a simple `.into()` does not obliterate the lint + +#[derive(Eq, PartialEq)] +struct K(u32); + +impl Ord for K { + fn cmp(&self, other: &Self) -> Ordering { + todo!(); + } +} + +impl PartialOrd for K { + //~^ non_canonical_partial_ord_impl + fn partial_cmp(&self, other: &Self) -> Option<Ordering> { + Ordering::Greater.into() + } +} diff --git a/tests/ui/non_canonical_partial_ord_impl.stderr b/tests/ui/non_canonical_partial_ord_impl.stderr index 86845df4ea9..c7de968588f 100644 --- a/tests/ui/non_canonical_partial_ord_impl.stderr +++ b/tests/ui/non_canonical_partial_ord_impl.stderr @@ -31,5 +31,18 @@ LL - fn partial_cmp(&self, _: &Self) -> Option<Ordering> { LL + fn partial_cmp(&self, other: &Self) -> Option<Ordering> { Some(self.cmp(other)) } | -error: aborting due to 2 previous errors +error: non-canonical implementation of `partial_cmp` on an `Ord` type + --> tests/ui/non_canonical_partial_ord_impl.rs:198:1 + | +LL | / impl PartialOrd for K { +LL | | +LL | | fn partial_cmp(&self, other: &Self) -> Option<Ordering> { + | | _____________________________________________________________- +LL | || Ordering::Greater.into() +LL | || } + | ||_____- help: change this to: `{ Some(self.cmp(other)) }` +LL | | } + | |__^ + +error: aborting due to 3 previous errors |