11use crate :: utils:: paths;
22use crate :: utils:: {
3- is_automatically_derived, is_copy, match_path, span_lint_and_help, span_lint_and_note, span_lint_and_then,
3+ get_trait_def_id, is_allowed, is_automatically_derived, is_copy, match_path, span_lint_and_help,
4+ span_lint_and_note, span_lint_and_then,
45} ;
56use if_chain:: if_chain;
67use rustc_hir:: def_id:: DefId ;
@@ -43,6 +44,57 @@ declare_clippy_lint! {
4344 "deriving `Hash` but implementing `PartialEq` explicitly"
4445}
4546
47+ declare_clippy_lint ! {
48+ /// **What it does:** Checks for deriving `Ord` but implementing `PartialOrd`
49+ /// explicitly or vice versa.
50+ ///
51+ /// **Why is this bad?** The implementation of these traits must agree (for
52+ /// example for use with `sort`) so it’s probably a bad idea to use a
53+ /// default-generated `Ord` implementation with an explicitly defined
54+ /// `PartialOrd`. In particular, the following must hold for any type
55+ /// implementing `Ord`:
56+ ///
57+ /// ```text
58+ /// k1.cmp(&k2) == k1.partial_cmp(&k2).unwrap()
59+ /// ```
60+ ///
61+ /// **Known problems:** None.
62+ ///
63+ /// **Example:**
64+ ///
65+ /// ```rust,ignore
66+ /// #[derive(Ord, PartialEq, Eq)]
67+ /// struct Foo;
68+ ///
69+ /// impl PartialOrd for Foo {
70+ /// ...
71+ /// }
72+ /// ```
73+ /// Use instead:
74+ /// ```rust,ignore
75+ /// #[derive(PartialEq, Eq)]
76+ /// struct Foo;
77+ ///
78+ /// impl PartialOrd for Foo {
79+ /// fn partial_cmp(&self, other: &Foo) -> Option<Ordering> {
80+ /// Some(self.cmp(other))
81+ /// }
82+ /// }
83+ ///
84+ /// impl Ord for Foo {
85+ /// ...
86+ /// }
87+ /// ```
88+ /// or, if you don't need a custom ordering:
89+ /// ```rust,ignore
90+ /// #[derive(Ord, PartialOrd, PartialEq, Eq)]
91+ /// struct Foo;
92+ /// ```
93+ pub DERIVE_ORD_XOR_PARTIAL_ORD ,
94+ correctness,
95+ "deriving `Ord` but implementing `PartialOrd` explicitly"
96+ }
97+
4698declare_clippy_lint ! {
4799 /// **What it does:** Checks for explicit `Clone` implementations for `Copy`
48100/// types.
@@ -103,7 +155,12 @@ declare_clippy_lint! {
103155 "deriving `serde::Deserialize` on a type that has methods using `unsafe`"
104156}
105157
106- declare_lint_pass ! ( Derive => [ EXPL_IMPL_CLONE_ON_COPY , DERIVE_HASH_XOR_EQ , UNSAFE_DERIVE_DESERIALIZE ] ) ;
158+ declare_lint_pass ! ( Derive => [
159+ EXPL_IMPL_CLONE_ON_COPY ,
160+ DERIVE_HASH_XOR_EQ ,
161+ DERIVE_ORD_XOR_PARTIAL_ORD ,
162+ UNSAFE_DERIVE_DESERIALIZE
163+ ] ) ;
107164
108165impl < ' tcx > LateLintPass < ' tcx > for Derive {
109166 fn check_item ( & mut self , cx : & LateContext < ' tcx > , item : & ' tcx Item < ' _ > ) {
@@ -116,6 +173,7 @@ impl<'tcx> LateLintPass<'tcx> for Derive {
116173 let is_automatically_derived = is_automatically_derived ( & * item. attrs ) ;
117174
118175 check_hash_peq ( cx, item. span , trait_ref, ty, is_automatically_derived) ;
176+ check_ord_partial_ord ( cx, item. span , trait_ref, ty, is_automatically_derived) ;
119177
120178 if is_automatically_derived {
121179 check_unsafe_derive_deserialize ( cx, item, trait_ref, ty) ;
@@ -180,6 +238,60 @@ fn check_hash_peq<'tcx>(
180238 }
181239}
182240
241+ /// Implementation of the `DERIVE_ORD_XOR_PARTIAL_ORD` lint.
242+ fn check_ord_partial_ord < ' tcx > (
243+ cx : & LateContext < ' tcx > ,
244+ span : Span ,
245+ trait_ref : & TraitRef < ' _ > ,
246+ ty : Ty < ' tcx > ,
247+ ord_is_automatically_derived : bool ,
248+ ) {
249+ if_chain ! {
250+ if let Some ( ord_trait_def_id) = get_trait_def_id( cx, & paths:: ORD ) ;
251+ if let Some ( partial_ord_trait_def_id) = cx. tcx. lang_items( ) . partial_ord_trait( ) ;
252+ if let Some ( def_id) = & trait_ref. trait_def_id( ) ;
253+ if * def_id == ord_trait_def_id;
254+ then {
255+ // Look for the PartialOrd implementations for `ty`
256+ cx. tcx. for_each_relevant_impl( partial_ord_trait_def_id, ty, |impl_id| {
257+ let partial_ord_is_automatically_derived = is_automatically_derived( & cx. tcx. get_attrs( impl_id) ) ;
258+
259+ if partial_ord_is_automatically_derived == ord_is_automatically_derived {
260+ return ;
261+ }
262+
263+ let trait_ref = cx. tcx. impl_trait_ref( impl_id) . expect( "must be a trait implementation" ) ;
264+
265+ // Only care about `impl PartialOrd<Foo> for Foo`
266+ // For `impl PartialOrd<B> for A, input_types is [A, B]
267+ if trait_ref. substs. type_at( 1 ) == ty {
268+ let mess = if partial_ord_is_automatically_derived {
269+ "you are implementing `Ord` explicitly but have derived `PartialOrd`"
270+ } else {
271+ "you are deriving `Ord` but have implemented `PartialOrd` explicitly"
272+ } ;
273+
274+ span_lint_and_then(
275+ cx,
276+ DERIVE_ORD_XOR_PARTIAL_ORD ,
277+ span,
278+ mess,
279+ |diag| {
280+ if let Some ( local_def_id) = impl_id. as_local( ) {
281+ let hir_id = cx. tcx. hir( ) . as_local_hir_id( local_def_id) ;
282+ diag. span_note(
283+ cx. tcx. hir( ) . span( hir_id) ,
284+ "`PartialOrd` implemented here"
285+ ) ;
286+ }
287+ }
288+ ) ;
289+ }
290+ } ) ;
291+ }
292+ }
293+ }
294+
183295/// Implementation of the `EXPL_IMPL_CLONE_ON_COPY` lint.
184296fn check_copy_clone < ' tcx > ( cx : & LateContext < ' tcx > , item : & Item < ' _ > , trait_ref : & TraitRef < ' _ > , ty : Ty < ' tcx > ) {
185297 if match_path ( & trait_ref. path , & paths:: CLONE_TRAIT ) {
@@ -242,7 +354,9 @@ fn check_unsafe_derive_deserialize<'tcx>(
242354 if_chain ! {
243355 if match_path( & trait_ref. path, & paths:: SERDE_DESERIALIZE ) ;
244356 if let ty:: Adt ( def, _) = ty. kind;
245- if def. did. is_local( ) ;
357+ if let Some ( local_def_id) = def. did. as_local( ) ;
358+ let adt_hir_id = cx. tcx. hir( ) . as_local_hir_id( local_def_id) ;
359+ if !is_allowed( cx, UNSAFE_DERIVE_DESERIALIZE , adt_hir_id) ;
246360 if cx. tcx. inherent_impls( def. did)
247361 . iter( )
248362 . map( |imp_did| item_from_def_id( cx, * imp_did) )
0 commit comments