1- use clippy_utils:: { diagnostics:: span_lint_and_sugg, get_parent_node, last_path_segment, ty:: implements_trait} ;
1+ use clippy_utils:: {
2+ diagnostics:: { span_lint_and_sugg, span_lint_and_then} ,
3+ get_parent_node, is_res_lang_ctor, last_path_segment, path_res,
4+ ty:: implements_trait,
5+ } ;
26use rustc_errors:: Applicability ;
7+ use rustc_hir:: { def:: Res , Expr , ExprKind , ImplItem , ImplItemKind , ItemKind , LangItem , Node , PatKind , UnOp } ;
38use rustc_hir:: { ExprKind , ImplItem , ImplItemKind , Node , UnOp } ;
49use rustc_lint:: { LateContext , LateLintPass } ;
510use rustc_middle:: ty:: EarlyBinder ;
611use rustc_session:: { declare_lint_pass, declare_tool_lint} ;
712use rustc_span:: { sym, symbol} ;
13+ use std:: borrow:: Cow ;
814
915declare_clippy_lint ! {
1016 /// ### What it does
@@ -45,10 +51,59 @@ declare_clippy_lint! {
4551 correctness,
4652 "manual implementation of `Clone` on a `Copy` type"
4753}
48- declare_lint_pass ! ( IncorrectImpls => [ INCORRECT_CLONE_IMPL_ON_COPY_TYPE ] ) ;
54+ declare_clippy_lint ! {
55+ /// ### What it does
56+ /// Checks for manual implementations of both `PartialOrd` and `Ord` when only `Ord` is
57+ /// necessary.
58+ ///
59+ /// ### Why is this bad?
60+ /// If both `PartialOrd` and `Ord` are implemented, they must agree. This is commonly done by
61+ /// wrapping the result of `cmp` in `Some` for `partial_cmp`. Not doing this may silently
62+ /// introduce an error upon refactoring.
63+ ///
64+ /// ### Example
65+ /// ```rust,ignore
66+ /// #[derive(Eq, PartialEq)]
67+ /// struct A(u32);
68+ ///
69+ /// impl Ord for A {
70+ /// fn cmp(&self, other: &Self) -> Ordering {
71+ /// todo!();
72+ /// }
73+ /// }
74+ ///
75+ /// impl PartialOrd for A {
76+ /// fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
77+ /// todo!();
78+ /// }
79+ /// }
80+ /// ```
81+ /// Use instead:
82+ /// ```rust,ignore
83+ /// #[derive(Eq, PartialEq)]
84+ /// struct A(u32);
85+ ///
86+ /// impl Ord for A {
87+ /// fn cmp(&self, other: &Self) -> Ordering {
88+ /// todo!();
89+ /// }
90+ /// }
91+ ///
92+ /// impl PartialOrd for A {
93+ /// fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
94+ /// Some(self.cmp(other))
95+ /// }
96+ /// }
97+ /// ```
98+ [ clippy:: version = "1.72.0" ]
99+ pub INCORRECT_PARTIAL_ORD_IMPL_ON_ORD_TYPE ,
100+ correctness,
101+ "manual implementation of `PartialOrd` when `Ord` is already implemented"
102+ }
103+ declare_lint_pass ! ( IncorrectImpls => [ INCORRECT_CLONE_IMPL_ON_COPY_TYPE , INCORRECT_PARTIAL_ORD_IMPL_ON_ORD_TYPE ] ) ;
49104
50105impl LateLintPass < ' _ > for IncorrectImpls {
51- #[ expect( clippy:: needless_return ) ]
106+ #[ expect( clippy:: too_many_lines ) ]
52107 fn check_impl_item ( & mut self , cx : & LateContext < ' _ > , impl_item : & ImplItem < ' _ > ) {
53108 let node = get_parent_node ( cx. tcx , impl_item. hir_id ( ) ) ;
54109 let Some ( Node :: Item ( item) ) = node else {
@@ -68,10 +123,7 @@ impl LateLintPass<'_> for IncorrectImpls {
68123 let ExprKind :: Block ( block, ..) = body. value . kind else {
69124 return ;
70125 } ;
71- // Above is duplicated from the `duplicate_manual_partial_ord_impl` branch.
72- // Remove it while solving conflicts once that PR is merged.
73126
74- // Actual implementation; remove this comment once aforementioned PR is merged
75127 if cx. tcx . is_diagnostic_item ( sym:: Clone , trait_impl_def_id)
76128 && let Some ( copy_def_id) = cx. tcx . get_diagnostic_item ( sym:: Copy )
77129 && implements_trait (
@@ -116,5 +168,71 @@ impl LateLintPass<'_> for IncorrectImpls {
116168 return ;
117169 }
118170 }
171+
172+ if cx. tcx . is_diagnostic_item ( sym:: PartialOrd , trait_impl_def_id)
173+ && impl_item. ident . name == sym:: partial_cmp
174+ && let Some ( ord_def_id) = cx
175+ . tcx
176+ . diagnostic_items ( trait_impl. def_id . krate )
177+ . name_to_id
178+ . get ( & sym:: Ord )
179+ && implements_trait (
180+ cx,
181+ hir_ty_to_ty ( cx. tcx , imp. self_ty ) ,
182+ * ord_def_id,
183+ trait_impl. substs ,
184+ )
185+ {
186+ if block. stmts . is_empty ( )
187+ && let Some ( expr) = block. expr
188+ && let ExprKind :: Call (
189+ Expr {
190+ kind : ExprKind :: Path ( some_path) ,
191+ hir_id : some_hir_id,
192+ ..
193+ } ,
194+ [ cmp_expr] ,
195+ ) = expr. kind
196+ && is_res_lang_ctor ( cx, cx. qpath_res ( some_path, * some_hir_id) , LangItem :: OptionSome )
197+ && let ExprKind :: MethodCall ( cmp_path, _, [ other_expr] , ..) = cmp_expr. kind
198+ && cmp_path. ident . name == sym:: cmp
199+ && let Res :: Local ( ..) = path_res ( cx, other_expr)
200+ { } else {
201+ // If lhs and rhs are not the same type, bail. This makes creating a valid
202+ // suggestion tons more complex.
203+ if let Some ( lhs) = trait_impl. substs . get ( 0 )
204+ && let Some ( rhs) = trait_impl. substs . get ( 1 )
205+ && lhs != rhs
206+ {
207+ return ;
208+ }
209+
210+ span_lint_and_then (
211+ cx,
212+ INCORRECT_PARTIAL_ORD_IMPL_ON_ORD_TYPE ,
213+ item. span ,
214+ "incorrect implementation of `partial_cmp` on an `Ord` type" ,
215+ |diag| {
216+ let ( help, app) = if let Some ( other) = body. params . get ( 1 )
217+ && let PatKind :: Binding ( _, _, other_ident, ..) = other. pat . kind
218+ {
219+ (
220+ Cow :: Owned ( format ! ( "{{ Some(self.cmp({})) }}" , other_ident. name) ) ,
221+ Applicability :: Unspecified ,
222+ )
223+ } else {
224+ ( Cow :: Borrowed ( "{ Some(self.cmp(...)) }" ) , Applicability :: HasPlaceholders )
225+ } ;
226+
227+ diag. span_suggestion (
228+ block. span ,
229+ "change this to" ,
230+ help,
231+ app,
232+ ) ;
233+ }
234+ ) ;
235+ }
236+ }
119237 }
120238}
0 commit comments