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 ;
3- use rustc_hir:: { ExprKind , ImplItem , ImplItemKind , ItemKind , Node , UnOp } ;
7+ use rustc_hir:: { def :: Res , Expr , ExprKind , ImplItem , ImplItemKind , ItemKind , LangItem , Node , PatKind , UnOp } ;
48use rustc_hir_analysis:: hir_ty_to_ty;
59use rustc_lint:: { LateContext , LateLintPass } ;
610use rustc_middle:: ty:: EarlyBinder ;
711use rustc_session:: { declare_lint_pass, declare_tool_lint} ;
812use rustc_span:: { sym, symbol} ;
13+ use std:: borrow:: Cow ;
914
1015declare_clippy_lint ! {
1116 /// ### What it does
@@ -46,10 +51,59 @@ declare_clippy_lint! {
4651 correctness,
4752 "manual implementation of `Clone` on a `Copy` type"
4853}
49- 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 ] ) ;
50104
51105impl LateLintPass < ' _ > for IncorrectImpls {
52- #[ expect( clippy:: needless_return ) ]
106+ #[ expect( clippy:: too_many_lines ) ]
53107 fn check_impl_item ( & mut self , cx : & LateContext < ' _ > , impl_item : & ImplItem < ' _ > ) {
54108 let node = get_parent_node ( cx. tcx , impl_item. hir_id ( ) ) ;
55109 let Some ( Node :: Item ( item) ) = node else {
@@ -72,10 +126,7 @@ impl LateLintPass<'_> for IncorrectImpls {
72126 let ExprKind :: Block ( block, ..) = body. value . kind else {
73127 return ;
74128 } ;
75- // Above is duplicated from the `duplicate_manual_partial_ord_impl` branch.
76- // Remove it while solving conflicts once that PR is merged.
77129
78- // Actual implementation; remove this comment once aforementioned PR is merged
79130 if cx. tcx . is_diagnostic_item ( sym:: Clone , trait_impl_def_id)
80131 && let Some ( copy_def_id) = cx. tcx . get_diagnostic_item ( sym:: Copy )
81132 && implements_trait (
@@ -120,5 +171,71 @@ impl LateLintPass<'_> for IncorrectImpls {
120171 return ;
121172 }
122173 }
174+
175+ if cx. tcx . is_diagnostic_item ( sym:: PartialOrd , trait_impl_def_id)
176+ && impl_item. ident . name == sym:: partial_cmp
177+ && let Some ( ord_def_id) = cx
178+ . tcx
179+ . diagnostic_items ( trait_impl. def_id . krate )
180+ . name_to_id
181+ . get ( & sym:: Ord )
182+ && implements_trait (
183+ cx,
184+ hir_ty_to_ty ( cx. tcx , imp. self_ty ) ,
185+ * ord_def_id,
186+ trait_impl. substs ,
187+ )
188+ {
189+ if block. stmts . is_empty ( )
190+ && let Some ( expr) = block. expr
191+ && let ExprKind :: Call (
192+ Expr {
193+ kind : ExprKind :: Path ( some_path) ,
194+ hir_id : some_hir_id,
195+ ..
196+ } ,
197+ [ cmp_expr] ,
198+ ) = expr. kind
199+ && is_res_lang_ctor ( cx, cx. qpath_res ( some_path, * some_hir_id) , LangItem :: OptionSome )
200+ && let ExprKind :: MethodCall ( cmp_path, _, [ other_expr] , ..) = cmp_expr. kind
201+ && cmp_path. ident . name == sym:: cmp
202+ && let Res :: Local ( ..) = path_res ( cx, other_expr)
203+ { } else {
204+ // If lhs and rhs are not the same type, bail. This makes creating a valid
205+ // suggestion tons more complex.
206+ if let Some ( lhs) = trait_impl. substs . get ( 0 )
207+ && let Some ( rhs) = trait_impl. substs . get ( 1 )
208+ && lhs != rhs
209+ {
210+ return ;
211+ }
212+
213+ span_lint_and_then (
214+ cx,
215+ INCORRECT_PARTIAL_ORD_IMPL_ON_ORD_TYPE ,
216+ item. span ,
217+ "incorrect implementation of `partial_cmp` on an `Ord` type" ,
218+ |diag| {
219+ let ( help, app) = if let Some ( other) = body. params . get ( 1 )
220+ && let PatKind :: Binding ( _, _, other_ident, ..) = other. pat . kind
221+ {
222+ (
223+ Cow :: Owned ( format ! ( "{{ Some(self.cmp({})) }}" , other_ident. name) ) ,
224+ Applicability :: Unspecified ,
225+ )
226+ } else {
227+ ( Cow :: Borrowed ( "{ Some(self.cmp(...)) }" ) , Applicability :: HasPlaceholders )
228+ } ;
229+
230+ diag. span_suggestion (
231+ block. span ,
232+ "change this to" ,
233+ help,
234+ app,
235+ ) ;
236+ }
237+ ) ;
238+ }
239+ }
123240 }
124241}
0 commit comments