From fc191669325d862306622f3bf6e3138bfa17f49c Mon Sep 17 00:00:00 2001
From: CarloMicieli <carlo.micieli@gmail.com>
Date: Mon, 9 Oct 2023 08:04:00 +0200
Subject: [PATCH] fix: fix the clippy violations

---
 .../src/catalog_items/delivery_date.rs        | 30 +++++++++++++++----
 crates/catalog/src/scales/ratio.rs            |  8 ++++-
 crates/catalog/src/scales/scale.rs            |  4 +--
 crates/cli/src/dataset/mod.rs                 |  2 +-
 4 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/crates/catalog/src/catalog_items/delivery_date.rs b/crates/catalog/src/catalog_items/delivery_date.rs
index 601b5c14..d5a151c7 100644
--- a/crates/catalog/src/catalog_items/delivery_date.rs
+++ b/crates/catalog/src/catalog_items/delivery_date.rs
@@ -53,7 +53,7 @@ impl DeliveryDate {
         let year = s
             .parse::<Year>()
             .map_err(|_| DeliveryDateParseError::InvalidYearValue)?;
-        if year < 1900 && year >= 2999 {
+        if !(1900..=2999).contains(&year) {
             return Err(DeliveryDateParseError::InvalidYearValue);
         }
 
@@ -68,7 +68,7 @@ impl DeliveryDate {
         let quarter = s[1..]
             .parse::<Quarter>()
             .map_err(|_| DeliveryDateParseError::InvalidQuarterValue)?;
-        if quarter < 1 && quarter >= 4 {
+        if !(1..=4).contains(&quarter) {
             return Err(DeliveryDateParseError::InvalidQuarterValue);
         }
 
@@ -87,7 +87,7 @@ impl str::FromStr for DeliveryDate {
         if s.contains('/') {
             let tokens: Vec<&str> = s.split_terminator('/').collect();
             if tokens.len() != 2 {
-                return Err(DeliveryDateParseError::InvalidByQuarterValue);
+                return Err(DeliveryDateParseError::InvalidDeliveryDateFormat);
             }
 
             let year = DeliveryDate::parse_year(tokens[0])?;
@@ -149,12 +149,12 @@ impl<'de> Deserialize<'de> for DeliveryDate {
 }
 
 /// The delivery date parsing errors enum
-#[derive(Debug, Error)]
+#[derive(Debug, Error, PartialEq)]
 pub enum DeliveryDateParseError {
     #[error("Delivery date cannot be empty")]
     EmptyValue,
-    #[error("Invalid delivery date by quarter")]
-    InvalidByQuarterValue,
+    #[error("Invalid format for a delivery date")]
+    InvalidDeliveryDateFormat,
     #[error("Delivery date year component is not valid")]
     InvalidYearValue,
     #[error("Delivery date quarter component is not valid")]
@@ -206,6 +206,24 @@ mod tests {
             assert_eq!(expected, result);
         }
 
+        #[rstest]
+        #[case("2020/Q11", DeliveryDateParseError::InvalidQuarterValue)]
+        #[case("2020/Q0", DeliveryDateParseError::InvalidQuarterValue)]
+        #[case("2020/Q5", DeliveryDateParseError::InvalidQuarterValue)]
+        #[case("2020/QA", DeliveryDateParseError::InvalidQuarterValue)]
+        #[case("202/Q1", DeliveryDateParseError::InvalidYearValue)]
+        #[case("1899/Q1", DeliveryDateParseError::InvalidYearValue)]
+        #[case("3000/Q1", DeliveryDateParseError::InvalidYearValue)]
+        #[case("3000/Q1/?", DeliveryDateParseError::InvalidDeliveryDateFormat)]
+        fn it_should_fail_to_parse_invalid_delivery_dates(
+            #[case] input: &str,
+            #[case] expected: DeliveryDateParseError,
+        ) {
+            let result = input.parse::<DeliveryDate>();
+            assert!(result.is_err());
+            assert_eq!(result.unwrap_err(), expected);
+        }
+
         #[test]
         fn it_should_deserialize_delivery_dates() {
             let test_struct = TestStruct {
diff --git a/crates/catalog/src/scales/ratio.rs b/crates/catalog/src/scales/ratio.rs
index ab271154..79e246ec 100644
--- a/crates/catalog/src/scales/ratio.rs
+++ b/crates/catalog/src/scales/ratio.rs
@@ -84,7 +84,13 @@ impl convert::AsRef<Decimal> for Ratio {
 
 impl cmp::PartialOrd for Ratio {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        other.0.partial_cmp(&self.0)
+        Some(self.cmp(other))
+    }
+}
+
+impl cmp::Ord for Ratio {
+    fn cmp(&self, other: &Self) -> Ordering {
+        other.0.cmp(&self.0)
     }
 }
 
diff --git a/crates/catalog/src/scales/scale.rs b/crates/catalog/src/scales/scale.rs
index c7128841..6822c45d 100644
--- a/crates/catalog/src/scales/scale.rs
+++ b/crates/catalog/src/scales/scale.rs
@@ -114,13 +114,13 @@ impl cmp::Eq for Scale {}
 
 impl cmp::PartialOrd for Scale {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        self.ratio.partial_cmp(other.ratio())
+        Some(self.cmp(other))
     }
 }
 
 impl cmp::Ord for Scale {
     fn cmp(&self, other: &Self) -> Ordering {
-        self.partial_cmp(other).unwrap()
+        self.ratio.cmp(other.ratio())
     }
 }
 
diff --git a/crates/cli/src/dataset/mod.rs b/crates/cli/src/dataset/mod.rs
index b9d458ec..9b64571a 100644
--- a/crates/cli/src/dataset/mod.rs
+++ b/crates/cli/src/dataset/mod.rs
@@ -126,7 +126,7 @@ impl fmt::Display for Resource {
 
 impl PartialOrd for Resource {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        self.file_name.partial_cmp(&other.file_name)
+        Some(self.cmp(other))
     }
 }