Fix LicenseItem ordering consistency

CHANGELOG.md

@@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 <!-- next-header -->
 ## [Unreleased] - ReleaseDate
+### Fixed
+- Fixed `LicenseItem`'s `PartialEq` and `PartialOrd` implementations to agree with its total ordering, resolving [#95](https://github.com/EmbarkStudios/spdx/issues/95).
+
 ## [0.13.4] - 2026-02-26
 ### Changed
 - [PR#92](https://github.com/EmbarkStudios/spdx/pull/92) updated SPDX license list to [3.28.0](https://github.com/spdx/license-list-XML/releases/tag/v3.28.0).

src/detection/preproc.rs

@@ -202,7 +202,7 @@ fn remove_common_tokens(input: Cow<'_, str>) -> Cow<'_, str> {
     // reconcile the count with other longer prefixes that may be stored
     let common_count = prefix_counts
         .iter()
-        .filter_map(|(s, count)| Some(count).filter(|_| s.starts_with(most_common)))
+        .filter_map(|(s, count)| s.starts_with(most_common).then_some(count))
         .sum::<u32>();
 
     let line_count = input.split('\n').count();

src/lib.rs

@@ -415,21 +415,15 @@ impl Ord for LicenseItem {
     }
 }
 
-#[allow(clippy::non_canonical_partial_ord_impl)]
 impl PartialOrd for LicenseItem {
     fn partial_cmp(&self, o: &Self) -> Option<Ordering> {
-        match (self, o) {
-            (Self::Spdx { id: a, .. }, Self::Spdx { id: b, .. }) => a.partial_cmp(b),
-            (Self::Other(a), Self::Other(b)) => a.partial_cmp(b),
-            (Self::Spdx { .. }, Self::Other { .. }) => Some(cmp::Ordering::Less),
-            (Self::Other { .. }, Self::Spdx { .. }) => Some(cmp::Ordering::Greater),
-        }
+        Some(self.cmp(o))
     }
 }
 
 impl PartialEq for LicenseItem {
     fn eq(&self, o: &Self) -> bool {
-        matches!(self.partial_cmp(o), Some(cmp::Ordering::Equal))
+        self.cmp(o) == cmp::Ordering::Equal
     }
 }
 
@@ -648,9 +642,10 @@ pub fn license_version() -> &'static str {
 
 #[cfg(test)]
 mod test {
-    use super::LicenseItem;
+    use super::{LicenseItem, LicenseRef, LicenseReq};
     use crate::{Expression, license_id};
-    use alloc::string::ToString;
+    use alloc::{borrow::ToOwned, boxed::Box, string::ToString};
+    use core::cmp::Ordering;
 
     #[test]
     fn gnu_or_later_display() {
@@ -676,4 +671,77 @@ mod test {
         assert_eq!(gpl_or_later_in_id.to_string(), "GPL-3.0-or-later");
         assert_eq!(non_gnu_or_later.to_string(), "Apache-2.0+");
     }
+
+    #[test]
+    fn license_item_ordering_is_canonical() {
+        let mit = license_id("MIT").unwrap();
+        let apache = license_id("Apache-2.0").unwrap();
+        let items = [
+            LicenseItem::Spdx {
+                id: apache,
+                or_later: false,
+            },
+            LicenseItem::Spdx {
+                id: mit,
+                or_later: false,
+            },
+            LicenseItem::Spdx {
+                id: mit,
+                or_later: true,
+            },
+            LicenseItem::Other(Box::new(LicenseRef {
+                doc_ref: None,
+                lic_ref: "custom".to_owned(),
+            })),
+            LicenseItem::Other(Box::new(LicenseRef {
+                doc_ref: Some("document".to_owned()),
+                lic_ref: "custom".to_owned(),
+            })),
+        ];
+
+        assert_eq!(items[1].cmp(&items[2]), Ordering::Less);
+        assert_ne!(items[1], items[2]);
+
+        for left in &items {
+            for right in &items {
+                assert_eq!(left.partial_cmp(right), Some(left.cmp(right)));
+                assert_eq!(*left == *right, left.cmp(right) == Ordering::Equal);
+                assert_eq!(left.cmp(right), right.cmp(left).reverse());
+
+                for last in &items {
+                    if left <= right && right <= last {
+                        assert!(left <= last);
+                    }
+                }
+            }
+        }
+    }
+
+    #[test]
+    fn license_req_ordering_is_canonical() {
+        let mit = license_id("MIT").unwrap();
+        let requirements = [
+            LicenseReq {
+                license: LicenseItem::Spdx {
+                    id: mit,
+                    or_later: false,
+                },
+                addition: None,
+            },
+            LicenseReq {
+                license: LicenseItem::Spdx {
+                    id: mit,
+                    or_later: true,
+                },
+                addition: None,
+            },
+        ];
+
+        for left in &requirements {
+            for right in &requirements {
+                assert_eq!(left.partial_cmp(right), Some(left.cmp(right)));
+                assert_eq!(*left == *right, left.cmp(right) == Ordering::Equal);
+            }
+        }
+    }
 }

src/licensee.rs

@@ -245,14 +245,24 @@ impl Licensee {
 impl PartialOrd<LicenseReq> for Licensee {
     #[inline]
     fn partial_cmp(&self, o: &LicenseReq) -> Option<core::cmp::Ordering> {
-        self.inner.partial_cmp(o)
+        let license = match (&self.inner.license, &o.license) {
+            (LicenseItem::Spdx { id: a, .. }, LicenseItem::Spdx { id: b, .. }) => a.cmp(b),
+            (LicenseItem::Other(a), LicenseItem::Other(b)) => a.cmp(b),
+            (LicenseItem::Spdx { .. }, LicenseItem::Other { .. }) => core::cmp::Ordering::Less,
+            (LicenseItem::Other { .. }, LicenseItem::Spdx { .. }) => core::cmp::Ordering::Greater,
+        };
+
+        match license {
+            core::cmp::Ordering::Equal => self.inner.addition.partial_cmp(&o.addition),
+            ordering => Some(ordering),
+        }
     }
 }
 
 impl PartialEq<LicenseReq> for Licensee {
     #[inline]
     fn eq(&self, o: &LicenseReq) -> bool {
-        self.inner.eq(o)
+        self.partial_cmp(o) == Some(core::cmp::Ordering::Equal)
     }
 }
 
@@ -309,12 +319,12 @@ mod test {
         // Licensees can't have the `or_later`
         assert!(licensees.binary_search_by(|l| l.inner.cmp(&req)).is_err());
 
-        match &licensees[licensees
+        let licensee = &licensees[licensees
             .binary_search_by(|l| l.partial_cmp(&req).unwrap())
-            .unwrap()]
-        .inner
-        .license
-        {
+            .unwrap()];
+        assert_eq!(licensee, &req);
+
+        match &licensee.inner.license {
             LicenseItem::Spdx { id, .. } => assert_eq!(*id, mpl_id),
             o @ LicenseItem::Other { .. } => panic!("unexpected {o:?}"),
         }