fix: reorder untagged enum variants to put Integer before Number

Cherry-picked from upstream PR oxidecomputer#991. When a schema defines an untagged
enum with both integer and number types, serde tries variants in order.
Having Number (f64) before Integer (i64) made the Integer variant
unreachable since any integer is also a valid f64.

Introduces ReorderedInstanceType to ensure Integer is always matched
before Number during deserialization.

Author: ndreno

typify-impl/src/convert.rs

@@ -7,7 +7,7 @@ use crate::type_entry::{
     EnumTagType, TypeEntry, TypeEntryDetails, TypeEntryEnum, TypeEntryNewtype, TypeEntryStruct,
     Variant, VariantDetails,
 };
-use crate::util::{all_mutually_exclusive, ref_key, StringValidator};
+use crate::util::{all_mutually_exclusive, ref_key, ReorderedInstanceType, StringValidator};
 use log::{debug, info};
 use schemars::schema::{
     ArrayValidation, InstanceType, Metadata, ObjectValidation, Schema, SchemaObject, SingleOrVec,
@@ -672,7 +672,13 @@ impl TypeSpace {
             } => {
                 // Eliminate duplicates (they hold no significance); they
                 // aren't supposed to be there, but we can still handle it.
-                let unique_types = instance_types.iter().collect::<BTreeSet<_>>();
+                // Convert the types into a form that puts integers before numbers to ensure that
+                // integer get matched before numbers in untagged enum generation.
+                let unique_types = instance_types
+                    .iter()
+                    .copied()
+                    .map(ReorderedInstanceType::from)
+                    .collect::<BTreeSet<_>>();
 
                 // Massage the data into labeled subschemas with the following
                 // format:
@@ -695,8 +701,9 @@ impl TypeSpace {
                 // but why do tomorrow what we could easily to today?
                 let subschemas = unique_types
                     .into_iter()
+                    .map(InstanceType::from)
                     .map(|it| {
-                        let instance_type = Some(SingleOrVec::Single(Box::new(*it)));
+                        let instance_type = Some(SingleOrVec::Single(Box::new(it)));
                         let (label, inner_schema) = match it {
                             InstanceType::Null => (
                                 "null",

typify-impl/src/util.rs

@@ -933,6 +933,63 @@ impl StringValidator {
     }
 }
 
+/// A re-ordering of JSON schema instance types that puts integer values before
+/// number values.
+///
+/// This is used for untagged enum generation to ensure that integer values
+/// are matched before number values.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub(crate)  enum ReorderedInstanceType {
+    /// The JSON schema instance type `null`.
+    Null,
+
+    /// The JSON schema instance type `boolean`.
+    Boolean,
+
+    /// The JSON schema instance type `integer`.
+    Integer,
+
+    /// The JSON schema instance type `number`.
+    Number,
+
+    /// The JSON schema instance type `string`.
+    String,
+
+    /// The JSON schema instance type `array`.
+    Array,
+
+    /// The JSON schema instance type `object`.
+    Object,
+}
+
+impl From<InstanceType> for ReorderedInstanceType {
+    fn from(instance_type: InstanceType) -> Self {
+        match instance_type {
+            InstanceType::Null => Self::Null,
+            InstanceType::Boolean => Self::Boolean,
+            InstanceType::Object => Self::Object,
+            InstanceType::Array => Self::Array,
+            InstanceType::Integer => Self::Integer,
+            InstanceType::Number => Self::Number,
+            InstanceType::String => Self::String,
+        }
+    }
+}
+
+impl From<ReorderedInstanceType> for InstanceType {
+    fn from(instance_type: ReorderedInstanceType) -> Self {
+        match instance_type {
+            ReorderedInstanceType::Null => Self::Null,
+            ReorderedInstanceType::Boolean => Self::Boolean,
+            ReorderedInstanceType::Object => Self::Object,
+            ReorderedInstanceType::Array => Self::Array,
+            ReorderedInstanceType::Integer => Self::Integer,
+            ReorderedInstanceType::Number => Self::Number,
+            ReorderedInstanceType::String => Self::String,
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use std::collections::BTreeMap;
@@ -944,7 +1001,7 @@ mod tests {
     };
 
     use crate::{
-        util::{decode_segment, sanitize, schemas_mutually_exclusive, Case},
+        util::{decode_segment, sanitize, schemas_mutually_exclusive, Case, ReorderedInstanceType},
         Name,
     };
 
@@ -1121,4 +1178,22 @@ mod tests {
         assert!(ach.is_valid("Meshach"));
         assert!(!ach.is_valid("Abednego"));
     }
+
+    #[test]
+    fn test_instance_type_ordering() {
+        let null = ReorderedInstanceType::Null;
+        let boolean = ReorderedInstanceType::Boolean;
+        let integer = ReorderedInstanceType::Integer;
+        let number = ReorderedInstanceType::Number;
+        let string = ReorderedInstanceType::String;
+        let array = ReorderedInstanceType::Array;
+        let object = ReorderedInstanceType::Object;
+
+        assert!(null < boolean);
+        assert!(boolean < integer);
+        assert!(integer < number);
+        assert!(number < string);
+        assert!(string < array);
+        assert!(array < object);
+    }
 }

typify/tests/schemas/multiple-instance-types.rs

@@ -41,14 +41,14 @@ pub mod error {
 #[derive(:: serde :: Deserialize, :: serde :: Serialize, Clone, Debug)]
 #[serde(untagged)]
 pub enum IntOrStr {
-    String(::std::string::String),
     Integer(i64),
+    String(::std::string::String),
 }
 impl ::std::fmt::Display for IntOrStr {
     fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
         match self {
-            Self::String(x) => x.fmt(f),
             Self::Integer(x) => x.fmt(f),
+            Self::String(x) => x.fmt(f),
         }
     }
 }
@@ -79,31 +79,31 @@ impl ::std::convert::From<i64> for IntOrStr {
 pub enum OneOfSeveral {
     Null,
     Boolean(bool),
-    Object(::serde_json::Map<::std::string::String, ::serde_json::Value>),
-    Array(::std::vec::Vec<::serde_json::Value>),
-    String(::std::string::String),
     Integer(i64),
+    String(::std::string::String),
+    Array(::std::vec::Vec<::serde_json::Value>),
+    Object(::serde_json::Map<::std::string::String, ::serde_json::Value>),
 }
 impl ::std::convert::From<bool> for OneOfSeveral {
     fn from(value: bool) -> Self {
         Self::Boolean(value)
     }
 }
-impl ::std::convert::From<::serde_json::Map<::std::string::String, ::serde_json::Value>>
-    for OneOfSeveral
-{
-    fn from(value: ::serde_json::Map<::std::string::String, ::serde_json::Value>) -> Self {
-        Self::Object(value)
+impl ::std::convert::From<i64> for OneOfSeveral {
+    fn from(value: i64) -> Self {
+        Self::Integer(value)
     }
 }
 impl ::std::convert::From<::std::vec::Vec<::serde_json::Value>> for OneOfSeveral {
     fn from(value: ::std::vec::Vec<::serde_json::Value>) -> Self {
         Self::Array(value)
     }
 }
-impl ::std::convert::From<i64> for OneOfSeveral {
-    fn from(value: i64) -> Self {
-        Self::Integer(value)
+impl ::std::convert::From<::serde_json::Map<::std::string::String, ::serde_json::Value>>
+    for OneOfSeveral
+{
+    fn from(value: ::serde_json::Map<::std::string::String, ::serde_json::Value>) -> Self {
+        Self::Object(value)
     }
 }
 #[doc = "`ReallyJustNull`"]