Non-canonical impl of `PartialOrd` for `LicenseItem` breaks on rustc 1.98.0-beta.1

[makai410]

Describe the bug

As said in the documentation for the PartialOrd trait:

If Ord is also implemented for Self and Rhs, it must also be consistent with partial_cmp (see the documentation of that trait for the exact requirements).

This means one requirement for implementing PartialOrd is partial_cmp(a, b) == Some(a.cmp(b)).

However, as shown in https://docs.rs/spdx/0.13.4/src/spdx/lib.rs.html#395-428, LicenseItem's PartialOrd and Ord impls disagree: Ord::cmp was implemented using both id and or_later, but PartialOrd::partial_cmp only compares id. This violates this requirement in the documentation.

It was previously not an issue because outer types (e.g. LicenseReq, which wraps LicenseItem and derives both PartialOrd and Ord) generated field-wise comparisons that happened to go through partial_cmp.

A recent rustc change (rust-lang/rust#155598) makes derived PartialOrd::partial_cmp delegate to Ord::cmp instead when both are derived together. Afterwards on 1.98 beta, LicenseReq's derived partial_cmp instead calls Some(self.cmp(other)), which calls LicenseReq's derived cmp, which in turn calls LicenseItem::cmp. So the same </> comparison on LicenseReq now resolves through a different one of LicenseItem's two disagreeing methods, changing the result.

To Reproduce
Steps to reproduce the behavior:

1. Build against rustc 1.98.0-beta.1
2. Run cargo test
3. licensee::test::handles_close and licensee::test::handles_or_later fail

Expected behavior
partial_cmp should agree with cmp for all inputs — e.g. by having partial_cmp delegate as Some(self.cmp(other)), per PartialOrd's documentation.

Device:

• Version: spdx 0.13.4 with rustc 1.98.0-beta.1

Additional context

• Crater log showing the failure: https://crater-reports.s3.amazonaws.com/beta-1.98-4/1.98.0-beta.1/reg/spdx-0.13.4/log.txt

• Upstream rustc issue: 1.98 beta regression: Behavior change involving comparisons rust-lang/rust#159519

• The rustc PR changing this derive behavior: rust-lang/rust#155598. This strategy is, at the very least, endorsed by the documentation:

This trait should only contain the comparison logic for a type if one plans on only implementing PartialOrd but not Ord. Otherwise the comparison logic should be in Ord and this trait implemented with Some(self.cmp(other)).

[theemathas]

Note: It's not finalized yet whether this behavior will be released as part of rust 1.98.0 or not.

[theemathas]

Pinging @Jake-Shadle, since you appear to be the main maintainer for this repo.

Note that rust 1.98.0, which might or might not change the behavior of the spdx crate, is scheduled to be released on August 20.