Fix number comparison logic.

src/bin.rs

@@ -366,74 +366,27 @@ impl CompareContextElement for serde_json::Value {
                 .partial_compare(other);
         }
 
-        // Handle other serde_json::Value
-        match other.to_any().downcast_ref::<Self>() {
-            None => (),
-            Some(other_json_value) => {
-                return match (self, other_json_value) {
-                    (
-                        serde_json::Value::Bool(self_boolean),
-                        serde_json::Value::Bool(other_boolean),
-                    ) => self_boolean.partial_cmp(other_boolean),
-                    (
-                        serde_json::Value::Number(self_number),
-                        serde_json::Value::Number(other_number),
-                    ) => return compare_json_numbers(self_number, other_number),
-                    (
-                        serde_json::Value::String(self_string),
-                        serde_json::Value::Number(other_number),
-                    ) => return compare_json_numbers(self_string, other_number),
-                    (
-                        serde_json::Value::Number(self_number),
-                        serde_json::Value::String(other_string),
-                    ) => return compare_json_numbers(self_number, other_string),
-                    (
-                        serde_json::Value::String(self_string),
-                        serde_json::Value::String(other_string),
-                    ) => self_string.partial_cmp(other_string),
-                    (
-                        serde_json::Value::Array(_self_array),
-                        serde_json::Value::Array(_other_array),
-                    ) => {
-                        // TODO: is this reachable given the early convert to string before this block?
-                        return self
-                            .render(&Vec::new())
-                            .unwrap_or("".to_owned())
-                            .partial_cmp(
-                                &other_json_value
-                                    .render(&Vec::new())
-                                    .unwrap_or("".to_owned()),
-                            );
-                    }
-                    _ => None,
-                };
-            }
+        let maybe_json_other = other.to_any().downcast_ref::<Self>();
+        let maybe_literal_other = other.to_any().downcast_ref::<OwnedLiteral>();
+
+        // If they're both strings, compare them directly
+        match (self, maybe_json_other, maybe_literal_other) {
+            // If they're both strings, compare them directly
+            (
+                serde_json::Value::String(self_string),
+                Some(serde_json::Value::String(other_string)),
+                _,
+            ) => return self_string.partial_cmp(&other_string),
+            (
+                serde_json::Value::String(self_string),
+                _,
+                Some(OwnedLiteral::LString(other_string)),
+            ) => return self_string.partial_cmp(&other_string),
+            // Otherwise, convert to numbers are compare them that way
+            (_, Some(json_other), _) => return compare_json_numbers(self, json_other),
+            (_, _, Some(literal_other)) => return compare_json_numbers(self, literal_other),
+            _ => panic!("Unimplemented comparison type."),
         }
-        // Handle literals
-        match (self, other.to_any().downcast_ref::<OwnedLiteral>()) {
-            (_, None) => (),
-            (serde_json::Value::String(self_text), Some(OwnedLiteral::LString(other_text))) => {
-                return self_text.partial_cmp(other_text);
-            }
-            (_, Some(OwnedLiteral::LString(text))) => {
-                return self.to_string().partial_cmp(text);
-            }
-            (_, Some(OwnedLiteral::LPositiveInteger(other_num))) => {
-                let other_json_num: serde_json::Number = std::convert::From::from(*other_num);
-                return self.partial_compare(
-                    &serde_json::Value::Number(other_json_num) as &dyn ContextElement
-                );
-            }
-            (_, Some(OwnedLiteral::LFloat(other_num))) => {
-                let other_json_num = serde_json::Number::from_f64(*other_num);
-                match other_json_num {
-                    None => return None,
-                    Some(ojn) => return self
-                        .partial_compare(&serde_json::Value::Number(ojn) as &dyn ContextElement),
-                }
-            }
-        };
-        None
     }
 }
 
@@ -445,6 +398,32 @@ enum JsonNumber {
     Failure,
 }
 
+impl From<&serde_json::Value> for JsonNumber {
+    /// Convert from a JSON value to a number for comparison based on
+    /// the logic described at
+    /// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Greater_than
+    fn from(original: &serde_json::Value) -> Self {
+        match original {
+            serde_json::Value::Null => JsonNumber::UnsignedInteger(0),
+            serde_json::Value::Bool(boolean) => {
+                if *boolean {
+                    JsonNumber::UnsignedInteger(1)
+                } else {
+                    JsonNumber::UnsignedInteger(0)
+                }
+            }
+            serde_json::Value::Number(num) => num.into(),
+            serde_json::Value::String(text) => text.into(),
+            serde_json::Value::Array(_) => {
+                panic!("Only primitives should be cast to numbers for comparisons")
+            }
+            serde_json::Value::Object(_) => {
+                panic!("Only primitives should be cast to numbers for comparisons")
+            }
+        }
+    }
+}
+
 impl From<&String> for JsonNumber {
     fn from(original: &String) -> Self {
         match original.parse::<u64>() {